mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-28 04:37:37 +00:00
127 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ravi Kumar L
|
a70bcf66a4
|
🪢 feat: show Langfuse session link in shared chats (#15273)
Some checks are pending
Backend Unit Tests / Build packages (push) Waiting to run
Backend Unit Tests / Codegraph select (push) Waiting to run
Backend Unit Tests / TypeScript type checks (push) Blocked by required conditions
Backend Unit Tests / Circular dependency checks (push) Waiting to run
Backend Unit Tests / Tests: api (shard 1/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 2/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 3/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: data-provider (push) Blocked by required conditions
Backend Unit Tests / Tests: data-schemas (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 1/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 2/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 3/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 4/4) (push) Blocked by required conditions
Codegraph E2E Votes / vote (full suite) (push) Waiting to run
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
Frontend Unit Tests / Codegraph select (push) Waiting to run
Frontend Unit Tests / Build packages (push) Waiting to run
Frontend Unit Tests / TypeScript type checks (client) (push) Blocked by required conditions
Frontend Unit Tests / Tests: @librechat/client (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 1/2) (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 2/2) (push) Blocked by required conditions
Frontend Unit Tests / Vite build verification (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
* feat: show Langfuse sessions in shared chats * fix: wait for optional auth before loading shares * fix: require Langfuse access for shared session links * refactor: move shared Langfuse policy into API package * fix: wrap shared chat actions on mobile * fix: coordinate shared Langfuse link loading * perf: batch shared Langfuse capability checks |
||
Danny Avila
|
69e7c73614
|
🎛️ feat: Expose Authoritative Subagent Controls (#15169)
* feat: expose authoritative subagent controls * fix: reconcile subagent control races * fix: reconcile durable control conflicts * fix: preserve authoritative subagent control outcomes * fix: fence subagent controls to child thread * fix: validate subagent control targets before routing * fix: close subagent control boundary gaps * fix: keep control reservations private * fix: close subagent control admission gaps * fix: preserve authoritative control history * style: sort subagent control imports * fix: preserve authoritative subagent control retries * style: sort control state imports |
||
|
Danny Avila
|
d864597731
|
⚡ perf: Append Saved Message Ids Instead of Rebuilding the Conversation Array (#15141)
* ⚡ perf: Append Saved Message Ids Instead of Rebuilding the Conversation Array Every saveConvo read every message id in the conversation (sorted) and wrote the array back onto the document — twice per chat turn, O(n) in conversation length, from a write path. The turn's savers know exactly which message they just wrote, so they now pass it as metadata.appendMessageIds and saveConvo $addToSet-s it, skipping the read and the full-array rewrite. Every save without the option — titles, archive, fork, import, threads — still rebuilds from the database, which remains the heal point for the drift that message deletion has always left behind (deletes never ran saveConvo). The array's consumers read presence or length, or use it as an optimistic cache placeholder, so incremental maintenance is behaviorally identical; on traced turns the array stays exactly equal to the messages collection. Per-turn queries: 15 -> 13 (two Message.find gone), and the growing array payload no longer crosses the wire twice per turn. * 🎯 fix: Brand the Lineage-Only Resolved Conversation Instead of Guessing by Shape The resolved-conversation files fast path treated an absent files property as unresolved so the lineage-only partial from a bound agent-event continuation could not silently hide a conversation's uploads. But MongoDB never stores an empty files array, so nearly every real conversation also lacks the property and the fast path never fired — a follow-up turn on an upload-free conversation still paid the getConvoFiles round trip. The synthesized partial is the one object that cannot speak for the database, so it now carries an explicit symbol brand (PARTIAL_RESOLVED_CONVERSATION, non-serializing and invisible to key iteration), and a stored document without files means what it means: no files. Traced follow-up turns drop from 14 queries to 13. * 🧪 test: Expect the Appended Message Id in the Route's saveConvo Metadata messages-get.spec.js pins the exact metadata POST /api/messages passes to saveConvo; the route now forwards the saved message's _id as appendMessageIds, which is the behavior the append path depends on. |
||
|
Danny Avila
|
1de88e7e91
|
📨 feat: Continue Bound Child Agents from Events (#15112)
* feat: add authenticated agent event ingress * style: sort agent ingress imports * fix: harden agent event ingress * fix: bind event provenance to API keys * fix: inspect event input with legacy PII filters * fix: scope event status reads to source keys * fix: bind event status reads to remote sources * feat: add bound event-driven child turns * fix: harden event-bound child continuations * fix: satisfy event binding type contracts * fix: close event actor lifecycle races * fix: harden event actor dispatch continuity * fix: fence event actor resume lifecycle * fix: bind event actor state to lifecycle * fix: preserve cascade write outcomes * test: type cascade failure injection * style: sort cascade test imports * fix: harden event child lifecycle boundaries * fix: make event cleanup retryable * fix: annotate event retention clock * fix: reconcile partial cascade metadata * fix: recheck event binding expiry on resume * fix: fence event actors by retention deadline * fix: close event actor lifecycle races * fix: harden event child lease acquisition * fix: lazy-load event child lease adapter |
||
|
Danny Avila
|
d3e70159ca
|
📡 feat: Stream Detached Subagent Activity (#15111)
* feat: stream detached subagent activity * fix: annotate activity stream limits * fix: isolate subagent activity imports * fix: harden detached subagent activity lifecycle * test: cover synchronous activity transport failure * test: include required subagent activity identity * fix: identify and reconnect subagent activity events * fix: bound subagent activity lifecycles * fix: close subagent activity handoff races * fix: bind and synchronize activity subscriptions * fix: detect fresh activity attachment * fix: complete activity synchronization handoff * fix: bind activity sync and failure circuits * fix: expose subscription-bound synchronization * fix: fence activity reconnect publications * test: make detached timeout settlement deterministic * fix: fence Redis activity attachments * fix: close failed activity streams * perf: reuse fenced activity frontier * style: sort subagent thread imports * fix: preserve queued subagent activity * test: type activity publication counter * fix: disconnect subagent activity subscriber * fix: close background activity lifecycle gaps * fix: preserve streamed activity spacing * fix: preserve bounded live subagent activity * fix: merge durable subagent activity safely * fix: model detached activity coverage * fix: type detached activity inputs * fix: order overlapping subagent activity * chore: sort activity test imports * fix: buffer subagent activity handoff gaps * fix: flush activity after parent close * fix: advance closed activity suffixes * fix: preserve detached activity ordering * fix: close detached activity delivery races * fix: bound shared Redis subscriber readiness * fix: expire shared Redis subscription readiness * fix: clean up late Redis subscriptions * fix: preserve late Redis subscription fallback |
||
|
Danny Avila
|
67b7b441b2
|
🛂 feat: Filter Model-Bound Content by Source (#14425)
* feat: introduce optional content protection seam * feat: enforce source-aware content filters * feat: complete source-aware content enforcement * test: activate skill file-text fail-close fixtures * fix: harden source-aware content filters * fix: harden model-bound content filtering * fix: preserve legacy filters and generated files * fix: inspect shared scalar metadata * test: align mocks with current dev dependencies * feat: add persisted content filter safeguards * feat: complete source-aware content filter enforcement * fix: move resume content preflight into TypeScript * fix: close content inspection edge cases * fix: harden content protection boundaries * fix: complete content protection safeguards * test: align persisted memory filter coverage * fix: reconcile content protection with current dev * fix: reconcile content protection with latest dev * fix: close content protection review gaps * fix: enforce source-aware provider boundaries * fix: preserve legacy PII preflight semantics * test: stabilize stored branch preflight fixture * fix: defer agent writes until protected model admission * perf: harden source-aware model-bound filtering * fix: canonicalize provider lineage before validation * fix: satisfy model-bound callback type checks * perf: Bound content protection filtering work * fix: Bound submission array traversal * fix: Stabilize bounded content snapshots * fix: Scope model-bound traversal overflows * fix: Preserve scoped content inspection * fix: Accumulate aggregate traversal scopes * fix: centralize content policy boundaries * test: align deferred tool policy context * test: align controller policy mocks * style: normalize content protection imports * fix: close content policy review gaps * fix: narrow active skill policy config * fix: address content protection review boundaries * fix: retain exact provenance overflow sentinel * fix: preserve literal and scoped provenance updates * fix: narrow persisted edit provenance * fix: isolate exact overflow attribution * fix: centralize stored prompt protection * fix: fail closed on incomplete transcript evidence * fix: align canonical transcript routing * refactor: centralize content policy preflights * fix: isolate upload policy error typing * style: sort policy preflight imports * refactor: centralize content policy boundaries |
||
|
Danny Avila
|
8ae94afa91
|
🪡 fix: Thread Parent Message ID Through MCP Request-Scoped Bodies (#15095)
* fix: Unify MCP request-scoped headers * fix: address request-scoped MCP review findings * test: preserve request scope on status errors * fix: treat authorized on-demand MCP servers as ready * refactor: separate MCP readiness from connection state * fix: preserve on-demand MCP readiness labels * test: satisfy OpenAI conversation ownership guard * fix: keep MCP action predicates boolean * fix: close deferred MCP request context gaps * fix: preserve on-demand MCP configuration actions * fix: fail closed on unavailable MCP parent context * test: complete MCP connecting-state mocks * fix: preserve missing MCP parent on continuations * fix: align native MCP request identities * fix: preserve edited MCP parent identity * test: use scoped Agent initializer fixture * test: expose MCP request body helper * fix: preserve MCP turn identity across resume * style: sort stream metadata imports * fix: carry normalized MCP identity to execution |
||
Oliver Faust
|
b399ad8370
|
📄 fix: Serve Stored Text for "Upload as Text" File Downloads (#14723)
* 📄 fix: Serve Stored Text for Text-Source File Downloads
"Upload as Text" attachments store extracted content in the DB with
source 'text'; OCR uploads persist the OCR strategy name (e.g.
'mistral_ocr') as a filepath placeholder since no backing file exists.
The download route resolved these records to the local strategy and
passed the placeholder to fs.createReadStream, which failed with
ENOENT — and the response was never ended after the stream error, so
the request hung until the client timed out.
Serve the stored text directly as a .txt download for text-source
files (re-fetched by _id, as getFiles excludes 'text' by default),
and end the response on stream errors: 500 without the download
headers before headers are sent, otherwise abort the truncated
response so clients detect the failure.
* fix: Preserve text-source preview semantics
* fix: Complete text-source download coverage
* fix: Tie text downloads to blob lifecycle
* fix: Isolate preview downloads and share text snapshots
* fix: Keep shared previews in share scope
* style: Sort text download imports
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
Marco Beretta
|
29f6ec6eae
|
🙈 feat: Config Option to Hide Response Feedback Buttons (#15085)
* feat: add interface option to hide response feedback buttons Adds `interface.feedback` to librechat.yaml. When set to false, the thumbs up/thumbs down buttons are removed from the message action row and the feedback endpoint rejects writes with 403, so deployments that do not consume the data can stop collecting it. Defaults to true. * refactor: hoist the feedback gate out of the message row and into typed middleware Reading startup config inside HoverButtons put a query observer and two Recoil subscriptions on every message row, and rows never unmount, so the cost grew with the conversation. Resolve the flag once per chat in useChatHelpers and carry it on TMessageChatContext; useMessageActions withholds handleFeedback when it is off, which the action row already treats as "no feedback controls". The flag now stays false until the config resolves, so a disabled deployment never flashes controls whose writes are rejected. Move the server-side policy into requireFeedbackEnabled under packages/api so the route keeps no policy of its own. * test: stub the feedback gate in specs that replace the api package The messages router now imports requireFeedbackEnabled, and express rejects an undefined handler at require time, so every spec that mocks @librechat/api wholesale has to carry the export. |
||
|
Danny Avila
|
d0f9d5625e
|
🧵 fix: Close Child-Thread Read and Search-Cleanup Gaps (#15055)
* fix: close child thread read and cleanup gaps * fix: preserve child search cleanup invariants * test: complete mocked update result * perf: parallelize scoped message reads * fix: close child thread compatibility gaps * test: expect preserved cleanup failure * fix: reconcile legacy Meili cleanup markers |
||
|
Danny Avila
|
c5276fc63d
|
⏱️ feat: Run Scheduled Chats Through Durable Agent Triggers (#14939)
* feat: Scheduled Chats — agent-centric scheduled runs creating real conversations
Squash of the full review-hardened branch (PR 14540, supersedes 14373) onto
latest dev, preserving the exact verified tree. History prior to this commit
lived on the pre-squash branch; every invariant below survived 25 Codex review
rounds plus two external audit rounds (R26) with regression tests that fail
without their fixes.
Feature:
- Schedules CRUD + side-panel UI (cadence dialog, run cards, Run Now), roles/
permissions (SCHEDULES:USE), interface.schedules availability, per-user limits
and capacity slots, timezone-aware cadence with DST-conservative floors and
misfire grace.
- Engine: single-process claim/fire loop with leases, loopback POST dispatch
(signed schedule-fire JWT claims, per-occurrence idempotency key inside the
route's clientRequestId charset), overlap/balance/capacity/duplicate skip
policies with auto-disable streaks (too_many_failures, insufficient_balance),
reconciliation from retained terminal-job evidence, erasure sweep.
- Scheduled runs create real conversations through the resumable agents chat
path: HITL pauses surface on the card (requires_action), resumes re-apply the
fire boundary's admission policy (revision fence, enabled, global kill switch,
SCHEDULES:USE, availability) before continuing a billed generation.
Correctness invariants (the audit surface):
- Settlement discipline: every persistence-producing write happens-before a
run's terminal outcome write; Stop/complete/pause race through single-winner
terminal CAS claims (dev's TerminalJobClaim substrate) with retained,
completedAt-less evidence for scheduled fires plus an owner-intended outcome
stamp (scheduleOutcome) the reconciler prefers over re-derived success —
round-tripped through the Redis hash mapper.
- Swallowed generation failures (client error content parts) classify to
error/skipped_balance instead of success on both initial and resumed paths;
stale stamps are refreshed evidence-first when persistence plus the Mongo
outcome write both fail.
- Abort honesty: delivery judged by generation ownership and the CAS's actual
from-status; republication escalates to the transport's acknowledged variant;
the Stop route settles only genuinely paused runs.
- Account deletion: one-way barrier (deletionRequestedAt) with auth-cache
tombstone-before-stamp, boundary rechecks across all auth strategies, quiesce
of scheduled + interactive work with durable per-stream abort fences
(positive-evidence acknowledgement only), owner-side finalization markers for
post-terminal billed writes, deferred-deletion sweep (explicitly ensured
partial index) that completes cascades autonomously. Remote OpenAI-compatible/
Responses requests are documented as outside the quiesce and tracked in issue
14594.
- Store compatibility: finalization markers optional on the legacy IJobStore
contract with coherent degradation (registration fails -> synchronous title
fallback; count reads 0).
* fix: fence terminal response persistence from deletion; retain stale-pause evidence
Two blockers from the third external review round.
Terminal persistence visible to account deletion:
- The finalization marker was registered only when post-terminal TITLE work was
possible, but every persistence-owning terminal CAS opens the same window: the
claim drops the job out of the active set BEFORE the response save (and the
background user-message/convo saves), so a deletion quiesce landing there saw
neither an active job nor a marker and could cascade while the admitted request
could still recreate messages. Both controllers now register the marker before
every persistence-owning claim — the fresh-turn path and the HITL resume — and
release it only after their pending saves (and any post-terminal title) have
landed, on success and failure paths alike. The TTL bounds a crash.
- settleAbortFence no longer clears a complete/error fence while
`terminalPersistencePending` is set: terminal at the CAS is not settled while
the owner is still persisting. The job facade now surfaces the flag.
- The marker trio is REQUIRED by the runtime store contract (assertJobStoreV2
refuses a store without it at configure time, keeping the failure loud and
deterministic) while remaining optional on the legacy public IJobStore type for
source compatibility. The silent degrade path from the previous round is gone —
it was not deletion-safe.
Stale-pause recovery retains scheduled evidence:
- Three crash/timeout recovery paths — ApprovalLifecycle.failStalePausePersistence
and the InMemory/Redis stale-pause cleanups — unconditionally stamped
`completedAt`, putting a scheduled fire's failed-pause error terminal on the
short completed TTL. A Mongo outage longer than that TTL erased the evidence and
the reconciler recovered the run as `interrupted` instead of `error`. All three
now follow the controller-observed path from the previous round: scheduled jobs
omit `completedAt` (retained-evidence TTL) and stamp the error outcome.
The PR description now explicitly narrows the deletion guarantee for the remote
OpenAI-compatible/Responses paths (tracked in issue 14594).
* fix: deletion-fence marker protocol — generation-scoped, atomic, fail-closed, all terminal paths
One consolidated pass over the finalization-marker mechanism, per the fourth
external review round. The invariant it establishes: NO persistence-owning
terminal CAS runs without a durable, generation-qualified marker covering the
window it opens, and every consumer treats a pending terminal as unsettled.
- Generation-scoped markers. Entries were keyed (userId, streamId), so a
Stop-superseded generation finishing late could clear the marker its
replacement registered on the same conversation. Marker fields are now
qualified by the generation's createdAt; clears must present the same
identity, and an unqualified legacy clear cannot drop a qualified entry.
- Atomic Redis registration. HSET-then-EXPIRE loses the fresh marker when the
user's existing hash expires between the two commands (or the process dies
there); registration is now a single Lua script carrying both.
- Fail closed everywhere. Registration failure (after one retry) now REFUSES
the terminal CAS instead of proceeding uncovered: the completion claim throws
into the error path, the error path skips completeJob and leaves the job
ACTIVE — deletion-visible by itself, recovered by the stale-running reaper —
and abortJob returns a new retryable `fence_unavailable` failure the Stop
route answers with 503 and the deletion quiesce treats as an unacknowledged
stop (fence kept). The previous round's log-and-proceed is gone.
- Every terminal path enrolled. abortJob now owns its window (register before
the abort CAS, clear in its finally — the Stop route's checkpoint prune and
partial save run inside beforePublish, between CAS and publication); the
interactive and background generation-error paths register before their
completeJob; the resume controller's error finalization registers before its
completeJob. A lost or thrown claim releases the marker after pending saves
flush instead of holding the user's deletion behind the TTL.
- Every pending terminal unsettled. settleAbortFence defers on
terminalPersistencePending for ALL statuses — including `aborted`, whose
route-side persistence the previous guard missed.
Also: a direct Redis regression for scheduled stale-pause retention (the P2
test gap), and the PR description no longer claims to carry every commit.
* fix: lease-token lifecycle fences — same-generation isolation, admission fence, undelivered-Stop retention, legacy abort enrollment
Fifth external review round; four P1s, handled as the requested consolidated
lifecycle-fence pass.
- Lease tokens. Marker fields were (streamId, createdAt), shared by every
contender on the same generation — completion and Stop, or two racing Stops —
so a losing contender's cleanup erased the winner's still-live marker. Every
registrant now carries a unique lease token in the field and may only ever
clear its own lease; unqualified legacy clears cannot touch qualified entries.
- Admission fence. Authentication can pass before the deletion barrier goes up,
and the durable createJob is several async steps later — a deletion quiesce in
that window saw neither an active job nor a marker and could cascade before
the admitted request created its job. The controller now registers an
admission lease and THEN rereads the deletion barrier: the ordering guarantees
either this request observes the barrier (403, lease released, slot/claim
cleanup) or the quiesce observes the lease and defers. Held until createJob is
durable; released on every refusal and initialization-error path. Fail closed
when the lease itself cannot be registered (503 retryable).
- Undelivered-Stop retention. abortJob released its lease in a finally even when
delivery AND publication had provably failed — the job reads terminal
(invisible to active-set scans), a user Stop writes no durable abort fence,
and the remote owner keeps generating and will persist its abort-catch writes
whenever the signal finally lands. The lease is now retained in exactly that
case, and each resignal attempt heartbeats a fresh lease so the fence outlives
the TTL for as long as delivery is still being driven. The abort-winning
turn's own loser-side pending saves are additionally fenced in the controller
catch (best-effort — those writes are already in flight).
- Legacy abort enrollment. abortMiddleware (assistants abort route fallback for
non-assistants endpoints) awaited abortJob and then spent usage and saved the
stopped response AFTER the abort's lease was released. Both writes now run
inside `beforePublish`, between the abort CAS and publication, covered by the
same lease as every other abort.
Barrier tests, each verified to fail without its fix: same-generation lease
isolation (store), racing two-Stop loser cleanup (manager, stale-read forced
CAS race), undelivered-Stop lease retention, resignal heartbeat, admission
refusal with lease-before-reread ordering plus release-on-durable-create, and
legacy-abort persistence inside beforePublish.
* fix: heartbeat-backed owner-lifecycle leases close the settlement handoff races
Sixth external review round: the remaining P1 interleavings were one structural
problem — lease handoffs that were not atomic — resolved as the requested
consolidated lifecycle-lease pass.
- Quiesce reads leases BEFORE the active-job scan. The admission-lease -> durable
-job handoff is only atomic against a reader in the OPPOSITE order of the
writer: writers hold the lease strictly until the job is active-set visible,
so leases-first shows every interleaving either the lease or the job.
Jobs-first allowed a request to create its job after the scan and release its
lease before the count — hiding both, cascading, and letting the new
generation persist into a deleted account.
- The abort acknowledgement is fenced by the owner-lifecycle lease. Redis ACKed
the moment the owner's AbortController tripped; the stopping side released its
lease on that ACK while the owner's asynchronous abort-catch persistence was
still ahead. The transport now awaits a manager-installed pre-ACK hook that
registers a DETERMINISTIC owner lease (exactly one owner exists per
generation, and determinism is what lets the signal-time registrant and the
owner's catch-side release agree across processes) before the acknowledgement
is persisted or published; an owned same-replica abort bridges to the same
lease before tripping its local controller. Both generation-owner catches
(fresh turn, resume) release it once their writes land.
- A failed replacement handoff no longer orphans the predecessor. The atomic
replacement removes it from active storage, and an unconfirmed handoff
terminalizes the replacement too — leaving nothing a quiesce could discover
while the predecessor's provider may still be generating. Its owner lease is
now retained at the point the receipt fails delivery; the owner replica renews
it through the pre-ACK fence when the signal finally lands.
- Leases HEARTBEAT while held. The five-minute store TTL only bounds a crashed
holder; live persistence — a stalled save, a long deferred title — must never
outlive its own fence. holdUserFinalization registers and renews every minute
until released; the controllers' completion/error/admission leases all hold.
The undelivered-Stop retention moved to a deterministic `stop` lease that
every resignal attempt renews and the first successful one clears (no more
opaque leases accumulating to TTL), and a THROWN abort transition releases the
contender lease instead of leaking it.
- The user-document abort-fence mutations now invalidate the auth user-doc
cache, matching every other user-doc write.
Barrier tests, each verified to fail without its fix: quiesce lease-scan
ordering (plus the observed-lease defer), pre-ACK fence ordering at the
transport, owned-abort owner-lease bridging, replacement-handoff predecessor
retention, held-lease heartbeat past the TTL, and failed-then-successful
resignal reaping the retained stop lease.
* fix: one manager-owned owner-lease span across every abort delivery path
Seventh external review round; four lifecycle-fence gaps, closed by making the
owner-lifecycle lease a single manager-owned, heartbeat-held span.
- Fail-closed acknowledgements. The pre-ACK hook registered a one-shot lease and
the transport ACKed even when it failed; a same-replica owned abort likewise
swallowed registration failure. The hook now acquires a HELD owner lease
(heartbeat until the owner's catch releases it via releaseOwnerLease) and a
rejection SUPPRESSES the acknowledgement — the stopping side stays retryable
behind its retention lease, and every resignal re-drives the handler. The hook
also stopped gating on `job.createdAt === generationId`: during a replacement
handoff the store holds the replacement while the abort targets the
predecessor, and that gate silently skipped exactly the generation being
acknowledged (the store job is owner identity, never a generation gate). A
local owned abort acquires the same held lease before tripping its provider;
post-CAS the trip cannot be withheld, so acquisition failure downgrades
delivery and the retention handoff keeps the user fenced.
- Committed-but-lost-reply disambiguation. A thrown abort transition released
the contender lease as if nothing had happened, but a Lua CAS can commit and
lose its reply — an aborted job invisible to active-set scans whose provider
was never signalled, with no fence left. The throw path now re-reads the exact
generation: only a job still live under the caller's identity proves no
commit; committed or ambiguous outcomes hand the fence to the deterministic
stop lease (kept on the contender lease if even that fails) before rethrowing.
- Replacement handoff covered end to end. A LOCAL replacement abort acquires the
predecessor's held owner lease before the trip (failure reports the receipt
undelivered, engaging retention). Failed-handoff retention is no longer a
swallowed one-shot: it heartbeats with the durable acknowledgement proof as
its renewal predicate — acquisition failures keep retrying for as long as the
fence is needed, and the retainer stands down (without clearing the shared
field) once the remote owner ACKs and thereby holds its own lease.
- A LOCAL resignal delivery hands off to the owner lease BEFORE clearing the
retained stop lease, and keeps the stop lease when that handoff fails.
Barrier tests: hook rejection suppressing the ACK, commit-then-lost-reply
retention with its provably-uncommitted counterpart, local-resignal owner
handoff ordering, pre-ACK owner lease held past the store TTL until release
(and provably stopped after), and failed-handoff retention retrying on its
heartbeat — verified fail-before/pass-after by stashing the fixes.
* fix: finish scheduled chat lifecycle hardening
* fix: close scheduled chat review follow-ups
* fix: generation-fence abort recovery evidence
* test: wait for settled approval tool output
* test: preserve scheduled init reconciliation option
* refactor: rebuild scheduled chats on durable agent triggers
* test: reset MCP cache mock between cases
* test: isolate scheduler startup in server specs
* fix: harden scheduled run lifecycle
* fix: normalize schedule capacity conflicts
* test: type schedule collision fixture
* fix: re-fence scheduled resume and expiry
* fix: fence scheduled resume handoffs
* fix: release superseded manual schedule leases
* fix: release failed run-now claims
* fix: release superseded engine claims
* fix: repair schedule dialog interaction and rework its form
The agent picker was unusable: ControlCombobox portals its popover to the
body by default, which lands it outside the dialog's Radix focus trap. Clicks
passed through it, it could not be tabbed into, and the trap fighting Ariakit
for focus locked the page up on selection. The prop is documented for exactly
this case — pass `portal={false}` and give the dialog `overflow-visible`, as
ProjectButton already does. The time and day dropdowns defaulted the same way.
Alongside that:
- Extract the agent builder's instructions editor (special-variable menu plus
expand-to-fullscreen) into a controlled `VariableEditor` and use it for the
schedule prompt. Insertions now route through `onChange`, so react-hook-form
sees them — the schedule PATCH is built from `dirtyFields`, and a `setValue`
that skipped dirty tracking would have dropped an inserted variable silently.
- Replace the hand-rolled frequency buttons with the shared `Radio`. They marked
the selection with `bg-surface-hover` on an outline button whose hover is the
same token, so the selected option was indistinguishable from a hovered one;
`Radio` is also a real radiogroup rather than four `aria-pressed` toggles.
- Wrap the fields in a real `<form>` and associate the footer button by id, so
Enter submits. Group the frequency, day and time controls in fieldsets.
- Add placeholders for name and prompt, match the textarea fill to the other
fields, and label the hourly case as minutes past the hour.
- Widen the dialog to `md:max-w-3xl` and pair name with agent so the form fits
without scrolling on desktop.
- Move scheduled chats below skills and above prompts in the side nav.
The new dialog spec fails when the portal fix is reverted.
* test: cover scheduled and subagent deletion drains
* refactor: own the form-control appearance in the client primitives
Addresses the codex finding on ScheduleDialog: a feature-local `FIELD_CLASS`
restated the `Input` primitive's border, radius, height and background so it
could be pasted onto the schedule dropdowns, leaving those controls with no
connection to the primitive they were imitating.
Move that appearance into `packages/client/src/components/Field.ts` as the
single source `Input` and `Textarea` now compose, and give `Dropdown` and
`ControlCombobox` a `variant="field"` that applies it. The schedule dialog
passes the variant and carries no class strings of its own.
This also repairs a break the dev merge would otherwise have introduced: the
newer `Dropdown` splits `className` (wrapper) from `triggerClassName`, so the
old pasted classes would have landed on the wrapper and left the triggers
unstyled.
The semantic-token guard now watches the shared module and asserts each
primitive still composes it, which covers more than the two files it read
before.
* fix: keep an explicit schedules disable from becoming an opt-in
`use` is two things at once for a dual-purpose runtime interface field: a
permission bit, which DB overrides strip, and the runtime disable signal that
`getLimits` reads. Stripping it from `{ use: false, maxPerUser: 2 }` leaves an
object, and `getLimits` treats any object without `use: false` as enabled — so
an admin override written to stop scheduled billing for a role or user started
it instead.
Collapse an explicit disable to the boolean form before the strip, on both
paths that accept it: the `interface.schedules` field patch, which admitted the
object wholesale because bare runtime paths deliberately bypass the permission
gate, and the overrides merge, which reached the composite-field branch and
kept `maxPerUser`. Objects that only narrow limits are untouched, so a
principal can still be given a smaller cap.
Both regressions fail without the normalizer.
* fix(schedules): Wave A — null-balance CAS, atomic paused-card clear, clustered erasure sweep
Slice 1 (thread r3804518381): route the existing-null balance initialization
through a { user, tokenCredits: null } compare-and-set instead of a blind $set,
so a concurrent initializer/charge landing between the preflight read and the
write is never handed back its spent starting balance. On a CAS miss the
preflight re-reads the winner. The absent-record $setOnInsert path and the
credited-record refill-config sync are unchanged. Adds the initializeNullBalance
adapter (no upsert) and regression coverage for winner/miss/sync cases.
Slice 2 (thread r3804518388): updateScheduleById now drops a `requires_action`
lastRun projection atomically with the configRevision bump. Any pause present at
edit time was projected under the pre-edit revision and can never be replaced by
its own revision-fenced terminal outcome — a disabling edit would strand the
card on "Needs approval" forever. Implemented as classic-operator CAS branches
(DocumentDB rules out a conditional pipeline $unset), fenced on the card STILL
being the pause so a terminal outcome or newer occurrence that races in is
preserved. Terminal history survives untouched.
Slice 4 (thread r3803826204): expose initializeScheduleErasureSweep from the
schedule runtime facade and start it in every clustered (experimental) worker
after Mongo is up. It re-drives eraseScheduleIfDrained for soft-deleted rows so
a hidden prompt cannot outlive its drain when the delete/erase-on-settle
attempts miss. It arms nothing else and never infers owner death from a
process-local missing job (isTopologySafeToArm gates that).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.1 — reversible account-deletion schedule suspension
Thread r3804518383. Account-deletion quiesce marked every schedule `deleting`,
disabled it, and cleared nextRunAt destructively. When a later cascade step (or
the drain itself) failed, the controller cancelled the user-deletion fence —
restoring the user — but the schedules stayed `deleting` and were erased by the
sweep, silently losing all of a live user's scheduled prompts.
Replace the destructive marking with a REVERSIBLE, token-fenced suspension:
- suspendUserSchedulesForDeletion(userId, token) snapshots each schedule's prior
enabled/nextRunAt under a per-attempt token, then fences firing (disable, clear
nextRunAt, rotate claimToken). It never sets `deleting`, so a suspended row is
not erasure-eligible. Snapshotting reads then bulkWrites (a classic update
cannot copy field values under DocumentDB), fenced per row so an already-
suspended/soft-deleted/edited row is left alone; idempotent per token.
- restoreUserSchedulesFromDeletion(userId, token) reverses it, re-enabling and
re-arming only rows still carrying the exact attempt token and not independently
deleted — so an owner-deleted or newer-attempt-suspended schedule is never
resurrected.
- deleteUserController generates the attempt token, passes it to quiesce, and on
any failure that cancels the user-deletion fence restores the suspended rows. A
successful deletion hard-deletes them (and their snapshots) via the existing
cascade and never restores.
Adds a `deletionSuspension` embedded field (excluded from the wire schedule),
data-method regression tests (suspend/restore/fence/idempotency/no-resurrect),
and controller tests (restore on drain-false and post-quiesce cascade failure,
no restore on success).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.2 — wire the interactive Stop persistence protocol
Thread r3804255932. The durable Stop primitives (requestRunAbort 'stop',
getScheduleRunAbortState, markRunAbortPersisted) already existed but production
only used requestRunAbort(..., 'deletion'). An interactive Stop flipped the job
to `aborted` and then persisted its partial message + checkpoint inside
`beforePublish`, without ever stamping the schedule Stop or acknowledging it —
so reconciliation, the generation owner, or a concurrent schedule/account
deletion could terminalize the run and release its capacity (and erase data)
mid-write.
Wire the request -> persist -> acknowledge -> settle barrier through the
schedule runtime (the route never touches raw Mongo):
- Expose beginScheduledStop / acknowledgeScheduledStopPersistence on the service.
- The abort route stamps the Stop BEFORE signalling abortJob (a serialized
'in_progress' loser returns 409 STOP_IN_PROGRESS without a second abort),
acknowledges only after beforePublish persistence succeeds, and on a
persistence failure leaves the barrier unresolved so the run stays preserved
(client retries; stale-owner timeout is the bounded recovery). A failed abort
releases the stamp it placed so a replacement/retry is never blocked through
its predecessor.
- recordScheduleOutcome (the owner settlement path) now waits, bounded, for the
Stop acknowledgement before terminalizing; a resolved/non-stop/stale marker
proceeds immediately. The paused Stop settles only after its own ack.
Adds service-layer barrier tests and route-level ordering/persistence-failure/
in-progress tests; the data-layer serialization is already covered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave C.1 — reconcile durable trigger delivery with the reservation
Threads r3803826192 (manual limiter) and r3804255924 (PII/moderation), plus the
independently-found long-Retry-After race. fireSchedule reserves a `started` run
and a global capacity slot BEFORE the durable trigger delivery reaches the chat
route, where an interactive limiter (manual Run Now), PII, or moderation can
reject it before any generation job exists — dead-lettering the delivery while
the run sat `started` until the 30-minute orphan sweep mislabeled it interrupted.
And a valid delivery deferred by Retry-After (up to 24h) could be orphan-settled
and have its capacity released, then fire anyway.
Translate durable delivery state into the schedule outcome:
- Store the deterministic trigger deliveryKey on the ScheduleRun reservation
(computed from the envelope BEFORE enqueue, so an ambiguous commit still has it).
- Add a getTriggerDelivery engine dep (wired to the merged trigger service's
getDelivery) that reads the durable delivery by key.
- Schedule reconciliation, for a jobless `started` run: staging/pending/leased →
admission is live, never orphan; dead → record `error` from the durable
lastError and release capacity promptly (no 30-minute wait), through the
ordinary outcome/auto-disable path; succeeded or no record → the existing
legacy orphan policy (interrupted only past the cutoff); a delivery lookup
failure defers rather than orphaning a possibly-live delivery. Limiter/PII/
moderation middleware writes no schedule state.
Adds reconcile state-mapping tests (dead/pending/leased/staging/succeeded/none/
lookup-failure) and a fire test that the reservation's deliveryKey equals the
enqueued delivery's idempotency key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): Wave C.2 — durable retry/ack for terminal host lifecycle actions
Thread r3804518375. Approval expiry won the `requires_action → aborted` CAS and
then invoked the host hook best-effort: `runApprovalExpiredHandler` swallowed a
failure, and because later sweeps enumerate only `requires_action` jobs, the now-
aborted job was never offered again. In the clustered entrypoint (no schedule
reconciler) the ScheduleRun stayed `requires_action` and its retained job
persisted indefinitely.
Make the host lifecycle work durable rather than schedule-specific:
- Add a generic `terminalHostActionPending` marker, set ATOMICALLY in the same
terminal transition (ApprovalLifecycle.expireWithIdentity), only when a host
adapter is installed.
- Retain and index such jobs: both stores keep them out of terminal reaping and
expose getTerminalHostActionJobs(); Redis adds a set + extended (24h-bounded)
TTL, in-memory a bounded 24h retention so a permanently-failing hook cannot leak.
- The manager clears the marker only after the adapter acknowledges success,
fenced by generation identity (clearTerminalHostAction), so a replacement
generation can neither clear nor execute its predecessor's action.
- cleanup()/expireStaleApprovals() enumerates unacknowledged terminal host actions
across restarts and replicas and retries the idempotent hook; the relay only
re-invokes while the marker is unacknowledged, so a successful ack prevents
duplicate work. Store-won expiry marks it too, so a loser-replica relay still
crosses the hook.
- Terminal SSE notification continues regardless of host-hook outcome.
Covers in-memory behavior (retry after failure, restart/other-replica retry, ack
prevents duplicates, identity fence, terminal notification on failure, no marker
accumulation for non-scheduled jobs) and updates the Redis cluster-membership
contract test for the new index.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* style(schedules): satisfy import sorting in fire.ts and fire.spec.ts
CI "Static checks" failed on IMPORT_SORT for the two files Wave C.1 added imports
to (the AgentTriggerEnvelope type import and getAgentTriggerIdempotencyKey).
Applied scripts/sort-imports.mts to exactly those files — imports-only reordering,
no behavior change. Other files reported by a repo-wide check are pre-existing on
dev and deliberately left untouched so this PR is not widened.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): repair the deletion CLI and order restore before the fence release
Addresses three findings from the fresh Codex review.
P1 — config/delete-user.js called methods.disableUserSchedulesForDeletion, which
Wave B removed in favor of suspendUserSchedulesForDeletion. The file is
@ts-nocheck and its spec mocked the removed name, so neither typecheck nor tests
caught it; the real CLI would throw a TypeError before deleting anything and then
only unwind the fence. The CLI now uses the tokenized protocol: it mints a
suspension token, suspends with it, and restores that exact attempt's rows in its
finally block when the deletion does not commit. Its spec mocks the real methods,
so the breakage can no longer hide.
P2 — both the HTTP controller and the CLI released the user-deletion fence BEFORE
restoring schedules. That fence is what refuses new schedule writes/claims, so the
gap let an owner PATCH edit a still-suspended row and have its enabled/next-run
state overwritten by the older snapshot, and let a second deletion attempt
re-suspend under a new token — making the first restore a no-op and stranding the
disabled snapshot permanently. Restore now runs first, while writes are still
fenced.
Hardening for the same defect class across a crash: suspendUserSchedulesForDeletion
now ADOPTS an existing suspension's snapshot when re-suspending a row abandoned by
an earlier attempt, instead of re-capturing the row's current (already-suspended)
state. Without this, an attempt that died before restoring would have its
successor snapshot "disabled, no next run" and permanently strand the schedule.
Tests: CLI restore-before-fence ordering and no-restore-on-success; the same
ordering assertion on both controller post-quiesce failure paths; a data-method
regression that a second attempt adopts the abandoned snapshot and restores the
original enabled/next-run state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge dead deliveries in topology-safe maintenance
Codex finding: the `dead` delivery mapping added in Wave C.1 lives only inside
startScheduleEngine's reconciler, but the clustered entrypoint arms no engine — it
runs erasure-only maintenance. A delivery queued before a restart into clustered
mode and then rejected before generation creation (interactive limiter, PII,
moderation) dead-letters while its ScheduleRun stays `started`, holding a global
capacity slot indefinitely for an ordinary non-deleting schedule.
Add a dead-delivery convergence pass to the erasure sweep, so every topology that
runs schedule maintenance settles it. The pass is POSITIVE-EVIDENCE-ONLY and is
therefore safe where absence-based reconciliation is not: a `dead` delivery is
durable shared state proving no generation owns the reservation. It settles only
when the job is confirmed absent or identity-mismatched (an identity-matched job
still owns the run), defers on an unknown job lookup, on an in-flight abort, and
on an in-flight resume hand-off, ignores legacy reservations with no deliveryKey,
and applies a short grace so an accepted delivery still creating its generation is
never settled mid-handoff. Auto-disable policy is deliberately left to the armed
engine; this path records the failure and frees the slot.
Deliberately does NOT touch api/server/experimental.js — the clustered entrypoint
already starts this sweep, so the convergence arrives through the existing
initializer and the shared-file footprint stays as-is.
Tests: settles a dead delivery as error under an explicitly UNSAFE topology,
leaves live deliveries alone, never settles under an identity-matched running
generation (delivery is not even consulted), defers an in-flight abort, and
ignores a reservation with no deliveryKey.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): refresh host-action retention on each retry attempt
Codex finding: unacknowledged terminal host-action evidence was capped at the
24h pause TTL, so a host dependency (Mongo) unreachable for longer than that let
the Redis key — and its pending marker — expire with no generation-fenced
acknowledgement, stranding the ScheduleRun where no reconciler is armed.
Measure retention from the LAST retry rather than from the terminal transition:
enumerating a pending host action IS the retry attempt, so both stores refresh
its retention as they hand it to the hook (Redis re-EXPIREs the job key; the
in-memory store stamps terminalHostActionRefreshedAt and bounds from it). Evidence
therefore survives as long as some replica is still actively retrying, while a
deployment that stops sweeping entirely still lets it age out — so this does not
reintroduce the unbounded leak the cap existed to prevent.
Test: after a failed hook, a later cleanup pass keeps the marker pending and moves
its retention basis forward.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): defer settlement when the Stop barrier times out
Codex finding: waitForStopPersistence returned after its 5s poll budget even when
the Stop was still fresh and unacknowledged, and recordScheduleOutcome then went
straight on to terminalize the run — releasing its capacity, deletion, and erasure
barriers while beforePublish may still have been writing. Slow checkpoint cleanup
is indistinguishable from a dead route on that signal, so the timeout was being
treated as if the barrier had been satisfied.
The poll budget now means "undecided", not "clear". On timeout with a fresh,
unacknowledged Stop the barrier DEFERS: recordScheduleOutcome returns false
without recording, leaving the run active/preserved. Settlement then happens
either when the route acknowledges, or once the existing stale-owner cutoff
(ABORT_OWNER_PRESUMED_ALIVE_MS) authorizes a later attempt — which the loop
already treats as clear-to-settle. Callers with durable retry (the approval-expiry
host action, reconciliation) re-drive it, so a deferral converges rather than
stranding the run.
Test: a fresh Stop that never acknowledges within the budget reports not-settled
and records no outcome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): require definite delivery failure, extract its message, converge deferred Stops
Third Codex round. Three findings in code from this closeout, plus one pre-existing
P1 that is a one-line operator-safety fix.
P1 — dead-delivery settlement demanded too little evidence. `dead` does not prove a
request was rejected: the trigger host marks response timeouts and invalid success
responses `certainty: 'ambiguous'`, and the engine dead-letters those once retries
are exhausted. The erasure sweep treated every dead letter as positive evidence, so
an ambiguous one sitting over a generation a peer had accepted could terminalize the
run and release its capacity mid-flight. It now settles only on a DEFINITE rejection,
unless job absence is deployment-authoritative (safe topology), where the
confirmed-absent job is itself the evidence.
P1 — `lastError` is an `AgentTriggerDeliveryFailure` object, not a string. A
duplicated local interface declared it `string` (against CLAUDE.md's no-duplicate-
types rule), so both the sweep and the engine reconciler passed the object into the
String-typed run/schedule `error` fields; Mongoose would reject the cast, the per-row
catch would swallow it, and the run would keep its global capacity slot. The dep type
now reuses the canonical `AgentTriggerDeliveryFailure` and both call sites pass
`.message`. Re-typing immediately surfaced a stale test that had asserted a string.
P1 (pre-existing) — the base-config global stop is honored in `getLimits` via
`isRuntimeDisabled` rather than a literal `=== false`. The stop has two shapes, and
deepMerge turns base `{ use: false }` plus a principal override of `true` into
`{ use: true }`, so the literal check reported the feature enabled and Run Now
dispatched straight through fireSchedule, bypassing the operator's emergency stop.
Now the same predicate the engine gate already uses.
P2 — a Stop whose settlement DEFERRED past the poll budget had no convergence path
where no reconciler is armed. `acknowledgeScheduledStopPersistence` now optionally
re-drives the terminal outcome once the barrier clears; `recordRunOutcome` is
match-guarded and idempotent, so an owner that already settled makes it a no-op. The
abort route passes it for a running generation; a paused job still settles explicitly.
Tests: ambiguous dead letters refused under unsafe topology but settled when absence
is authoritative, definite rejections settled either way, the failure message carried
through, and the abort route's re-drive present for running / absent for paused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge terminal runs in clustered workers, retry suspension restore
Fourth Codex round on
|
||
|
Danny Avila
|
259f1e0c32
|
🛰️ feat: Route Live Subagent Controls Across Replicas (#14971)
* feat: route live subagent controls across replicas * fix: initialize task routing in cluster workers * fix: harden cross-replica task routing * fix: expire routed task owners independently * fix: close cross-replica routing edge cases * fix: bound owner refresh and close routed cancellation gaps Refresh owned task registrations in bounded parallel batches so a full heartbeat pass stays well inside the 30-second directory lease instead of serializing one Redis EVAL per registration. Route conversation-deletion cancellation through a dedicated owner-side scope operation. The owner applies the deletion predicate to its complete local task set, so a scope holding more children than the model-facing list cap no longer leaves live executors running after their parent is removed. Key a consumed claim's retained response by its operation rather than by one caller's correlation id, so a later poll recovers a terminal result whose responses were all lost. Live claim statuses stay uncached so a poll always observes the task's current state. Type the model-facing `maxLength` bounds with a narrow local string schema; the SDK's JsonSchemaType does not declare the keyword, and the runtime checks continue to enforce the same limits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: retain claimed results apart from control replays A consumed claim is the only routed response whose loss destroys data, so it no longer shares one bounded cache with control replays that unrelated command traffic can evict. Claims are retained under their own budget, and the requester acknowledges a result it received so the owner releases the copy immediately instead of holding it for the full replay window. Resolve the post-delete cancellation pass from durable leases. The deleted conversations cannot be read back, so re-reading each one only scaled the cascade while probing the owner directory once per removed id; one lease read now resolves every live child address instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: never consume a result the owner cannot replay Retention for consumed claims is bounded, so a burst of undelivered results could evict an earlier one and lose it for good. The owner now admits a claim only while it can retain a worst-case result, and refuses the routed claim otherwise instead of consuming it, leaving the result on the task for a later poll. Retained claims are never displaced; control replays keep evicting. Key a control replay by the command itself rather than by one caller's correlation id. The transport's own retry reuses a single envelope, but a caller that saw the owner as unavailable reissues the command under a new id, which steered, queued, or interrupted the child a second time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: own a claimed result until it is acknowledged A consumed terminal result is task-owned state, not a cache entry. It now carries no expiry at all: the owner holds it until a caller acknowledges receipt, and only then is it released. Retention stays bounded by the existing admission gate, which refuses a claim the owner could not keep rather than consuming a result it might drop. Identify a control by the caller's invocation instead of by its content. The tool mints one id per invocation and routing carries it, so a routed retransmission of that invocation replays the owner's result while two deliberate identical commands arrive under distinct ids and both apply. Content-derived identity could not tell those apart and would have answered the second from a stale snapshot. Wait for the dpkg frontend lock in the best-effort Playwright font step. Its timeout kills npx while the apt-get it spawned keeps the lock, which then failed the fatal Redis install and ended the MCP replica jobs before any test ran (#14983). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: treat acknowledgement as part of delivering a result Publishing an acknowledgement once and ignoring the outcome meant a result could be reported as delivered while the owner never learned it could let go, and since that retention neither expires nor evicts, enough lost acknowledgements would fill it and refuse every later remote claim. An acknowledgement is now confirmed: publishing to zero subscribers is not success, it retries inside the ordinary request window, and a claim whose acknowledgement cannot be confirmed reports the retryable unavailable path instead of handing back a result the owner still holds. A later poll recovers that result and acknowledges it, and releasing is idempotent. Owner registration also outlives the task while a result is unacknowledged, so the retained result cannot become unreachable. Take the control invocation identity from the provider's tool-call id rather than minting one per execution, so replaying the same tool call stays idempotent while two distinct calls with identical payloads both apply. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * style: sort the widened node:crypto import Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: own control invocations and cancellation plans at the task seam Applies one logical control exactly once for its owning task rather than in the transport, so a local caller and a routed caller of the same invocation agree, and reusing an invocation id for different content is refused instead of silently applied. Invocation identity now comes from the run, agent, and provider tool-call id hashed to a bounded 32 characters, so a repeated `call_0` never bleeds across tasks and no id can overrun the routed bound. Cancellation for conversation deletion is now resolved into a plan while those rows are still readable, then replayed against the owner directory after the cascade is deleted. Owner registration is awaited before any provider work, so a child that cannot be addressed never starts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * style: separate the control invocation map from the next member Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: close subagent deletion, claim, and control invocation gaps Bulk conversation deletion now runs behind a durable owner admission fence. Draining alone could not close the race: a child admitted on another replica after the drain read its leases would start provider work against a parent about to disappear. The fence is written before any lease is read and each child revalidates it after its own lease is written, so one of the two always observes the other. It expires on its own, so a process lost mid-deletion cannot leave an account unable to run subagents. A terminal child result is no longer kept alive in the owning replica's memory until someone acknowledges it. Collection is recorded durably on the child's own message against the polling invocation, so the poll whose response was lost recovers its own result while a different invocation is told the result was already collected. Owner-side retention returns to an ordinary bounded cache that expires, which is what abandoned polls needed: they can no longer occupy claim capacity until the process restarts. The deletion drain now cancels each task under one invocation held for the whole drain, stops re-sending once the owner answers, and retries only deliveries it could not confirm. A routed control replay also validates the command fingerprint, so one invocation id carrying different content reaches the owner to be refused instead of collecting the earlier command's success. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: assert the drain's calls before restoring its spies Restoring a spy also clears its recorded calls, so the drain assertions ran against an emptied mock. Formats the durable claim method tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: close the follow-on gaps in the deletion fence and result claim The admission fence now carries an ownership token, so an overlapping deletion's fence is never lifted by the one that finishes first, and both fence writes invalidate the cached auth user document. It also covers the other bulk-delete path: `DELETE /` with no conversation filter removes every conversation, so it runs behind the same fence rather than a bare drain. The durable record now decides who holds a one-shot result. An owner replaying a retained response could hand the same terminal claim to a second invocation; that invocation is told the result was already collected, while the one that consumed it still recovers its own. A task with no durable record to arbitrate keeps whatever the owner answered. Drain cancellation treats `not_found` as unconfirmed: a missing registration while the durable lease is still live means the child may be running, so the command is retried under its invocation once the owner republishes itself. Control fingerprints are hashed, so retaining one per invocation costs a fixed few bytes instead of a bounded message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: hold every deletion fence and keep live idempotency records An owner now holds one admission fence per concurrent bulk deletion instead of one at a time, so admission reopens only when the last deletion finishes regardless of completion order. Expired fences are pruned as new ones arrive and the set is bounded, so an abandoned fence cannot accumulate or lock an account out. A failed durable claim write is no longer read as an absent record. Handing a terminal result over without recording its claimant would let another invocation collect the same one-shot output once the database recovered, so the collection reports the retryable unavailable path and leaves the result for a later poll. Control invocation records now evict tasks the store no longer holds before live ones, over a bounded scan. Dropping a live task's record would let a caller retry apply its queue, steer, or interrupt a second time once the transport replay had also expired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: keep the deletion fence portable and never drop a live record The admission fence is written with plain update operators again. DocumentDB rejects pipeline-form updates, and this runs before any deletion, so the pipeline form would have failed both bulk-delete endpoints outright on a supported database target. An excess deletion is now refused rather than silently displacing the oldest active fence, which would have reopened admission for a deletion still running. Expired fences are pruned before the cap is tested, so only genuinely concurrent deletions count against it. Control invocation records now sweep every settled task's entry when the window fills, and a window of entirely live records refuses the new control before touching the child instead of evicting one. Applying a command with no room to record it would let the caller's own retry apply it twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: hold the fence, bound recovered results, and expire stale commands The admission fence is renewed for as long as its deletion runs, so a very large account or a stalled database cannot let it lapse while conversations are still being removed. Only the deletion's own fence is renewed, and the renewal stops with the operation. Cancellation now covers every conversation the cascade removed, not only the ones a plan named: a grandchild lives in its own parent's scope, which a plan naming the deleted root never reaches. A routed request carries the deadline its caller waits for, and an owner drops one that arrives past it. A publisher disconnected mid-request queues the envelope offline and delivers it after the caller was told the owner was unavailable, which would otherwise steer a child the caller believes untouched. A result recovered from its durable child message is bounded like a routed one. The message keeps the child's untruncated output, so recovery could otherwise return far more than the routed result limit allows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: size the fence window so a renewal can be observed The renewal test set a 30ms drain timeout but the five-minute grace window dominates it, so the interval was 100 seconds and no renewal could fire inside the test's deletion. The grace window is an option now, matching the store's other timings, and the test sizes the window to 90ms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: wire the durable claim method and close the fence follow-ons The production store never received `claimSubagentTaskResult`, so every terminal result would have surfaced as unavailable once a task settled. The host wires that object from JavaScript, where the factory's parameter type checks nothing, so the factory now refuses a store missing any method it calls rather than failing at the first claim. The routing transport takes a dedicated publisher with the offline queue disabled. The shared client held commands issued during a disconnect and delivered them after the caller had given up, which the request deadline narrowed but could not close inside the clock-skew allowance. Fence renewal invalidates the cached auth document like the fence and release paths, and a renewal reporting its entry gone re-takes the fence instead of letting the deletion run on unfenced. The post-delete cancellation retries a transiently unreachable owner: the conversations are already gone, so it is the only pass that can still stop a late-admitted child. A replaced replay entry no longer leaves its bytes counted, which would have inflated the cache's total until unrelated responses were evicted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: wait on observed lease renewal instead of a fixed delay The shared-lease renewal test held a 60ms lease and slept 100ms before asserting an overlapping worker was refused, so a loaded runner that starved the 10ms heartbeat past the TTL let the lease lapse and the second worker run. Spy on acquisition and renewal, then wait until a renewal succeeds past the acquired lease's own deadline — direct evidence the heartbeat carried it past expiry, with no timing assumption — and give the lease enough headroom that a stalled timer no longer decides the outcome. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): close the routing, fence, and cache gaps found in review Five separate seams, each with its own failure: `Cluster.duplicate` reads its first argument as a startup-node list and its second as the overrides, unlike `Redis.duplicate`, so the publisher's `enableOfflineQueue: false` was silently dropped under `USE_REDIS_CLUSTER` and a command issued mid-disconnect could still reach a child after its caller was told `unavailable`. Route both through `duplicateIoRedisClient`. The control window's capacity refusal ran before the store knew whether it owned the task, so unrelated local load could veto a cancellation bound for another replica. Establish that the task is local first and leave a remote one to its owner's window. `clearInterval` stops only future fence renewals. One already waiting on the database could resolve after the release, read its own lifted fence as expiry, and write a replacement that nothing remained to lift — closing subagent admission for the account until it aged out. Track the in-flight renewal, refuse overlapping passes, and await it before releasing. Every owner bounds its own task list, but the aggregation appended each batch whole, so the model-facing list grew with the number of replicas holding the scope. Cap the merged list while still reading every reply for the stale-registration sweep. The admission-fence prune commits independently of the fence that follows it, so a refused or failed push left the cached auth document describing entries the collection no longer held. Invalidate whichever way the second write goes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): cap the merged task list the poll tool actually reads Each owner bounds its own reply and the remote aggregation bounds their sum, but `listTasks` merged that bounded remote list with however many children this replica owns and returned it whole. `check_background_task` could therefore still receive roughly twice the advertised cap. Bound the deduplicated, sorted result and export the cap so both seams share one number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: admit every task the merged-list cap test starts The base store admits ten concurrent runs per scope by default, so starting 150 at once left most refused for capacity and the assertion never reached the merge it was written to check. Raise the cap for this store only; admission is a different invariant with its own tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): let a deletion notice its admission fence lapsing Renewal failures were logged and swallowed, so a run of rejected writes let the last confirmed `fencedUntil` pass while the deletion carried on believing admission was still closed — long enough for another replica to admit a child against conversations about to be removed. Track the deadline only a confirmed write advances, and check it after the drain, before anything is deleted: nothing has been removed at that point, so the operation fails closed and the caller retries once the fence can be held. A lapse detected after the rows are gone is logged instead, since reporting failure there would invite a retry against conversations that no longer exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: raise both concurrency caps the merged-list test trips Raising the per-scope limit left the store-wide `maxRunningTotal` at its default hundred, so fifty of the hundred and fifty starts were still refused. Verified against the base store directly this time: with only the per-scope cap raised it admits a hundred, and with both raised it admits all hundred and fifty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): close the fence renewal gap and keep running tasks listed A renewal that started before its deadline but landed after it was still credited with extending the fence from its own start time, so a window in which admission stood open was papered over: a child could take a lease the drain had already read past and the deletion would proceed without cancelling it. The deadline now only advances when the write lands while the previous one still holds; anything later records a lapse the fence cannot be restored backwards over. The model-facing cap sorted oldest-first and sliced, which dropped the newest tasks — including children that had only just started running, and which the poll tool offers no other way to discover. Bound by status instead: running children first, then the most recent settled results. Both caps share one helper, and the routed aggregation now bounds after its loop so the choice is made across every owner's reply rather than by whichever answered first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): finish the cap and the fence at the seams they still missed The status-aware cap only reached the requester: an owner's own reply still sliced positionally, so a replica holding more than the cap dropped its running children before the requester could bound anything. Both sides now share `boundedTaskList`. A fence that lapsed during the deletion itself was only logged. The rows are gone by then, so failing is still wrong, but the child another replica admitted while the fence was down is not: the fence is retaken and the drain repeated to cancel it. A child's lease renewal had the same retroactive hole the admission fence had — Mongo filters on the `now` captured before the call, so a write landing after the lease expired still moves the row forward, while an owner drain reading active leases in that gap saw the thread as free. The lease now carries its own deadline and a late renewal stops the executor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: cover the lease lapse and the post-deletion re-drain The owner-side cap shipped with a regression test; these two did not. One drives a lease renewal that succeeds only after the lease it was extending had expired and asserts the executor stops; the other lets the fence lapse during the deletion itself and asserts a second drain runs while the request still reports success. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): close live-task lifecycle gaps * test(redis): exercise cluster node discovery * fix(test): type cluster discovery seam * fix(ci): wait for orphaned apt processes * fix(ci): reserve time for apt drain * fix(ci): skip optional fonts in MCP jobs * fix(agents): recover tasks after owner loss * fix(agents): preserve local task discovery * fix(agents): initialize fail-fast cluster publisher * style(agents): sort routing test imports --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Ravi Kumar L
|
006e421cd2
|
💡 feat: add DB-backed admin insights (#14898)
* feat: add Mongo-backed admin insights * feat: gate insights with environment variable * fix: tighten insights access and activity metrics * fix: preserve insights date selections * perf: parallelize insights search aggregation * test: wait for MCP conflict recovery * test: satisfy strict MCP recovery typing * fix: disable insights pagination while loading * fix: localize insights range shortcuts * fix: bound insights search input |
||
|
Danny Avila
|
547bd8c4bf
|
🧵 feat: Persist View-Only Subagent Threads (#14957) | ||
|
Marco Beretta
|
fdc9c77f6e
|
🗄️ feat: Archive All Chats From Data Controls (#14885)
* feat: archive all chats from data controls
Adds an "Archive all chats" row under Data controls > Your data, next to
Shared links, with a confirmation dialog. It calls a new
POST /api/convos/archive/all endpoint backed by archiveAllConvos, which
archives every conversation currently visible to the user in a single
updateMany and refreshes the stats of every chat project the archived
conversations belonged to.
Temporary and retention-expired conversations are skipped: they are
already hidden from the chat list, so archiving them would only surface
them in the archived view. The update runs with timestamps disabled so
each conversation keeps its own updatedAt and the archived list stays
sorted by real activity.
Archiving a conversation now also drops the new-chat message cache alias
for it. A chat's first turn writes the same message array under both the
conversation key and the new-chat key, so without this the messages of a
just-archived chat kept rendering on the new chat screen until a reload.
Deleting already handled this; archiving did not.
* fix: keep archive-all state consistent
* fix: drop stale detail caches after bulk archive
* fix: harden archive-all request handling
* fix: reconcile archive batch failures
* Fix project stats refresh races and archive route boundary
* Fix archive-all review findings
* Fix archive scan index and partial-batch stats refresh
Reconcile project stats for already-committed archive batches when a later
batch fails, and index the archive scan as { user, _id } so non-tenant
pagination can use _id order.
* Fix Recoil reset after a partial archive-all failure
Refetch the submitted conversation on error and start a new chat only
when that conversation is still active and already archived.
* Fix archive recovery from resetting a newly opened chat
Re-read the active Recoil conversation after the archive-state lookup
resolves, so a slow getConversationById cannot start a new chat if the
user already opened another conversation.
* Fix project-stat reconciliation after archive races
Keep retrying optimistic project-stat writes instead of returning a
stale document after three lost CAS attempts, and retry destination
project discovery after a transient distinct failure.
* Fix archive reset and project-count increment races
Leave already-archived chats open after archive-all, recount new
project conversations instead of incrementing, and skip a delayed
increment when a concurrent refresh already recorded that chat.
* Recover destination projects after discovery retries exhaust
Keep committed conversation IDs when post-archive distinct fails, then
rediscover those projects in finally so a moved conversation's
destination still gets reconciled after the error is rethrown.
* Fix archive-all recovery batching and remount pending state
Recover destination projects in 500-id chunks so the final lookup
cannot exceed Mongo's command size, and share archive-all pending
state through a mutation key so Settings remounts stay disabled.
* Stamp bulk-archived chats and refresh the pinned cache
Bulk archive wrote only isArchived, so the archived table dated every
swept chat by createdAt and the default archivedAt sort dropped the whole
run into the legacy null group. Stamp one timestamp for the sweep; the
filter only matches unarchived chats, so an existing stamp cannot move,
and timestamps: false still preserves each updatedAt.
The pinned section fetches on its own key with a five-minute stale time,
so an archived pin kept rendering in the sidebar until that expired.
Invalidate it alongside the other lists on both success and failure.
Also drop the async from the failing-batch updateMany mock: its
Promise<never> is not assignable to the Query return type, while a plain
synchronous throw types as never.
* Bound archive recovery state with the sweep marker
Recovery held every committed conversation id for the life of the
request so the finally block could re-run project discovery after an
in-loop distinct gave up. Slicing that array into 500-id queries capped
the BSON command size but not the heap, so a very large history could
exhaust a worker mid-archive.
The archivedAt stamp already identifies exactly what this call
committed, so recovery is now one distinct scoped to it. That filter is
a prefix of the existing user/isArchived/archivedAt index, and the two
discovery call sites collapse into one filter-taking helper.
* Reconcile archive stats when a write outcome is unknown
A batch that commits but whose result never returns, a stepdown or a
connection drop between commit and acknowledgement, left archivedCount
at zero, so the finally block skipped both marker recovery and the stats
refresh. The chats were archived, so no retry could find them again: the
sweep filter no longer matches them and their projects kept stale
counts.
Both now key off the write attempt rather than the returned count.
Nothing else needs to change, because the marker is stamped by the same
write whose result went missing.
* Retry dropped project refreshes and guard stale pointer writes
Two ways a project could keep stale stats after archive-all.
A refresh that rejected was logged and dropped for good. Its chats are
archived, so no retry of archive-all can find them again to recompute
against, and the likeliest rejection is the recoverable one:
refreshChatProjectStatsForUser gives up when the project changed under
every compare-and-set attempt. Failures are now collected and replayed
once the rest of the run has stopped competing with them.
A save already in flight could also undo the sweep. Its conversation
document still said visible, so its tail took the pointer branch and
wrote lastConversationId back to a chat the sweep had just archived,
leaving the project advertising activity on a chat the workspace hides.
The pointer write now confirms the chat is still visible first, and
recomputes the project when it is not.
* Verify project pointers after the write, not before
Checking visibility before the pointer write only moved the race earlier:
a sweep landing between the check and the update still archived the chat
and cleared the project, and the write then restored it as
lastConversationId.
The check now runs after the write and repairs instead of preventing. A
sweep that lands earlier is caught here; one that lands later refreshes
the project itself, and refreshChatProjectStatsForUser compare-and-sets,
so it cannot commit a count it read before this write. Same single
indexed read as the check it replaces.
|
||
|
Danny Avila
|
1b7e2a4e6a
|
⚡ perf: Optimize First Load of Large Conversations (#14901)
* ⚡ perf: Index the Conversation Fetch and Trim the Client Message Projection * ⚡ perf: Memoize the Message Tree per Cache Write * ⚡ perf: Serve Message Reads via the Trimmed Projection and an Ownership Probe * ⚡ perf: Defer Collapsed Disclosure Bodies Until First Expansion * ⚡ perf: Progressively Mount Long Threads from the Scroll Anchor * 🩹 fix: Address Codex Findings on Retention, Anchoring, and Cache Bounds * 🩹 fix: Poll the Oversized Export Precondition Through the Progressive Mount * 🩹 fix: Keep Video Results in the Client Message Projection |
||
|
Danny Avila
|
7d850c308a
|
🧠 feat: Add Live Reasoning Labels (#14893)
* feat: add live reasoning labels * fix: Stabilize reasoning label checks * fix: Address reasoning label review findings * chore: Bump Agents SDK for reasoning labels * fix: Reset reused reasoning step evidence * fix: Reconcile cleared reasoning labels * fix: Fence reasoning label resets * fix: Reset reasoning ownership before gap labels * fix: Preserve THINK type through label reset * test: Expect run-global reasoning revision |
||
Marco Beretta
|
832bac39ad
|
🗄️ feat: Record When a Conversation Was Archived (#14863)
* feat: record when a conversation was archived The archived chats dialog has a "Date Archived" column that was bound to createdAt, so it showed when the chat was created rather than when it was filed away. Nothing recorded the latter. Conversations now carry archivedAt, set on archive and cleared on unarchive, and the column reads it. Chats archived before the field existed have no stamp and fall back to createdAt, which is exactly what that column already showed for them. The archive view sorts on the new field. archivedAt is absent on every previously archived chat, so the missing-value group is the common case here rather than an edge case: the cursor's null handling, written for titles, now covers both, and an absent stamp survives the cursor as null instead of collapsing to the epoch and replaying the whole archive. * fix: address review findings on the archived-at stamp - Protect `archivedAt` from saveMessageToDatabase's unset sweep. Any persisted field missing from endpointOptions is unset, so sending a message in an archived chat cleared the stamp while leaving isArchived true, silently dropping it into the legacy fallback group. - Order the legacy group by the createdAt the dialog displays rather than by last activity. The cursor's secondary key is now chosen per sort field, so the fallback the cell renders and the order the server returns cannot disagree. - Put that secondary key in the archive index too, so paging the legacy group does not fall back to a blocking sort. * fix: keep archivedAt on a redundant archive request Opening an archived chat and hitting the archive shortcut, or retrying the POST, sent isArchived: true again and replaced Date Archived with now. saveConvo now stamps only on the unarchived-to-archived transition and still clears the field on unarchive. * fix: make archive timestamp updates atomic * test: type the archive race spy against the driver signature * fix: archive without an aggregation-pipeline update DocumentDB documents no support for pipeline-form updates on any engine version, and the repository's compatibility assessment records that a prior P0 rewrote the three that existed. Stamping archivedAt through a $cond pipeline reintroduced one, which would have sent every archive and unarchive to the route's 500 handler on a supported 5.0 deployment. The conditional stamp is now a compare-and-set on isArchived, which keeps the transition atomic without a pipeline: only the write that finds the chat unarchived stamps it, so a duplicate or retried archive leaves the original date alone and an unarchive that lands first is re-stamped. Schema defaults and createdAt-on-insert go back to mongoose's own setDefaultsOnInsert and $setOnInsert, and tenantId is once again stripped by the tenant-isolation plugin rather than by hand. * fix: do not report a racing archive as a missing chat Both conditional writes of the compare-and-set miss when the archive flag flips between them: the chat was already archived when the transition write ran and unarchived again before the already-archived write. saveConvo returned null for a conversation that plainly exists, so POST /api/convos/archive answered 404. Confirm the conversation is really gone before accepting that result, and retry the pair when it is not. An unknown id still costs one existence read and falls straight through to the 404. * fix: resolve a fully contended archive to the chat's real state Alternating archive and unarchive requests can split every attempt of the compare-and-set: each transition write sees the chat archived and each already-archived write sees it unarchived. Exhausting the retries therefore proved nothing about whether the conversation exists, and the no-upsert archive route turned a lost race back into a 404. Read the conversation once more when the retries run out and answer with its actual current state instead. |
||
|
Marco Beretta
|
7857a99d63
|
⚡ perf: Flip the Pinned Flag Without a Full Conversation Save (#14862)
Pinning routed through saveConvo, which reads every message id for the conversation and writes the whole array back just to set one boolean, and can trigger a project-stats recompute on top. None of that applies to a pin: it moves no chat between projects, changes nothing the project workspace hides, and opens no retention window. A dedicated setConvoPinned does the single findOneAndUpdate instead. Measured against an in-memory MongoDB with the real message methods wired in, on a 120-message chat: two driver commands and 3706 bytes before, one command and 245 bytes after. The write scales with the message count, so the gap widens on longer chats. Archiving keeps using saveConvo, which it needs for exactly the project stats and retention work a pin does not. |
||
|
Marco Beretta
|
5fc05ac037
|
🕰️ fix: Stop Pinning and Archiving From Counting as Chat Activity (#14861)
* fix: stop pinning and archiving from counting as chat activity Both routes went through saveConvo, which lets mongoose stamp updatedAt. The sidebar orders chats by that field, so pinning hoisted an untouched chat to the top of Today, and unarchiving a year-old chat dropped it there too instead of back into its own date group. saveConvo now takes preserveUpdatedAt, and both routes pass it. They also pass noUpsert: with timestamps suppressed an upsert would insert a conversation carrying neither createdAt nor updatedAt, so an unknown conversation id is now a 404 rather than a silently created stub. * test: pin a project's activity pointer against metadata-only saves Review raised that preserving updatedAt could drag a project's lastConversationAt back to the pinned chat's older timestamp, since the incremental path $sets it outright. That path is not reachable here: a pin carries no chatProjectId, so previousChatProjectId stays null while the conversation has a real one, projectMembershipChanged is therefore true, and saveConvo takes the full recompute branch instead. This test holds that in place, with a newer sibling conversation in the project so a regression to the incremental path would fail it. * fix: keep updatedAt through the retention backfill Under RetentionMode.ALL a legacy chat with no stored isTemporary gets a second write after the main update, and that one still had mongoose timestamps enabled. The first archive of such a chat therefore bumped updatedAt anyway and landed in Today, defeating preserveUpdatedAt on exactly the old conversations it was meant to protect. |
||
|
Marco Beretta
|
8a946290f6
|
📌 fix: Fetch Pinned Chats Independently of the Chats List (#14860)
* feat: give the pinned chats section its own fetch The sidebar's pinned section filtered pinned chats out of the paginated chats list, which only holds the 25 most recently updated conversations. Once 25 newer chats existed, a reload hid the pin until the list was scrolled far enough to fetch the page it lived on. Pins are now fetched directly via GET /api/convos?pinned=true behind a dedicated query, so every pin paints with the sidebar regardless of where it falls in the chats list. Pin and unpin invalidate that query, and the shared conversation cache helpers keep it in step so a rename, delete or archive is reflected without waiting for a refetch. Pins stay out of the date groups, which groupConversationsByDate already handled. * fix: address review findings on the pinned chats section - Drain the cursor rather than capping the pinned request at 100. Since pins are kept out of the chats date groups, anything this query dropped was invisible in the sidebar entirely, not merely further down a list. - Apply the active bookmark filter to the pinned request and key its cache by it, matching the chats list beside it. - Move a pin to the top of the section when the caller asks for it, so a pin that just received a message leads the way it does in the chats list instead of waiting for a refetch. - Invalidate the pinned list when a conversation is unarchived, since archiving removes it from that cache and nothing put it back. - Index the pinned lookup: it filters on user + pinned and sorts by updatedAt, which no existing compound index covered. - Protect `pinned` from saveMessageToDatabase's unset sweep. Any persisted field missing from endpointOptions is unset, so sending a message in a pinned chat silently unpinned it. * fix: keep the pinned cache reconciled across the other convo mutations Second review pass on the independent pinned query. - Fall back to the pins already loaded in the chats pages when the dedicated request fails. Pins are stripped from the date groups, so an error otherwise emptied the section and hid them everywhere. - Restore default focus and reconnect refetching, matching the conversations query. A pin changed in another tab is only reconciled by a refetch, since that tab's mutation never touched this cache. - Invalidate the pinned list from the mutations that can produce or alter a pinned chat without going through pin itself: duplicate, fork, import, project assignment, and shared-link deletion. * fix: invalidate pins on tag and project-deletion changes Third review pass, same class as the last: the pinned query is keyed by the active bookmark filter, so changing a chat's tags can move it in or out of that filtered set, and deleting a project unsets chatProjectId on its chats, pinned ones included. * fix: cancel in-flight pinned fetches when deleting a conversation Deletion cancelled the regular and archived queries but not the pinned one, so a pinned GET issued before the delete could resolve after the row was stripped and write the deleted conversation back, leaving a row that navigates to a missing chat. Restoring default focus and reconnect refetching in the previous commit made those in-flight fetches more likely, so this widened rather than appeared. Cancelled on mutate, and invalidated on success since cancelling a race is best effort. * test: make the SSE query-cache mock key-aware The conversation cache helpers now run a second, pinned-keyed findAll pass. This mock ignored its key argument and always returned an allConversations entry, so those pinned writes were attributed to allConversations and the write-count assertions saw three instead of two. * fix: keep pins in sync through upsert and pin-only pages Root-level SSE updates and resumable settlement call upsert rather than update, so the independently cached pinned row never moved or refreshed. An all-pin first page also left the chats virtual list empty, so onRowsRendered never asked for the next cursor. * fix: keep pins current through SSE recovery and project delete Resumable SSE reconciliation invalidated conversation and allConversations only, so an independently cached pin kept stale title and order. Deleting a project-backed pin that lived only in that cache also skipped the project query, because the mutation never read chatProjectId there. * fix: keep pins current after bookmark edits and failed pages Renaming or deleting a bookmark rewrote tags on conversations but left the tag-keyed pinned cache pointing at the old filter. An all-pin page whose next fetch failed also retried forever because the empty-list effect had no memory of the attempt. Unpinning a pin that only lived in the dedicated cache removed it from Pinned without inserting it into Chats, and later cursor pages cannot recover a row whose updatedAt just jumped ahead of the current cursor. * fix: keep pins visible after a failed refetch A failed pinned refetch left React Query holding the previous list, so the nullish fallback never ran and a newly pinned chat vanished from both sections. Unpinning an older pin also inserted it into every cached chats variant, including bookmark and search results it would not match. Drop the checked-in agent task prompt. * test: type the pinned conversation fixtures correctly The delete mutation takes a plain string conversationId, but reading it back off a TConversation fixture widens it to string | null. Hoist the id into its own constant so the call site passes the real string. Type the tag fixture as TConversationTag so it carries the required _id and user fields the mocked resolved value expects. * style: sort the sidebar imports to the repo order The new pinned-section imports went in out of the longest-to-shortest order the import sorter enforces. * fix: keep drained pins and empty chat caches from breaking the sidebar A pinned page failing partway through the drain rejected the whole query, so every pin already fetched was discarded and the section fell back to whatever the chats cache happened to hold. Publish the accumulated pins before rethrowing so the retry renders against the partial set. Unpinning a chat that only lives in the pinned cache reinserted it into the chats list by spreading the first page, which is absent once removal has filtered out the last loaded row. Rebuild that page instead, matching the upsert path. * fix: order fallback pins by their timestamp The merge kept dedicated rows in Map insertion order and appended the pins recovered from the chats cache after them. A chat pinned while the dedicated refetch is failing is the newest pin, so the server would return it first, yet it landed last and could sit below the section's visible 30vh. Sort the merged set newest-first so a fallback row takes the place the server would give it. * fix: keep the shared badge and the move-to-top order on pins The pin response has no isShared: the flag is derived per list request by attachSharedFlags, which only runs for the list queries. Reinserting an unpinned chat into Chats therefore dropped its shared-link badge, because unlike an in-place update there is no existing row to carry the flag over from. Read it off the cached pin before the update removes that row. The chats cache refreshes updatedAt when it moves a conversation to the top, but the pinned cache only reordered, leaving the previous turn's timestamp on the row. Sorting the section newest-first then put it straight back. Refresh the timestamp there too, so the move survives the sort and both caches agree. |
||
|
Marco Beretta
|
d920328bfa
|
💬 style: Unify Message Row Layout and Edit Surfaces (#14770)
* style: Unify message row layout and edit surfaces Route chat, share, and search messages through a shared MessageRow so user turns render as right-aligned bubbles and assistant turns keep a visible identity column. Replace per-part text editors with one edit surface that keeps tools, errors, and artifacts visible. Preserve non-text fields when saving content parts, copy the full serialized message, and hide hover actions that do not apply during streaming or errors. * style: Align edit footer and lighten editor field in dark mode Drop the divider above the user edit footer so both edit surfaces share the same footer treatment. Move the editor fields to surface-tertiary-alt. Light mode is unchanged at #fff, while dark mode lifts from #0d0d0d to #2f2f2f so the field sits above the #212121 panel instead of sinking into near-black. * style: Drop focus border and ring from message editors The editor fields changed border color and added a ring on focus. Keep the border static and rely on the app-level focus handling instead. * fix: Keep a triggered message action visible when the row is not hovered Hover actions fade out on non-last rows, and mobile.css only restored display and visibility for an active button, never opacity. Opening the fork popover therefore left it anchored to an invisible trigger once the pointer left the row. Skip the fade entirely while a button is active. Extract the recipe the three toolbars repeated so the rule has one home. Rework the streaming guard to the contract the toolbar now implements: edit and fork are omitted from a streaming response rather than rendered disabled, and the settled turn above keeps its own actions. It asserted the removed disabled-and-transparent behaviour and its opacity check only held because the growing response shifted the row out from under the pointer. * style: Trim message edit chrome and stabilize the status row The edit surface was a titled card sitting inside the conversation: a bordered panel with an "Edit message" heading wrapping bordered fields, which read as a settings dialog rather than an inline editor. Drop the card background, border and heading, and take the footer buttons down to the small size so the editor reads as a field in the message flow. The captured row goes from 253px to 187px. Move "Unsaved changes" into the footer and merge the rerun hint into the same slot. Both previously added their own row, so typing pushed the rest of the conversation down. The slot is clamped to two lines, which stays under the 36px button row, so the footer height holds at 36px regardless of which message is showing. * test: Cover message edit layout stability Add a mock e2e spec that measures the edit footer and section boxes and asserts they hold steady as the status text appears, for both the single-part user editor and a multi-part response. The multi-part case needs an assistant message with two editable parts, so add an E2E_THINK_REPLY marker to the fake model. Its think tags are parsed downstream by the agents stream pipeline, which yields a reasoning part followed by a text part. * fix: Read the fork popover open state from its store Fork mirrored the popover state into its own useState and reset it from an onClose prop. Ariakit 0.4 has no onClose, and React's DOM types accept the name on any element, so it type-checked, landed on a div and never fired. Closing by Escape or an outside click therefore left the button reading as active until the trigger was clicked again. Read the state from the store instead so every close path clears it. * fix: Keep the whole toolbar visible while an action is open Only the triggered button escaped the hover fade, so opening the editor or the fork popover left the row as a single floating button once the pointer moved away. Mark the active button and have every action in the toolbar key off it, so the group stays opaque for as long as a surface is open. The marker is a dedicated class rather than the existing `active`, which HoverButtons pins to the edit button of every assistant message and would hold those toolbars open permanently. The existing guard pressed Escape to close the editor while focus sat on the body, so the editor never closed and its assertion only held because the sibling faded regardless. Close the editor through its own control, and drop focus before measuring the fade now that Escape returns it to the trigger. * fix: Withhold copy while a response is still streaming Text-to-speech, fork and feedback were all withheld from a message that is still generating, but copy was rendered throughout, so the button offered to put half a sentence on the clipboard. Gate it on the same condition. That empties the toolbar for the duration, and SubRow collapses an empty row, so a streaming response now carries no actions at all until it settles. Both guards encoded the old contract: the unit test asserted copy was present and counted a single button, and the browser guard used copy as its proof that the toolbar had mounted. The settled turn above takes over that role. * fix: Move retry navigation to the outer edge of a user turn A user turn is right-aligned, but its sibling navigation rendered ahead of the actions, so the retry counter sat inboard of the icons instead of under the edge of the bubble it belongs to. Order it last on user turns. * fix: Ride the stream instead of chasing it Following a generating answer went through a helper throttled at 145ms, so the thread caught up in visible jerks rather than flowing. It now writes the scroll position directly on each frame, which is what an answer arriving a few pixels at a time actually needs, and glides only for the one long trip a turn makes, when sending has to travel from wherever the reader was down to the newest word. Whether to follow at all is now answered by where the reader is and which way they were going, rather than by the abort flag. `useMessageProcess` raises that flag on any wheel at all, downward ones included, through a throttle whose trailing call lands after the gesture has ended, so nothing timed to the gesture could outlive it. Scrolling down to the newest word could therefore never resume the ride, while the scroll-to-bottom button, which touches no wheel, always could. Arrival is judged on the scroll it produces rather than the wheel tick that started it, because wheel scrolling is animated and at tick time the thread is still far short of where the tick is taking it. Arriving also counts from further out than leaving does: while an answer streams the end recedes between the last tick and the frame that measures it, so judging arrival as tightly as departure leaves a reader unable to catch it at all. * fix: Reveal retry navigation on hover while an answer generates Copy, edit, fork and read-aloud are all withheld from a response that is still generating, which left the retry counter as the only thing rendering under a half-written answer. It now reveals on hover there, like the actions it sits with, and stays put on a settled turn. * fix: Keep a refused rerun from discarding the edit While a response is streaming, the edit action stays available on every earlier row, and those editors see a per-message submitting flag that is false, so Update and rerun is enabled. The send itself is still refused: ask() returns false for the duration of the active submission. Both editors ignored that and closed anyway, so the draft went with them and no rerun ever started. Both rerun paths now check the result and leave the editor untouched when the send is refused, so the work survives until the thread is free. * fix: Let an upward gesture beat the pending send glide Sending arms a smooth glide down to the newest word, and the landing re-pins the thread to the bottom. The landing was scheduled two ways, on scrollend and on a 700ms fallback, and neither was ever cancelled. A reader who changed their mind and headed up mid-flight was pinned again regardless, then dragged back by the next streaming resize. The fallback fires for the whole window, so this held even after the glide had visibly settled. The gesture now marks the glide interrupted, wherever it lets go of the bottom, and the landing stands down when it sees that. A glide the reader leaves alone still re-affirms the ride. * fix: Fade retry navigation on every streaming response format Every other action is withheld from the row that is still generating, so the retry counter is the only thing left under a half-written answer. The plain text row already faded it to hover-only there; the structured rows did not, and left it sitting on its own. Both structured paths now apply the same condition, and the class string the three of them share moves next to the hover action styles it belongs with. * i18n: Correct the copy the edit surface rewrite left behind The multi-part hint told the reader to save first and then rerun, but a save closes the editor and reopening seeds the drafts from what was just saved, so there is nothing left to rerun and the button stays disabled. Rerunning carries a single edited section by design, so the hint now states that limit rather than pointing at a step that is not there. Drop com_ui_save_submit as well: the per-part editor that used it is gone. * test: Make the message visual baselines opt-in The suite asserts sixteen screenshots and the repository tracks none, so Playwright's default treats every one as a miss and the mock e2e job fails on Linux. Baselines only compare cleanly against the machine that produced them, and nothing here can generate ones that match the runner image. The flows keep running and asserting their structure, which is where their value was; only the pixel comparison is now gated behind E2E_VISUAL_SNAPSHOTS. * style: Restore import order in the reworked message files The repository sorter and CI disagreed with what these files were left holding after the edit surface rework. No behavior change. * test: Follow the reworded rerun hint in the edit layout spec The multi-part hint was restated in the previous commit; this assertion still expected the old wording and would have failed the mock e2e suite. * fix: Leave the send glide alone while the answer streams in Every delta of an answer reruns the scroll effect, and the plain follow writes scrollTop outright, which cancels an animation on its first frame. So the glide a send starts was killed by the first token to arrive and the reader was snapped down instead of carried. The follow now stands down while a glide is travelling, which is what the hook already documented but only enforced on the resize path. * fix: Write a saved edit onto the thread as it stands An earlier turn stays editable while the newest answer streams, and the save captured the thread before the request but wrote it back after. Every delta that landed during the round trip was overwritten. Most of the time the next delta re-merged and the damage showed as a one-frame truncation, but a save that resolved after the stream's final write left the cache wrong for the rest of the session. The thread is now read once the request has resolved, which is what the content part editor already did. The editor actions in this file also wrap again rather than hold one unbreakable row, for the reason given in the following commit. * fix: Let the editor actions wrap on a narrow row At 320px an assistant turn gives the editor about 252px once page padding, the identity column and the row gap are taken out, and Cancel, Save and Update & rerun need more than that in English alone. The group was pinned with shrink-0, so it ran past the edge of the row instead of wrapping. A longer translated label makes it worse, and the user turn had no margin left either. Both editors wrap again, which is what the footer did before the status row was folded into it. * fix: Catch up to the new bottom when the glide lands Following stands down for the length of the glide, so an answer that arrives while it travels moves the bottom past the target the glide aimed at. A short response that finished before the glide reported landing left the thread a few lines short of its own end, with nothing left to correct it. Landing now closes whatever gap opened, unless the reader took over on the way. * test: Follow the renamed rerun button in the edit flow specs The button became 'Update & rerun' when the edit surfaces were unified, but two edit-flow specs still located 'Save & Submit' and would have waited for it until they timed out. A type comment named the old button too. * fix: Judge the first thread scroll against a real position The direction check seeded its last-position ref at 0, so the first scroll event on an opened thread, which arrives carrying a large positive scrollTop, read as a jump downward. Near the end that cleared the abort flag and re-pinned a reader to the stream they were scrolling away from. Take the first event as a baseline and judge direction from the next. * fix: Hold the content part editor to what it replaced EditContentParts took over from EditTextPart and left two of its behaviors behind. An emptied box now blocks Save and rerun instead of persisting a blank part. EditTextPart refused the same edit through its form's required rule and the sibling EditMessage still does, so both editors hold one line. The keyboard shortcuts reach the save paths directly, so they are guarded there too, and the footer says why the buttons are down. The editor also follows the chat direction again, taking dir and text alignment from the same setting EditMessage reads. * fix: Hold the footer height while a response streams Every action is withheld from the row that is still generating, and a lone sibling counter renders nothing, so the footer measured zero until the answer landed and then sprang to the height of the buttons. The transcript stepped upward under the reader at the moment a response completed. The placeholder that used to reserve this space went when the footer became unconditional, so hold the height on the row itself instead. * fix: Remember where the thread was put before judging a gesture Direction is judged against the last sample, and the thread is placed at its end without the reader touching it. With no record of where it was put, their first gesture was spent taking the baseline instead of being obeyed: a single PageUp cleared no flag of its own, so the next streamed resize rode the reader straight back to the end they were leaving. Every programmatic move now records the position it left the thread at, so the sentinel stands only until something has actually placed it. * fix: Spend the start of a turn only once it can be honored A reader who scrolls away during one answer leaves the abort flag raised, and nothing lowers it until the next connection opens, which is after this effect has already seen the send. Marking the turn as started on that first pass spent it against a closed gate: by the time the flag cleared there was no start left to honor, the reader was still detached, and the answer they had just asked for streamed on offscreen. Record the turn as started only on the pass that acts on it. * fix: Show the part edits that survived a refused save The editor saves every changed part through one button, but the endpoint takes a single part per call and nothing rolls a write back. A part the server refused therefore left the earlier ones stored while the editor reported that the message could not be saved, so cancelling from there walked away from edits that were already live. Record the writes that landed and reconcile the transcript with them whichever way the save ended. The refused parts are the only ones left holding a draft, so a retry no longer rewrites what already arrived. * fix: Stop a shared transcript from calling the sharer the reader The share row reused the chat view's user label, which reads "You". It is the screen-reader heading for the user turn, so anyone opening a share link heard every prompt the sharer wrote credited to themselves. Use the neutral "User" label on this surface. It keeps the localization the row gained, unlike the untranslated string it replaced. * fix: Let go of the stream when an interaction settles over several resizes Expanding a tool result mid-answer renders the container first and fills it once its contents arrive, so one gesture produces more than one resize. Only the first was credited to the interaction. The second read the reader as still riding the stream and put them back on the bottom they had just left. The suppressed resize now settles the ride as well as the near-bottom measure, using the position the interaction actually left the reader at, so an interaction that kept them on the end still streams. * fix: Edit inside a structured text part instead of flattening it A text content part holds either a string or a { value, annotations } object. The Assistants thread sync persists the structured form with its file citations intact, and the editor reads the part through the same union, so saving an edit wrote a bare string over the whole object and took every citation with it. The same object was handed to the tokenizer, which measures length, so a part that had been edited this way also stored a NaN token count. Write the edit into value, keep the rest of the part, and count the text itself. * fix: Keep a saved part's citations in the transcript it is written back to A text or think part holds either a bare string or a { value, annotations } object, and the editor already read both through getPartText. Writing the draft back into the local message cache put the string over the whole value, so a response carrying file citations lost them the moment it was edited and did not get them back until a refetch. Reading and writing now go through the same accessor, so an edit lands in the shape it was read from and the rest of the part survives. * fix: Let the message editor follow the chosen font size Editing a message dropped the draft to a fixed 14px regardless of the Font Size setting. On dev the textarea carried the markdown class, so it read --markdown-font-size like the rendered message does; restyling it into a bordered box replaced that with text-sm, and the new per-part editor was written the same way. Anyone on Extra Small, Large or Extra Large saw the text jump the moment they entered edit mode. Share the .message-content typography with the editors through a message-editor-text class so a draft is sized like the message it replaces and keeps tracking the setting. |
||
|
Danny Avila
|
1a3e2aebcb
|
🛰️ fix: Attach Request-Scoped MCP Servers (#14780)
* fix: attach request-scoped MCP servers * fix: satisfy MCP static checks * fix: format MCP runtime hint |
||
|
Danny Avila
|
dccef82254
|
🪶 chore: Aggregate Empty MCP Tool Logs (#14767)
* fix: aggregate empty MCP tool logs * fix: retain server names in MCP tool logs |
||
|
Danny Avila
|
7fc62023eb
|
🧷 fix: Safely Recover Runtime MCP OAuth Rejections (#14684)
* fix runtime MCP OAuth recovery * style: sort LC-008 imports * fix: single-flight runtime OAuth handlers * fix: retain transport OAuth failures for recovery * fix(mcp): preserve OAuth recovery connections * test(mcp): type request-scoped config fixture * fix(mcp): harden shared OAuth recovery * fix(mcp): bound OAuth recovery escalation * style(mcp): sort OAuth integration imports * fix(mcp): harden OAuth recovery boundaries * fix(mcp): abort shared recovery waiters * fix(mcp): bound request OAuth recovery phases * fix(mcp): close OAuth recovery ownership gaps * fix(mcp): retry borrowers closed by OAuth recovery * fix(mcp): drain borrowers before OAuth reconnect * fix(mcp): preserve eviction across OAuth recovery * fix(mcp): unify OAuth recovery leases * fix(mcp): serialize cache reuse with recovery * fix(mcp): make recovery checkout atomic * test(mcp): use numeric config timestamp * fix(mcp): reacquire recovery checkouts * fix(mcp): retain shared recovery disposal * fix(mcp): restart checkout after recovery takeover * fix(mcp): close recovery lifecycle gaps * refactor(mcp): deepen OAuth recovery lifecycle * fix(mcp): harden OAuth lifecycle disposal * style(mcp): sort OAuth lifecycle imports * fix: lease MCP OAuth lifecycle edges * fix(mcp): isolate shared OAuth flows from aborts --------- Co-authored-by: Dennis Schenk <dennis@gridonic.ch> |
||
|
Marco Beretta
|
152dcf4721
|
🔗 feat: Shared Conversation Badge and Stable Share Links (#14712)
* feat: improve shared conversation links * test: Cover Shared Link Lifecycle * test: Cover Shared File Snapshots * fix: address review findings on shared links Stop double-decoding the conversation search term. Express already decodes req.query, so the route's extra decodeURIComponent threw URIError on any term containing a bare percent sign and mangled percent-escape-looking text. The sidebar already sent the term raw, so this failed there too. Advance a share's stored target to its branch tail when an update omits one. Updating from the conversation list could not resolve the tail and reused the stored target verbatim, silently republishing the same snapshot instead of the turns added since. Require revalidation on shared files. Updates now keep the shareId, so the file URL no longer changes and a cached response could outlive a revoked share-files choice; an ETag over the pinned snapshot fields keeps unchanged files on 304. * fix: keep the shared badge across conversation cache replacements isShared is derived per list request and absent from single-conversation payloads, so rename, pin, and the SSE conversation updates dropped it when they swapped a server response into the sidebar cache, hiding the badge until an unrelated list refetch. Carry the cached value forward in updateConvoInAllQueries so every replacing caller is covered, while an explicit value still wins. * test: mock syncStaticTools in server boot specs initializeMCPs now calls syncStaticTools when no MCP servers are configured, but the server boot specs stub ~/server/services/Config without it. Post-listen initialization threw, hit process.exit(1), and took the jest worker down until it exceeded the retry limit. * fix: address codex findings on the shared DataTable and file ETag Keep the published DataTable export bound to the legacy component and ship the design-system table as VirtualizedDataTable, so external consumers of @librechat/client keep the props they compile against. Fold the snapshot's stored location into the shared-file ETag, so a re-published output that keeps its size and revision but moves its object no longer revalidates to a stale 304. Auto-fill the table when a first page is too short to overflow its container, since pagination is otherwise only reachable through the scroll handler. * fix: re-scope share grants before publishing and retry stalled auto-fill Move the shared-link ACL expiration write ahead of the content update. The shareId survives an update, so a failed ACL write after the write-through left the new messages and file snapshot readable at the same URL while the owner saw a 500. Retry a rejected auto-fill fetch up to three times: an unscrollable table has no scroll event to fall back on, and the sentinel alone would strand it on the first page. * fix: follow regenerated branches and pin forks to the payload they saw advanceTargetToBranchTail only walked descendants, so a target replaced by a regeneration (a sibling, not a child) left the update parked on the obsolete branch and published none of the turns that followed. A childless target now hops once to the newest sibling the conversation continued under. A shareId survives an update, so an owner republishing between a viewer's load and their Continue click would resolve targetMessageIndex against different messages. The fork request now carries the payload's updatedAt and is rejected with 409 when it no longer matches; the viewer gets the current version pulled in and can retry. * fix: keep table sorting and legacy backfills from breaking share flows Restore the union formatting a local lint-staged prettier collapsed in data-provider types, which broke the CI lint run. Header clicks now toggle direction instead of cycling through an unsorted state, which the controlled tables translated straight back into the default and made one direction unreachable. Re-arm the auto-fill guard on a sort change: a re-sorted first page arrives with the same row count, and the guard would otherwise suppress paging on a container that still cannot scroll. Lazy fileSnapshots backfills no longer touch updatedAt. That timestamp is the revision a viewer's fork is validated against, so a legacy share's first read would have made the Continue click that followed it fail with a 409. * fix: break pagination ties by id and reset share state per conversation Both list cursors marked a page boundary with values that repeat: conversations by (sort field, updatedAt) and shared links by the sort field alone. Imported chats share a title and a timestamp, so every row tied with the boundary was skipped. Both now carry the boundary row's _id and sort by it last, and the shared-links cursor is an opaque composite the route still validates before querying. The share dialog outlives a switch between conversations, so a link with files disabled left the next conversation's dialog showing the switch off and quietly published without files. The stored choice now falls back to the enabled default, and a stale link no longer sits in the copy field. * fix: keep titleless shared links in the paginated list A share has no title default, and BSON orders a missing title before every string, so encoding the boundary as an empty string skipped the remaining untitled links when sorting Name ascending and re-admitted all of them descending. The cursor now carries the boundary's null rather than flattening it, and because $lt and $gt are type-bracketed against a string, descending adds an explicit clause for the untitled tail that a string comparison can never reach. * style: sort share method imports * fix: fail closed on orphaned share targets and guard snapshot backfills getMessagesUpToTarget walked levels from the roots and returned everything it had accumulated when the target was never reached. An imported or partially deleted branch whose parent is missing therefore published the whole conversation instead of the selected branch; the walk now returns nothing unless it actually reaches the target. A lazy backfill wrote fileSnapshots unconditionally, so a viewer's first read of a legacy link could land after a republish and restore the snapshot it replaced, re-authorizing the stable URL of a file the owner had just removed. The write is now conditional on the link still having no snapshot, and the stored one wins any race. Regenerating a message above the shared tail leaves the whole stored branch childless, so the target walk now climbs to the closest ancestor the conversation continued under instead of stopping at the stored tail's own siblings. Changing the search or sort also returns the table's viewport to the top, since the query holds the previous rows while it refetches. * fix: page through titleless rows on both sides of the cursor The route validator still required a string primary, so the composite cursor the data layer issues for a titleless boundary came back as a 400 and the shared-links table stopped at that page. Conversations had the same type-bracketing gap the shared links just closed: a name-sorted page could not reach conversations with no title, since a comparison against a string never matches a missing field. The cursor now carries the null and the filter spells out the titleless clauses for both directions. * fix: keep the share badge read-only and refresh rows on cell changes ensureLinkPermissions re-granted the owner ACL entry on every call, so rendering the header's shared badge turned ordinary navigation into a permission write. It now checks for the grant first and only migrates a link that still lacks one. A fork carrying a positional target but no revision falls back to the whole share, since nothing proves which payload the index was counted against. The memoized table row compared row data and selection only, so a cell rendering external state (the archived list's pending Restore, for one) kept its stale rendering until the row object itself moved; rows now also compare a marker that moves with the column definitions. * fix: keep the shared badge honest when a delete fails or a link remains A failed delete left the conversation looking unshared: the optimistic snapshot covered only the shared-link queries, not the conversation caches the badge reads. The cleared conversations are now restored with the rest. A conversation can hold one link per target message, so clearing the badge on delete is a guess. The conversation list is invalidated once the mutation settles, letting the server decide from the links that are actually left. * fix: refetch every cached conversation page after deleting a link The invalidation was pinned to page zero, so a conversation cached further down the sidebar kept the badge the optimistic update had already cleared even when another targeted link survived. * fix: treat a failed page fetch as a failed auto-fill React Query resolves fetchNextPage with an error result instead of rejecting, so the rejection handler never ran: the guard stayed armed on the unchanged row count and an unscrollable table could never reach the next page. * refactor: move the share request helpers into the typed backend Cursor validation, page-size clamping and the shared-file cache validator were plain backend logic sitting in the legacy JS route. They now live in packages/api with their own tests, and the route keeps only the Express-side wiring: reading query params, mapping domain error codes to status codes, and writing the response. Also carries the requested file choice into the cache entry the create and update mutations synthesize, since the response never echoes it and the dialog reads a resolved entry with no choice as the enabled default. * fix: hold auto-fill while the replacement page is in flight A search or sort swap keeps the previous rows and hasNextPage on screen while the new first page loads, so the re-armed auto-fill asked for page two against a query that was still fetching its first. An infinite query runs one fetch at a time, so that request could cancel or interfere with the one already out. Both tables now pass their fetching state and the guard waits for it. * fix: stop advertising links a deployment no longer serves The sidebar badge rendered from the derived flag alone, so a deployment that turned shared links off still told owners a link was live while the public routes were unregistered. The share dialog called the link a snapshot, but the payload populates the referenced message documents on every request: an edit to an already-shared message is visible immediately, and Update only adds newly referenced ones. The copy now says that. Scroll pagination inspects a resolved error the way auto-fill already does, since React Query reports a failed page that way instead of rejecting. * a11y: gate the shared conversation label on the feature flag The icon stopped rendering when a deployment turns shared links off, but the row still announced the conversation as shared to screen readers. Both now read the same condition. * fix: accept long title cursors and stop badge work the feature disables The cursor cap was tight enough that a Name-sorted page ending on a long title produced a nextCursor the next request rejected, stranding the rest of the list. It now sits well clear of anything the server can issue. The conversation list skipped straight into the shared-link lookup even where ALLOW_SHARED_LINKS is off, paying a round trip on the sidebar's first page for a badge that is never rendered. A regeneration is newer than what it replaced, so only a newer sibling counts: an older one that still has follow-ups is the branch the target was regenerated away from, and resuming there published turns the target had excluded. * fix: hold scroll pagination while a replacement page loads Resetting the viewport to the top after a search or sort change fires a scroll event, and the retained previous rows still report another page, so the handler asked for page two of a query that was still loading page one. * fix: keep the legacy share migration ahead of the owner-grant shortcut A legacy row keeps its marker until every grant it needs exists, so an owner grant on its own is not proof the migration finished. Reading the marker first means a half-migrated public link still gets its public grant, while a fully migrated one keeps the read-only settled path the badge lookup depends on. |
||
|
Danny Avila
|
1bccc2bc18
|
📡 fix: Refresh MCP Tools After List-Changed Notifications (#14686)
* fix(mcp): handle dynamic tool list changes Co-authored-by: Pascal Garber <pascal@artandcode.studio> * test(mcp): fix CI validation * fix(mcp): keep dynamic tool catalogs live * fix(mcp): harden dynamic catalog lifecycle * test(mcp): use typed startup connection * test(mcp): isolate dynamic e2e fixtures * fix(mcp): refresh tools after reconnect * fix(mcp): close dynamic catalog cache gaps * test(mcp): update OAuth connection mocks * fix(mcp): preserve app snapshot ownership * style(mcp): sort connection imports * fix(mcp): close review race conditions * fix(mcp): preserve cache ownership edges * fix(mcp): harden recovery lifecycle * fix(mcp): guard tool-less app refresh * fix(mcp): fence distributed cache races * fix(mcp): retire stale connection state * fix(mcp): keep tool snapshots authoritative * fix(mcp): fence stale app tool publications * style(mcp): sort repository test imports * test(mcp): mock empty startup publication * fix(mcp): preserve app publication generations * fix(mcp): harden publication recovery races * fix(mcp): address tool catalogs by runtime config * fix(mcp): load scoped catalogs for assistant writes * fix(mcp): harden catalog publication recovery * fix(mcp): serialize forced connection replacement * fix(mcp): serialize ordinary creation with replacements * fix(mcp): harden catalog fallback boundaries * fix(mcp): close lifecycle fencing gaps * fix(mcp): preserve catalog authority on failures * fix(mcp): compensate failed catalog mutations * fix(mcp): fence catalog refresh ordering * style(mcp): sort agent loader imports * fix(mcp): cancel stale connection creation * fix(mcp): fence catalog coordination * fix(mcp): close catalog race windows * fix(mcp): harden cross-pod catalog fencing * fix(mcp): close catalog lifecycle edges * style(mcp): sort assistant imports * fix(mcp): reject stale recovery authority * fix(mcp): restore static catalog on every startup * fix(mcp): order app catalog publications * style(mcp): sort catalog revision imports * fix(mcp): separate catalog allocation and commit fences --------- Co-authored-by: Pascal Garber <pascal@artandcode.studio> |
||
Dustin Healy
|
c6bb77325a
|
🔗 feat: Admin Panel Link in Settings for Admins (#14662)
Expose ADMIN_PANEL_URL through the startup config for users holding the access:admin capability, and render an Admin section in Settings > General with an external link to the admin panel. The URL is omitted server-side for unauthenticated requests and users without admin access. |
||
|
Danny Avila
|
56175af0b5
|
🎟️ fix: Reconcile MCP OAuth Readiness Across Pods (#14629)
* fix: stabilize MCP OAuth readiness across pods * fix: harden MCP readiness review findings * fix: resolve CI type check and terminal OAuth polling * fix: address MCP OAuth readiness review * fix: align MCP OAuth readiness state * test: stabilize MCP OAuth readiness assertion * fix: reject stale MCP OAuth callbacks * fix: close distributed MCP OAuth readiness gaps * style: sort Redis MCP test imports * fix: preserve MCP OAuth polling across rolling pods * fix: finalize distributed MCP OAuth readiness * fix: preserve runtime-detected MCP OAuth * fix: report runtime MCP OAuth readiness * fix: preserve live MCP OAuth classification * style: sort MCP connection imports |
||
|
Danny Avila
|
22642df40c
|
🧹 fix: Exclude Mongo ID From Conversation Updates (#14631)
* fix: Exclude Mongo ID from conversation updates * fix: Limit conversation sync to conversation ID * fix: Preserve explicit conversation metadata |
||
|
Danny Avila
|
db6ba5392a
|
🪢 fix: Bind MCP OAuth Secrets to Trusted Endpoints (#14578)
* fix: bind MCP OAuth secrets to trusted endpoints * fix: bind stored MCP OAuth clients during refresh * fix: address MCP OAuth review findings * fix: bind stored MCP OAuth credentials * fix: make MCP OAuth credentials generation-safe * test: update MCP OAuth uninstall binding fixtures * fix: harden MCP OAuth credential persistence * fix: scope MCP OAuth refresh single-flight * style: sort MCP OAuth token imports |
||
|
Danny Avila
|
6f45a9e32e
|
🔗 fix: Normalize MCP Tool Keys at Every Producer, Resolve Raw Names via Aliases (#14553)
* 🔗 fix: Normalize MCP Tool Keys at Every Producer, Resolve Raw Names via Aliases Tool keys had two spellings that could diverge for any server whose name contains characters outside [a-zA-Z0-9_.-]: the tool cache (and registry inspector) built keys with the RAW server name, while runtime instances are named with normalizeServerName(serverName). Three code comments already asserted "tool keys embed the normalized server name" - no producer honored it. For a special-character server that meant: - definitions-only mode shipped raw def names the model echoed back, but the executor's tool map held the normalized instance name, so every call failed with "Tool not found"; - per-tool tool_options (defer_loading / allowed_callers / run_in_background / describe_intent) were persisted under raw keys that never matched the definition names the option passes resolve against, so builder settings were silently inert; - tool-key parsing against normalized candidate lists silently fell back to last-delimiter splitting, which mis-parses delimiter-bearing tool names. The reconciliation is one contract enforced in three moves: 1. PRODUCERS NORMALIZE. The tool cache (packages/api/src/mcp/tools.ts) and the registry inspector build keys with the normalized server name, matching the instance names MCP.js has always assigned. The builder's tool ids, agent.tools entries, tool_options keys, and definition names all flow from these keys, so every model-facing name now agrees. The cache STORE stays keyed by the raw config name. 2. CONFIG LOOKUPS RESOLVE ALIASES. New shared helpers in data-provider (buildServerNameAliases, normalizeMCPToolKey) map a parsed normalized name back to the raw config name that the registry, config maps, tool cache, and plugin-auth rows are keyed by. Applied in the definitions loader closure, handleTools grouping, createMCPTool's parsing fallback, getUserMCPAuthMap, and the MCP tools endpoint - matching both spellings so legacy raw keys keep resolving. 3. LEGACY DATA HEALS AT ONE BOUNDARY. initializeAgent rewrites raw-keyed agent.tools entries and tool_options keys to the normalized form (normalizeAgentToolKeys) before anything consumes them, so agents persisted under the old convention load their tools AND have all four per-tool options honored. Placeholder and server-pin tokens stay raw - they are config-identity references, not model-facing names. Servers whose names are already in the safe character set (the common case) produce byte-identical keys before and after; the fast path allocates nothing. Stale Redis-cached raw keys self-heal via the existing reconnect-on-missing path within one cache cycle. * 🧯 fix: Deterministic Alias Collisions + Raw Names in Definition Metadata Two review findings on the normalization contract: - Two configured server names that normalize to the same segment (e.g. 'Sales Force' and 'Sales:Force' -> 'Sales_Force') produce inherently ambiguous tool keys; the alias map silently resolved last-wins, so a tool selected from one server could execute against the other's config. buildServerNameAliases now resolves collisions to the FIRST configured name deterministically, and resolveMCPServerContext warns once per colliding pair per process so the operator can rename one server. A collision-resistant identifier would change every existing tool key, so detection + stable routing is the right treatment here; startup-time config validation can follow separately. - The definitions loader resolved parsed (normalized) server names to raw only inside the ToolService closure, while the definition metadata (serverName -> mcpRawServerName) kept the normalized value. Server instructions are keyed by raw config names, so a special-character server's instructions were silently omitted in definitions-only mode. loadToolDefinitions now takes rawServerNames, resolves the boundary against both spellings, and stores the RAW name in definition metadata - consistent with the instance path. * 🧯 fix: Heal Stale Caches, Skill Allowed-Tools, and Builder Selectors Three review findings on the normalization rollout, all in the transition class: - Stale cache entries (P1): the definitions-only loader treats the per-server tool map as authoritative and never reconnects on a per-key miss, so a pre-change raw-keyed Redis entry would make a special-character server's tools vanish for up to the cache TTL. getMCPServerTools now heals legacy raw-keyed entries to the normalized format at read time (keys and function names), covering every consumer with no coordinated invalidation; safe names return the map untouched. - Skill allowed-tools: a skill declaring a raw MCP key in allowed-tools bypassed the initialize-boundary heal (the union runs after it) and would neither dedupe against healed agent tools nor match the normalized tool map. The primes' allowedTools now pass through the same normalizeAgentToolKeys heal before unioning. - Builder selectors: matchesMcpServer and useVisibleTools parsed tool ids against raw server names only, so an attached special-character server rendered as an unselected orphan card. Both now accept the normalized spelling and resolve it back to the raw map key, keeping legacy raw ids working. * 🧯 fix: Fail Closed on Normalized Server-Name Collisions Escalation of the collision finding: a deterministic first-wins alias plus a warning still let the tools listing publish BOTH colliding servers, so a tool selected under the shadowed second server would silently execute against the first server's configuration (their model-facing keys are identical, so routing cannot ever distinguish them). - findShadowedServerNames identifies later-configured names whose normalized form an earlier different name claimed. - getMCPTools excludes shadowed servers from the published listing entirely (with a warn naming the collision), so their tools are never selectable - nothing ambiguous can be picked. - Server creation reserves both spellings: a generated slug may not collide with a raw config name OR the normalized form its tool keys would carry. Collision-resistant model-facing IDs remain out of scope: changing normalizeServerName's output would rewrite every existing tool key (agent documents, caches, instance names) for ALL servers to handle a misconfiguration that is now blocked from exposure instead. * ✅ fix: Dedupe Reserved Server-Name Spellings at Creation The reservation list appended normalized forms unconditionally, which duplicated every safe name (raw === normalized) and broke the route-level contract test pinning the exact list. Dedupe via a Set so safe names contribute one entry, while special-character names still reserve both spellings; adds the special-character reservation case. * 🧯 fix: Never Heal a Shadowed Server's Keys; Align Authorization Tie-Break Persisted references were the remaining collision vector: an agent or skill saved with the shadowed later server's raw key was HEALED into the shared normalized key, authorized through a last-wins map, and routed first-wins - authorized as one server, executed as another. - normalizeAgentToolKeys now refuses to rewrite keys of shadowed servers (findShadowedServerNames): rewriting would produce exactly the first server's key. Left raw, the key cannot match the normalized-keyed tool map and the tool fails visibly - broken beats misrouted. Covers agent.tools, tool_options, and skill allowed-tools through the shared heal. - filterAuthorizedTools (agents/v1.js) builds its normalized-to-raw map via the shared buildServerNameAliases instead of a last-wins Map constructor, so authorization resolves a colliding key to the SAME first server execution routes to. * 🧯 fix: Direct Identity Wins Over Aliases; Heal Client Forms and Degraded Contexts Four review findings on the normalization edges: - Alias hijack (P1): a user-DB server named exactly like an operator server's normalized form ('foo' vs YAML 'foo!') had its tools rerouted to the operator server by unconditional alias resolution. Resolution is now DIRECT-FIRST everywhere: the parsed name is tried as-is, and only when nothing resolves is it treated as a normalized spelling (definitions loader, handleTools grouping, createMCPTool fallback). buildServerNameAliases seats identity entries before derived ones so a literal name owns its slot regardless of config order, findShadowedServerNames and the collision warning derive from the same construction, and getUserMCPAuthMap fetches auth under both spellings so either owner finds its rows. - Builder double-match: a normalized name containing the delimiter ('foo mcp bar' -> 'foo_mcp_bar') also suffix-matched a server named 'bar', selecting both cards and making removal strip the wrong tool. matchesMcpServer now resolves the token ONCE against the full configured list (longest boundary, both spellings) when the caller supplies it; selection and removal share the resolution. - Builder legacy ids: an agent saved with raw-keyed ids showed its tools unchecked while the runtime heal kept them active, and selection updates never replaced the legacy entries. McpSection maps legacy raw ids to their current normalized ids when deriving and rewriting this server's selection. - Degraded context: a transient ensureConfigServers failure returned an entirely empty context, leaving normalized keys unresolvable for the request. resolveMCPServerContext now keeps the name lists (they derive from the config snapshot alone) and degrades only the lazy-init configs. * 🧯 fix: Collision Detection Sees Accessible Servers; Shadowed Refs Fail Closed End to End Round follow-ups on the collision design, all in the DB-server-visibility class: - The legacy-key heal detected collisions against operator-config names only, so healing could still produce a key that direct-first resolution routes to an invisible user-DB server. initializeAgent gains an optional getAccessibleMcpServerNames dep (wired through ToolService for controllers that mock it, directly elsewhere), consulted ONLY when a configured name needs normalization - zero cost for safe-name deployments. The heal then sees the full accessible set and skips shadowed servers' keys. - Wildcard and legacy raw tokens bypassed catalog filtering, letting a shadowed server's instances join a run under the same normalized names as the winner's. filterAuthorizedTools rejects tools of shadowed servers at authorization (its merged map sees DB + config), and handleTools skips them at execution. - The builder migrated only tool selection, not tool_options: legacy raw option keys showed disabled while the runtime honored them, and toggles could not clear them. McpSection now migrates option keys to the current normalized ids (existing normalized entries win). - A transient ensureConfigServers failure degraded to an EMPTY server context, leaving normalized keys unresolvable for the request. resolveMCPServerContext keeps the name lists (derived from the config snapshot alone) and degrades only the lazy-init configs. * 🧯 fix: Complete the Collision Audit at Every Gate; Safer Heal Semantics Round follow-ups hardening the collision audit: - Execution guards now consult the FULL accessible set: the caller's heal threads its already-fetched names through loadTools, and handleTools fetches them itself when a configured name needs normalization (never for safe-name deployments) - so a cross-tier collision (user-DB 'foo' vs operator 'foo!') fails closed at eager execution instead of joining the run under one normalized name. - Healing is SKIPPED when the collision audit cannot complete (transient lookup failure, or no dep): un-healed raw keys still resolve through the direct-first candidates, so skipping is safe while rewriting against an incomplete audit is not. - The audit lookup is gated on the agent actually carrying delimiter-bearing keys (tools, tool_options, or skill allowed-tools), so non-MCP agents never pay a registry round-trip even on specially named deployments. - normalizeAgentToolKeys gives the CURRENT (normalized) entry precedence when both spellings carry options, matching the builder's migration semantics instead of letting insertion order decide. - The builder's toCurrentToolId resolves entries boundary-exactly against every configured server (longest match, both spellings), so a raw suffix shared with a LONGER server name can no longer reassign that server's selection or options while another dialog is open. * 🧯 fix: Shared Collision Audit for Definitions Loading; Fail Closed on Audit Failure Round follow-ups closing the remaining audit gaps: - The definitions-only loader now consumes the same collision audit as eager loading: shadowed servers' entries (wildcards included) are dropped before definitions are emitted, so the default execution path can never resolve a shadowed server's normalized function name to another server. The audit names thread from initializeAgent's heal; the loader self-fetches only when a configured name needs normalization. - resolveCollisionAuditNames centralizes the audit-resolution policy (threaded set > self-fetch when needed > incomplete on failure), and BOTH loaders now fail closed under an incomplete audit: any normalization-sensitive reference (its own name needs normalizing, or it equals the normalized form of a configured special-character name) is skipped with a warning instead of being audited against operator names alone. isNormalizationSensitiveName lives in packages/api as a pure helper so test mocks use the real predicate. - normalizeAgentToolKeys collapses duplicate ids after healing (order-preserving): a document carrying both spellings converges on one key, never two instances with the same function name. * 🧯 fix: Thread the Audit Everywhere; Identity-Aware Alias Fallback Round follow-ups on audit plumbing: - The OpenAI-compatible and Responses tool loaders now forward the already-resolved accessibleMcpServerNames instead of discarding it, so the definitions loader neither repeats the registry lookup nor fails closed on a transient second lookup after the first succeeded. - The skill-only path threads its audit: when the baseline agent has no MCP keys but a primed skill's allowed-tools fetched the complete set, that set (not the operator-only list) reaches the loader, so the collision remains visible and the shadowed reference stays rejected end to end. - OAuth discovery iterates the collision-FILTERED tool list, so a request can no longer emit an OAuth prompt, wait out the connection timeout, and reconnect a server whose definitions were deliberately rejected. - The definitions loader's alias fallback is identity-aware: when the parsed name IS a known accessible server, a null tool fetch means temporarily unavailable (OAuth pending, missing user variables, disconnected) and no longer reroutes to the raw alias - previously the aliased operator server's definitions could be emitted under the unavailable DB server's names. * 🧯 fix: Legacy-Key Definition Lookup; Retain Audit for Deferred Execution - createMCPTool resolves tool definitions by BOTH spellings: the key as persisted plus the canonical normalized key built from the resolved server name. Assistants and direct tool calls persisted before the rollout bypass the agent-boundary heal and arrive with raw keys, while availableTools is now indexed canonically - previously every such call missed the index, burned a reconnect, and returned the unavailable stub permanently via the negative cache. - The initialized agent retains accessibleMcpServerNames (the COMPLETE collision audit this initialization resolved), buildAgentToolContext copies it into every per-agent tool context, and loadToolsForExecution threads it into the eager loader as bare options. Deferred/event-driven execution therefore reuses the snapshot instead of repeating the merged registry read - a transient failure there could fail-closed a tool the same turn already advertised from the successful first audit. - MCP.spec.js keeps @librechat/api pure helpers REAL (requireActual spread) so normalization paths are exercised rather than mirrored. * 🧯 fix: Parse Legacy Keys Against Both Server-Name Spellings createMCPTool's boundary candidates were normalized-only, so a legacy raw key whose server name contains the delimiter (foo_mcp_bar!) missed the suffix match and fell to the generic last-delimiter split - the canonical rebuild then produced a key that could never hit the index and the persisted call stubbed out. The candidate list now carries the RAW resolved name (and raw config names on the parse-only path) next to the normalized spellings. * 🧯 fix: Honest Audit Completeness; Shadowed-Server Form-Key Guard - resolveAllMcpConfigs tolerates ensureConfigServers failures, so the merged registry read can silently omit config-only servers while the audit still reported complete: true - a foo/foo! collision would go unseen and a persisted key could route to the wrong server. Both audit consumers now union the snapshot-derived raw config names back in (resolveCollisionAuditNames unions the caller's rawServerNames; the initializeAgent heal unions configRawServerNames), keeping the completeness label honest without an extra read: operator names come from the registry-independent config snapshot, user-DB names from the merged read that fails loudly into the existing incomplete path. - The client tool_options migration now mirrors the runtime heal's fail-closed rule for SHADOWED servers: when the dialog's server has lost its normalized slot to another catalog name, legacy raw keys stay raw instead of being rewritten onto the winning server's key, where a later save would apply the wrong server's per-tool settings. The dialog's own server joins the alias construction so a stale catalog map can't misread as a collision. * 🧯 fix: Heal Legacy Assistant MCP Tool Names on Save The assistants create/update controllers look tools up in the cached definitions by exact key, and the cache is now normalized-keyed - an assistant saved before the convention resubmits its raw-suffixed MCP name on every edit, so any save silently removed the tool. healMcpToolNames pre-heals the payload's tool list: a delimiter-bearing string that misses the cache resolves through the configured raw names (longest-suffix, boundary-exact) and rewrites to the normalized key only when that key actually exists in the cache. SHADOWED raw names stay raw and fail closed, mirroring the runtime heal; the config read happens only when a delimiter-bearing name actually misses, and read failures propagate (write path) rather than silently dropping tools. v2's update loop also stops re-reading the tool cache per iteration. * 🧯 fix: Full-Audit Shadow Set + Dedupe in the Assistant Key Heal - The assistant-save heal built its shadow set from operator config names alone, so a cross-tier collision (user-DB `foo` owning the normalized slot of operator `foo!`) looked unshadowed and the legacy key healed into the shared normalized name - which direct-first execution then binds to the DB server. The shadow set now comes from resolveCollisionAuditNames' full accessible audit, and an incomplete audit skips healing outright (every rewrite candidate is normalization-sensitive by construction, so raw-and-fail-closed is the only safe answer). - Healed string entries dedupe order-preserving: a payload carrying both spellings of the same tool collapses to one entry instead of expanding into duplicate function definitions the provider rejects. |
||
|
Danny Avila
|
78ec1940a2
|
🧹 fix: Clean Up MCP OAuth State Mappings on Uninstall + Reject Superseded Callbacks (#14549)
* 🧹 fix: Clean Up MCP OAuth State Mappings on Uninstall + Reject Superseded Callbacks Disconnecting an OAuth MCP server deleted its mcp_oauth flows but left the mcp_oauth_state:{state} mappings behind for the full TTL. Because flow ids are deterministic (userId:serverName) and the CSRF token is HMAC(flowId), a stale browser tab's callback could resolve its orphaned state to the NEXT flow for the same server, pass CSRF, burn the fresh flow's one-shot CSRF cookie, and fail the PKCE exchange, sabotaging the legitimate retry. - Add MCPOAuthHandler.deleteFlowAndStateMapping: reads the flow's stored state and deletes the mapping before the flow (mapping-first so a crash between deletes fails closed instead of recreating the orphan) - Route mcp_oauth deletions in clearStoredMCPOAuthState through the helper for both tenant-scoped and legacy flow ids - Reject callbacks whose state does not match the resolved flow's stored state: the only control distinguishing a superseded attempt from the current one on a deterministic flow id Fixes #14534 * fix: gate failFlow on state match in the OAuth error branch (Codex P1) The provider-error branch failed the resolved flow on CSRF/session alone, so a superseded error callback resolved through an orphaned mapping could mark the current flow FAILED. Apply the same stored-state equality gate before failFlow. * fix: leave the flow in place when the state-mapping delete fails (Codex P2) deleteFlow swallows storage errors and returns false, and deleteStateMapping discarded that result, so a failed mapping delete followed by a successful flow delete would silently recreate the orphan. Surface the boolean from deleteStateMapping and throw from deleteFlowAndStateMapping before touching the flow, so the caller's allSettled warn branch fires and the next replacement retries both. * fix: restore the state mapping when the flow delete fails (Codex P2) The inverse partial failure of the round-3 fix: a successful mapping delete followed by a silently failed flow delete left a PENDING flow whose reused authorization URL could never resolve, dead-ending every callback in invalid_state until the flow went stale. Check deleteFlow's result, re-store the mapping on failure, and throw so the caller's allSettled warn branch fires. * fix: never leave a callback-capable flow behind on uninstall (Codex round 6) Teardown runs after the server's tokens are deleted, so a preserved flow+mapping pair (the round-3 early-throw path) let a lingering consent tab complete the callback and recreate credentials post-uninstall. Now that both callback branches gate on stored-state equality, an orphaned mapping is the benign failure mode, so invert the order: delete the flow first, attempt the mapping delete regardless, and reject when either reports a storage failure. This supersedes the round-4 mapping restore, which also preserved a callback-capable pair. * fix: delete the flow even when its metadata read fails (Codex round 7) A storage error on the initial getFlowState aborted teardown before any delete ran, preserving the callback-capable flow after token deletion. Tolerate the read failure, delete the flow blindly, skip the mapping it could not identify (the callback gates neutralize the possible orphan), and reject so the caller's warn branch fires. |
||
Jens Schumann
|
ad74a282d1
|
🪃 fix: Resolve User Vars Before the First Post-OAuth Reconnect (#14538)
* fix: resolve customUserVars before first post-OAuth-callback MCP reconnect
The OAuth callback route reconnects the user's MCP connection immediately
after storing new tokens, but never resolves customUserVars before doing
so - unlike the /reinitialize route a few hundred lines below, which does.
As a result, headers/oauth_headers templates like `{{MY_KEY}}` are sent
to the MCP server literally, unsubstituted, on this first connection
attempt, even though the user's value is already saved. The upstream
server rejects it as an invalid credential.
Fixes #14537
* refactor: share getServerCustomUserVars reader from @librechat/api
The mcp_-prefixed key shape was built by getUserMCPAuthMap but re-derived
by hand at each read site (a private helper in services/MCP.js, and the
new callback-route extraction). Export a reader from the same module that
owns the writer and reuse it at both sites, so the key shape has a single
source of truth.
* chore: sort destructured require members in routes/mcp.js
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
Danny Avila
|
8af6414e13
|
🪟 fix: Surface MCP Initialization Errors (#14529) | ||
|
Dustin Healy
|
af795be0c2
|
🪢 feat: Langfuse Fanout Connection Setting (#14108)
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: encrypt tenant Langfuse secret in admin config Add generic per-field secret encryption to the admin config layer: registered secret paths (langfuse.secretKey) are encrypted with encryptV3 on write and a non-secret fingerprint companion is stored. Admin config reads (base + per principal) redact registered secrets so they are never returned; the fingerprint is kept so the UI can show which key is configured. The Langfuse fanout read path decrypts the tenant secret before export. Adds secretKeyFingerprint to langfuseConfigSchema and tests for the encrypt/redact policy. * fix(api): secure admin config secret handling * fix(api): preserve encrypted langfuse config secrets * fix(api): couple config secret fingerprint deletion * fix(api): read langfuse fanout collector url from env * fix(api): display langfuse secret key hint * fix(api): remove langfuse secret fingerprint breadcrumbs * fix(api): use langfuse destination keys for tenant config * fix(api): remove langfuse config compatibility fallbacks * refactor(api): simplify langfuse secret helpers * refactor(api): simplify langfuse config secret handling * feat: in-app Langfuse connection settings panel Add a discoverable, admin-gated Langfuse connection panel inside LibreChat Settings (Dify-style): enable toggle, host, public key, masked write-only secret, configured-key fingerprint, and a test-connection action. Backed by a dedicated /api/admin/langfuse/connection endpoint that encrypts the secret at rest, returns metadata plus fingerprint on read, and validates credentials. Builds on the per-field encryption and fanout decrypt from the langfuse-config-encryption branch. * refactor: align Langfuse secret field to CustomUserVars pattern Use the established SecretInput plus Set/Unset state pill (com_ui_set/com_ui_unset) from the MCP CustomUserVars UI for the saved-secret state, instead of a bespoke masked input. * fix: drop em dash from saved-secret placeholder * feat: show loading state on Langfuse test connection button * feat: gate in-app Langfuse settings on fanout config and admin role * test: align Langfuse connection spec with SecretInput refactor * feat(langfuse): refine tenant connection controls * fix(admin): refine Langfuse connection verification * fix(langfuse): refine tenant connection settings * fix(langfuse): simplify export enablement controls * fix(langfuse): validate tenant export configuration * fix(langfuse): align startup fanout gate * fix(admin): time out Langfuse verification * fix(ui): rename Langfuse connection setting * fix(admin): enforce Langfuse config capability * feat(langfuse): require explicit tenant export activation * feat(langfuse): support single-tenant connection settings * fix(i18n): remove obsolete integrations label * fix(langfuse): authenticate ingestion verification * fix(langfuse): validate public key independently * fix(langfuse): localize connection errors * perf(config): skip Langfuse checks for non-admins * fix(langfuse): preserve trace sampling for feedback * test(langfuse): fix feedback sampling fixture * fix(langfuse): align secret preview field * fix(langfuse): harden connection settings state * fix(langfuse): preserve trace destination state * fix(langfuse): enforce tenant-wide routing invariants * fix(langfuse): preserve verified connection invariants * fix(langfuse): preserve stable project identity * fix(langfuse): warm project identity asynchronously --------- Co-authored-by: Ravi Kumar L <ravi.lazar@clickhouse.com> Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Ravi Kumar L
|
f4723220cc
|
🛡️ fix: validate message feedback payloads (#14500)
* fix: validate message feedback payloads * fix: enforce feedback rating tag consistency |
||
|
Ravi Kumar L
|
44d1275f36
|
⚙️ perf: reduce first-load MongoDB round trips (#14101)
* perf(api): reduce first-load database round trips * docs: move agent guidance to claude docs * refactor(api): move message validation into api package * fix(api): narrow active generation job lookup * fix(api): preserve omitted source identity |
||
|
Danny Avila
|
84fa6aa820
|
🧹 feat: Eager HITL Checkpoint Cleanup (Expiry + Deletion) & Full-Wiring E2E (#14123)
* feat: eager HITL checkpoint cleanup on expiry + deletion, full-wiring e2e Follow-up to the lazy checkpointer (#14024): two paths still left a paused run's durable checkpoint to the 24h Mongo TTL, and no test exercised the whole HITL seam with real components. 1. Approval expiry: GenerationJobManager.setApprovalExpiredHandler(fn) — a non-destructive host hook fired after expireApproval's CAS succeeds (periodic sweeper AND stale-submit path), safe on startups that run constructor defaults. Both startups (index.js configureGenerationStreams, experimental.js) register a handler that prunes the checkpoint, resolving config lazily per expiry (streamId === conversationId === thread_id). 2. Conversation deletion: deleteConvos now returns the deleted conversationIds; the three deletion paths (DELETE /convos, DELETE /convos/all, account deletion) prune them via the new bulk deleteAgentCheckpoints (one $in deleteMany per collection). The delete routes gain configMiddleware for the checkpointer config. 3. Full-wiring e2e (hitlCheckpoint.e2e.spec.js): real SDK Run driven by FakeChatModel calling a gated tool, real PreToolUse/humanInTheLoop wiring, real LazyMongoSaver over mongodb-memory-server, real GenerationJobManager, real /resume controller via supertest. Asserts: clean turn persists nothing; error turn persists nothing; pause -> HTTP approve -> gated tool executes exactly once -> finalize prunes the checkpoint; expiry prunes the abandoned pause eagerly. Tests: 3 handler unit tests (pendingAction.spec), 2 bulk-prune integration tests (checkpointer.integration.spec), convos route + deleteUser specs updated, 4 e2e scenarios. 207 tests green across changed areas. * fix: tenant-scoped expiry prune, store-won expiry relay, resilient deleteConvos ids Codex round 1 on #14123 — all three valid: 1. The approval-expired handler now receives the expired JOB so both startups resolve config in the paused job's tenant/user scope (getAppConfig({userId, tenantId})) — a tenant checkpointer override no longer sends the prune to the base config's collections. expireApproval fetches the job best-effort. 2. Multi-replica: when RedisJobStore.cleanupRequiresActionIndex wins the expiry CAS on another replica, this replica's sweeper relay branch now runs the approval-expired cleanup too (prune is idempotent) — store-driven expiry no longer bypasses the hook. 3. deleteConvos: post-delete cleanup (deleteMessages, project stats refresh) is now best-effort — the conversations are already gone, so throwing hid the deletion and dropped the conversationIds the checkpoint prune needs, unrecoverable on retry. Updated the existing tag-decrement-on-failure test to the new contract (ids still returned). Tests: handler-receives-job, store-won relay path, ids-survive-cleanup-failure. 134 tests green across changed suites. * fix: relay cleanup independent of cached errorEvent; enter tenant ALS context Codex round 2 on #14123: 1. The sweeper's relay branch gated BOTH the terminal-error emit and the new checkpoint cleanup on !runtime.errorEvent — but a reconnect seeds errorEvent from the aborted job (runtime-state creation), which then suppressed the cleanup entirely. The emit stays gated; the idempotent cleanup now runs independent of the cached error, once per runtime lifetime (approvalCleanupRan flag — the aborted job is swept repeatedly). 2. Passing userId/tenantId to getAppConfig only keys the config cache; the Config query is ALS-scoped by the tenant-isolation plugin. Both startup handlers now ENTER the paused job's tenant context via tenantStorage.run before resolving config + pruning, so a tenant checkpointer override is honored in strict and non-strict modes. Tests: relay-cleanup-with-cached-error (reconnect simulation), repeated sweeps run the cleanup once. 27+4 green. * fix: dedup expiry cleanup across winner and relay paths Codex round 3 (P3): expireApproval ran the handler without marking the runtime's approvalCleanupRan flag, so the next sweep's relay branch (the aborted job outlives expiry for the completed-job TTL) ran the cleanup a second time. The dedup now lives inside runApprovalExpiredHandler — the single choke point both paths call — set-before-run, once per runtime lifetime. Test: local expiry followed by a sweep fires the handler once. |
||
|
Danny Avila
|
b6c0bc7c0d
|
⚡ perf: Skip Role Route Capability Probe for Own and Default Roles (#14073) | ||
|
Ravi Kumar L
|
a0529c9af7
|
🪭 feat: Add opt-in Langfuse fanout gateway + collector (#13872)
* feat: add opt-in Langfuse fanout collector * feat: fan out Langfuse feedback scores * docs: prepare Langfuse fanout for OSS setup * fix: clarify Langfuse fanout collector config * test: stabilize librechat suite * test: fix upload dialog import order * fix: omit empty Langfuse tenant fields * fix: gate tenant Langfuse fanout * test: cover central Langfuse env fallback * style: format Langfuse fanout config * feat: route langfuse fanout by destination * docs: clarify langfuse compose destination scope * test: remove unrelated suite stabilization * style: sort agent imports * fix: treat blank tenant fanout toggle as disabled * fix: rename tenant fanout emergency toggle * test: guard langfuse fanout collector config drift * feat: tune langfuse fanout batching * test: render fanout helm tests without dependencies * fix: narrow remote agent run config * refactor: share string normalization helper * fix: align langfuse fanout env parsing * fix(langfuse): align score fanout toggles with traces * fix(langfuse): keep central fanout config collector-only * fix(langfuse): type fanout collector config * fix(langfuse): harden tenant fanout config * feat(langfuse): support media fanout gateway * fix(langfuse): route tenant fanout through destination URL * fix(langfuse): harden fanout routing checks * ci(langfuse): test fanout gateway changes * ci(langfuse): check fanout go formatting * fix(langfuse): satisfy api typecheck |
||
|
Marco Beretta
|
61016e328a
|
🔄 feat: Continue Shared Conversations as Personal Copies (#13714)
Adds a "Continue this chat" button to the shared conversation view that forks the shared conversation into a new conversation owned by the viewer and opens it to continue (issue #13001). - POST /api/share/:shareId/fork, gated by requireJwtAuth, the fork rate limiters, and the canAccessSharedLink ACL (view access = fork access). - forkSharedConversation clones from the anonymized getSharedMessages payload, so only share-visible data is copied. - Strips file ids from cloned files/attachments so a fork grants no more file access than viewing the read-only share, and honors the global shared-file kill switch via the snapshotFiles option. - Reduces the clone to the viewer's active branch, located by its index in the shared payload (shared ids are re-anonymized per request and createdAt can collide, while the payload order is stable). - Resolves config/retention, persists, and reads back under the requesting user's tenant, not the share owner's; canAccessSharedLink also falls back to a system-wide share lookup so cross-tenant public shares resolve (ACL still enforced under the share's own tenant). - Resolves a usable endpoint/model from the viewer's models config instead of hard-coding OpenAI, so deployments without OpenAI can send the first message. - Routes the fork's 401s (logged-out or cold-loaded viewers) through login, including when the refresh itself is rejected for a stale session. - Hides the Temporary Chat toggle once a conversation has a real id, and portals the share-settings theme/language dropdowns above the dialog. Rebased onto dev; collapses the share-fork feature and its review fixes into a single commit. |
||
|
Danny Avila
|
e807c63d5d
|
🔐 fix: Gate Shared Startup Config By Link Access (#13897)
* fix: gate shared startup config by link access * fix: satisfy shared config CI checks * fix: align shared config client types * fix: reject expired shared link access |
||
|
Danny Avila
|
e515063ffe
|
🔗 feat: Snapshot Files for Shared-Link Attachments (#13740)
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: Snapshot Files for Shared-Link Attachments Shared-link viewers could read a shared conversation snapshot but not its attachments: file preview/download still went through the owner-scoped file ACL (the /api/files router sits behind requireJwtAuth + owner/agent checks), so anonymous viewers got 401s and authenticated non-owners got 403s — the repeated `[fileAccess] denied` warnings seen for the preview poller. Capture an immutable per-share file snapshot (embedded on the SharedLink document, referencing the original stored object — no byte copy) at share create/update, and serve those files through new share-scoped routes authorized by the existing shared-link view permission (public/ACL) plus snapshot membership, never the owner's live file ACL. - data-schemas: fileSnapshots on the share doc; capture in create/update; read-time rewrite of filepath/preview to /api/share/:id/files/:fileId; getSharedLinkFile + lazy backfillSharedLinkFiles for legacy links - api: GET /api/share/:shareId/files/:file_id[/download|/preview]; route context added to fileAccess denial logs - packages/api: isFileSnapshotEnabled resolver (env + yaml) - data-provider: interface.sharedLinks.snapshotFiles (default on) + client endpoints/services - client: ShareContext.shareId wired to Image, preview hook, and downloads - config: SHARED_LINKS_SNAPSHOT_FILES env override (default on) * 🔒 fix: Address Codex review on shared-link file snapshots Triage of the Codex review on PR #13740 (2 P1, 7 P2 — all valid): - P1 (cross-user access): scope the snapshot lookup to the sharing user's own files so a message referencing another user's file_id can't widen access. - P1 (stored XSS): the inline share-file route now serves only safe preview types inline (raster images/pdf); everything else is forced to attachment with X-Content-Type-Options: nosniff. - Stream shared downloads by default; redirect to a signed URL only on ?direct=true (blob/XHR callers work without bucket CORS). - Read preview status live from the file record (always current for deferred previews) and stop embedding extracted text in the share doc (16MB-limit risk). - Only lazily backfill when the fileSnapshots field is absent (legacy), not on every snapshot miss. - Backfill legacy shares before rewriting message URLs, and gate URL rewriting to public shares so non-public (ACL) shares keep prior behavior (img/anchor can't carry the bearer token). - Frontend: only route a download through the share path when the file was actually snapshotted (rewritten href / filepath), else fall back. * 🔑 feat: Authorize shared-link files for non-public shares via cookie Extends shared-link file access to non-public (ACL) shares (Codex finding 5). `<img>`/anchor requests can't carry the bearer access token, so non-public shares previously 401'd on file loads. Add an optional cookie-auth fallback on the share file routes that resolves the viewer from the `refreshToken` cookie (or signed `openid_user_id` cookie) — the same mechanism secure image links use (validateImageRequest) — then let canAccessSharedLink run the viewer's ACL check. - new middleware optionalShareFileAuth (+ unit spec); applied to the three share file routes after optionalJwtAuth - URL rewriting in getSharedMessages is no longer gated to public shares (the route now authorizes header-less requests), so files work uniformly across public and non-public shares; revert the now-unused req.sharePublic plumbing * 🔒 fix: Second Codex pass on shared-link file snapshots Addresses the follow-up Codex findings on PR #13740: - Don't snapshot transient text-source files: FileSources.text filepaths are Multer temp paths the upload route deletes, so they can't be streamed — removed from the streamable allowlist. - Unset stale snapshots on a disabled-feature update: updateSharedLink now $unsets fileSnapshots when snapshotFiles is false, so an opted-out update can't keep serving file ids the update dropped. - Load tenant config after share resolution: configMiddleware now runs after canAccessSharedLink (which enters the share's tenant ALS context), so per-tenant interface.sharedLinks.snapshotFiles overrides apply to anonymous public views. - Return a clean 404 when the snapshotted object is gone: resolveShareFile now requires the live file record and 404s if it's been deleted/expired, instead of letting the stream error after headers are sent (ENOENT / 500). (The re-flagged P1 about private-viewer rewriting was already fixed in the prior commit's cookie-auth change.) * 🔒 fix: Third Codex pass on shared-link file snapshots Addresses the third Codex review pass on PR #13740: - P1: keep shared previews/files pinned to the snapshotted version. Snapshot the small previewRevision; resolveShareFile 404s when the live file's revision no longer matches (file_id reused/overwritten by a later turn), so old links can't surface post-share content — covers both preview text and streamed bytes. - Honor the toggle as a kill switch: resolveShareFile 404s when snapshotFiles is disabled, instead of only skipping backfill, so disabling stops serving already-snapshotted file URLs. - Lazy-sweep orphaned 'pending' previews to 'failed' in the share preview route (mirrors the owner route) so the client poller reaches a terminal state. - Resolve the cookie-fallback user in runAsSystem so strict tenant isolation doesn't throw before canAccessSharedLink establishes the share tenant context. * ✨ feat: Per-link "share files" checkbox for shared links Add a checkbox to the share-link dialog (checked by default) letting the user choose whether to include the conversation's files in the shared link, with copy explaining images/files won't be visible to viewers otherwise. Opting out skips snapshot creation/serving for that link. - client: ShareButton renders the checkbox gated on the new startupConfig.sharedLinksSnapshotFilesEnabled flag; state threads through SharedLinkButton into the create/update mutations as `snapshotFiles`. - data-provider: createSharedLink/updateSharedLink send `snapshotFiles` in the body; TStartupConfig gains `sharedLinksSnapshotFilesEnabled`. - api: POST/PATCH /api/share compute snapshotFiles as isFileSnapshotEnabled(req.config) && body.snapshotFiles !== false (admin gate AND per-link opt-out); config.js exposes the effective enabled flag to clients. - en locale: com_ui_share_files (+ _description). * 🐛 fix: Make the "share files" opt-out actually hide files Unchecking "share files" at creation didn't hide anything: the shared message JSON still carried each file's original (e.g. static-served) path, and because opting out only meant "no fileSnapshots field" — indistinguishable from a legacy link — getSharedMessages would backfill snapshots on first view whenever the admin feature was on, re-enabling files entirely. Fix by persisting and honoring the per-link choice: - Store `snapshotFiles` (boolean) on the SharedLink so opt-out is distinct from a legacy link; set it on create and update. - getSharedMessages computes includeFiles = adminEnabled && link not opted out; when excluded it strips files/attachments from the payload (no original-path leak) and never backfills the opted-out link. - Surface the stored choice via getSharedLink so the dialog checkbox reflects an existing link's actual setting instead of always defaulting to checked. Note: changing the checkbox on an already-created link still applies only when the link is refreshed (which regenerates the URL) — a UX follow-up. * 🔒 fix: Close remaining shared-link file opt-out leaks (Codex) Follow-up to the per-link opt-out, addressing the third Codex pass: - Honor the opt-out on the file route too: getSharedLinkFile now returns the link's `optedOut` choice; resolveShareFile 404s (and never backfills) an opted-out link, so a direct /files/:id request can't re-create snapshots. - Make read/serve viewer-independent: the gate no longer uses the viewer's resolved config (isFileSnapshotEnabled(req.config)) — it uses the link's stored choice plus a global env-only kill switch (isFileSnapshotKillSwitchActive). A viewer's own interface.sharedLinks.snapshotFiles can no longer hide a link's files. Create/update still use the creator's config to set the per-link choice. - Neutralize render URLs for non-snapshotted files: applyShareFileRoute now strips filepath/preview for any file/attachment not in the snapshot, so the owner's original (e.g. static) path can't be loaded through the share. * 🔒 fix: Harden shared-file version pinning and local path handling (Codex) - Refuse reused/overwritten file snapshots more broadly: resolveShareFile now refuses to serve when either previewRevision OR `bytes` changed vs the snapshot. `bytes` catches non-office reused outputs (e.g. code-exec same-filename images that lack previewRevision) and is stable across S3 URL refresh and the pending->ready transition. Same-size content swaps remain a best-effort gap inherent to the no-byte-copy design. - Strip cache-busting query strings before local streaming: code-output images add `?v=...` to filepath; the share route now splits it off so getLocalFileStream resolves the real filename instead of a literal `*.png?v=...` path. * 💬 fix: Clarify that file-sharing changes apply on link refresh For an already-created shared link, changing the "share files" checkbox only takes effect when the link is refreshed (which regenerates the snapshot). Add a note under the checkbox, shown only when a link already exists, so the behavior isn't surprising: "Refresh the link to apply this change — files are snapshotted when the link is refreshed." |
||
Dan Lew
|
743f57f63e
|
🔖 feat: Add Pinned Conversations (#13492)
* feat: add `convo.pinned` We want to be able to pin convos (so users can easily find them), thus we added a new field to the DB schema: `pinned`. We also had to add an API method for pinning a convo. It's got thorough tests. It's structured just like how /api/convos/archive works, only for pinning. * feat: add 'pinned' section to conversation list If there are any pinned conversations, they will appear above the normal "chats" list, with a pinned icon next to them. * feat: added pin/unpin to convo options ConvoOptions now has a pin/unpin button which lets you change the pin status of any given conversation. * fix: adjust ellipsizing gradient on ConvoLink Because it went across the whole ConvoLink, it would cover up any children (i.e. icons) that appear after the title. However, the point of the gradient is just to gradually make the title disappear, not the icons. This change places the gradient on the title only, so it achieves the same ellipsizing effect without interfering with the display of the child icons. * Fixed import sorting |
||
|
Danny Avila
|
49f4b659f6
|
🔐 fix: Honor Admin-Panel MCP Allowlist Overrides Without Restart (#13814)
* 🔐 fix: Honor Admin-Panel MCP Allowlist Overrides Without Restart MCPServersRegistry was built once at boot from getAppConfig({ baseOnly: true }), freezing allowedDomains/allowedAddresses to YAML. Admin-panel mcpSettings overrides were ignored by both inspection (addServer/ reinspectServer/updateServer/lazyInitConfigServer) and runtime connection enforcement (assertResolvedRuntimeConfigAllowed), so a domain allowed only via the panel failed inspection and never connected. Make the registry's effective allowlists mutable and refresh them from the merged admin-panel config: seed at boot, and re-apply on every config mutation via invalidateConfigCaches -> clearMcpConfigCache. Both inspection and connection paths read the same getters, so both honor overrides without a restart. Fail-safe: current allowlists are preserved when the merged read fails. * 🛡️ fix: Scope MCP allowlist refresh to global config, fail-safe on DB error Address Codex P1 review findings on the allowlist-refresh path: - Tenant-scoped config mutations no longer push one tenant's merged mcpSettings into the process-wide registry singleton (read by all MCP connection paths), which would leak allowlists across tenants. Only global (non-tenant) mutations refresh the registry; tenant mutations still evict the config-server cache. - The refresh read now uses strictOverrides:true so a transient DB error throws instead of silently returning YAML base config — preserving the last-known allowlists rather than overwriting them with fallback values. Adds the strictOverrides option to getAppConfig (default off, no behavior change for existing callers). * ♻️ refactor: Resolve MCP allowlists per-request (tenant-scoped) instead of a global singleton Supersedes the prior global-mutation approach. MCP allowlists live in mcpSettings, which is tenant/principal-scoped admin config, so a process-wide singleton value is the wrong model — it caused cross-tenant bleed and stale reads. Instead, inject a resolver (from the app layer, where the merged config lives) that the registry calls per inspection and per connection. It reads the ALS tenant context via getAppConfig and accepts the acting user so user/role-scoped overrides resolve; config-source inspection (no user) resolves at tenant scope. Falls back to the YAML base allowlists when no resolver is set or the lookup fails, so a transient error fails to the operator baseline rather than disabling the allowlist. Removes the now-unnecessary setAllowlists / boot-seed / invalidateConfigCaches refresh / getAppConfig.strictOverrides machinery. * 🔒 fix: Scope config-source cache by allowlist; resolve OAuth allowlists per-request Address Codex review of the per-request resolver: - Config-source cache key now folds in the resolved allowlists, not just the raw-config hash. Inspection results became allowlist-dependent, so without this a tenant whose allowlist rejects a URL could poison the shared key with an inspectionFailed stub for a tenant that allows it (and vice versa). The tenant-scoped allowlist is resolved once per ensureConfigServers pass and threaded through the cache key + inspection. - The two remaining request-time OAuth allowlist reads now use the merged config instead of the YAML base getters: the fallback OAuth-initiate path (routes/mcp.js) via resolveAllowlists, and OAuth revocation (UserController.maybeUninstallOAuthMCP) via the request's already-merged appConfig.mcpSettings. Without this, an OAuth endpoint allowed only by an admin-panel override was rejected while inspection/connection allowed it. * ✅ test: Update MCP OAuth registry/config mocks for per-request allowlists CI fix for the Finding-12 change. The OAuth-initiate route now calls registry.resolveAllowlists() and the revocation path reads the merged appConfig.mcpSettings, so the affected specs' mocks were asserting the old base-getter values: - routes/__tests__/mcp.spec.js: add resolveAllowlists to the registry mock. - UserController.mcpOAuth.spec.js: provide mcpSettings on the getAppConfig mock so revokeOAuthToken still receives the expected allowlists. * 🧪 test: e2e proof that admin-panel MCP allowlist override takes effect Adds a Playwright mock-harness spec for #13809. A URL-based MCP fixture (e2e-http, streamable-http SDK server) boots inspectionFailed because its origin is omitted from the YAML mcpSettings.allowedDomains; the spec adds that origin via an admin config override (PUT /api/admin/config/user/:id) and asserts the server reinitializes — exercising the real resolver path through the backend + DB. Before the fix, reinspection used the frozen YAML allowlist and the server stayed unreachable. - e2e/setup/fake-mcp-http-server.js: streamable-HTTP MCP fixture (health GET /). - e2e/playwright.config.mock.ts: boot the fixture as a second webServer. - e2e/config/librechat.e2e.yaml: mcpSettings.allowedDomains (excludes 127.0.0.1) + the e2e-http server. - e2e/specs/mock/mcp-allowlist-override.spec.ts: login → baseline reinit fails → apply override → reinit succeeds. |
||
|
Dustin Healy
|
054fa4bfa7
|
🥽 fix: Restrict MCP Server URL Disclosure to Admins, Owners, and Editors (#13784)
* 🥽 fix: Redact Non-User-Sourced MCP Server URLs by ACL Edit Permission GET /api/mcp/servers and GET /api/mcp/servers/:serverName return MCP server configs to any caller with MCP-use permission. For user-sourced configs (DB-stored, UI-submitted), the URL is the caller's own and is intentionally disclosed. For non-user-sourced configs (YAML or config-tier, operator-defined), the URL and OAuth flow endpoints (authorization_url, token_url) are operator-sensitive: they can encode internal infrastructure hostnames and are not editable through the API. This change redacts those fields on non-user-sourced configs unless the caller has edit authority on the resource, using the same ACL check (PermissionBits.EDIT) that the PATCH and DELETE routes already enforce via canAccessMCPServerResource. Callers with broad MANAGE_MCP_SERVERS capability bypass the per-resource check, matching the existing capability bypass in canAccessResource. customUserVars is intentionally not redacted: its values are UI hint metadata (title, description, sensitive), not user-supplied secrets; blanking it would give non-editor callers a Configure form with no field labels. * 🥽 fix: Correct getResourcePermissionsMap import path + tighten redact comments The MCP server redaction commit imported getResourcePermissionsMap from ~/server/controllers/PermissionsController, but that controller is a consumer of the helper, not its exporter. The canonical export lives in ~/server/services/PermissionService (which controllers/agents/v1.js already imports from). Fixes the runtime getResourcePermissionsMap is not a function failure on GET /api/mcp/servers and the four downstream route-spec failures whose config mocks lacked a source field and were therefore wrongly treated as non-user-sourced; mocks now reflect the real registry behavior (addServer/updateServer tag DB-stored configs with source: 'user'). Trims narrating JSDoc on the redact helpers and resorts the librechat-data-provider destructure by length. * chore: import order * 🥽 fix: Redact OAuth Revocation Endpoint Alongside Authorization And Token URLs The OAuth-URL strip path only dropped authorization_url and token_url. The UserOAuthOptionsSchema in packages/data-provider/src/mcp.ts (line 146) accepts revocation_endpoint as another operator-configurable URL, and the OAuth handler uses it to revoke tokens; it can hold the same internal IdP hostnames the existing strip is trying to hide. Adds revocation_endpoint to the destructure so a non-user-sourced YAML/config MCP server config no longer leaks the revocation URL to non-editor callers. The existing strip url and oauth flow URLs spec is extended with a revocation_endpoint value to lock in the new field. * 🥽 fix: Gate Shared DB Server URL Disclosure On ACL Edit Permission source-driven URL disclosure was incorrect for shared DB-backed MCP servers. ServerConfigsDB.mapDBServerToParsedConfig (packages/api/src/mcp/registry/db/ServerConfigsDB.ts:465) sets source: 'user' on every DB-stored config it returns, regardless of who is accessing it. A user with only VIEW share on a DB server, or with agent-mediated access, was therefore treated by the redaction layer as if they owned the URL, and GET /api/mcp/servers disclosed the owner's URL and OAuth flow URLs to viewers who could not edit the resource. The redaction is now driven purely by ACL edit authority: computeCanEditByServer routes every dbId-bearing config through PermissionBits.EDIT regardless of source; redactServerSecrets strips on !canEdit regardless of source. POST and PATCH controllers explicitly pass canEdit: true since both endpoints establish edit authority (POST creates the resource, PATCH is gated on the EDIT middleware). Legacy/ephemeral configs without a dbId still fall back to the source heuristic. * 📝 docs: correct redactServerSecrets URL-disclosure comment --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Ravi Kumar L
|
fbc990f684
|
📈 fix: Isolate RUM Telemetry Proxy Auth from App Auth (#13765)
* fix(rum): isolate telemetry proxy auth * feat(rum): track proxy error metrics * refactor(rum): simplify proxy auth strategy flow * test(rum): clarify proxy success metric assertion * test(metrics): use typed supertest import * test(metrics): add local supertest types * test(metrics): keep supertest types local * test(metrics): use official supertest types * fix(rum): log proxy auth strategy errors * fix(rum): classify proxy auth errors in metrics * style(rum): sort telemetry metric imports * ci: mention import sort check command * ci: show targeted import sort example |
||
|
Danny Avila
|
49859c04a2
|
🗄️ fix: Gate Request-Scoped MCP Servers Out of Persistent Tool Cache (#13672)
* 🗄️ fix: Gate Request-Scoped MCP Servers Out of Persistent Tool Cache PR #13626 established that request-scoped MCP servers (runtime OPENID/GRAPH/BODY placeholders) must not use the persistent 12h tool cache, but only gated three of five touchpoints. The panel endpoint still back-filled the cache and the OAuth callback still wrote to it, while agent loading read those entries ungated — pinning ephemeral model-spec/agent toolsets to stale definitions for up to 12h. Centralize the invariant in createMCPToolCacheService: a getServerConfig resolver dep gates both writers and a new service-owned getMCPServerTools read, so every current and future caller is covered. Callers that already hold the parsed config pass it to skip resolution; the per-call skipCache flag and duplicated call-site gates are removed in favor of the single config-based mechanism. Resolution failures fail open to preserve prior behavior. * 🩹 fix: Address Codex Review on Cache Gating - Repair getCachedTools.spec.js, which destructured the relocated getMCPServerTools directly from the module; its coverage now lives in the service-level tools.spec.ts. - Resolve the merged (Config-tier-aware) server config in the OAuth callback before writing tool definitions, so the cache gate detects request-scoped servers supplied via admin Config overlays that the base registry lookup cannot see. - Discover tools actively for request-scoped servers in the panel endpoint via ephemeral reinitialization: such servers have no stored app/user connections, so the previous getServerToolFunctions fallback returned an empty toolset once the cache read was gated. * 🧵 fix: Address Second Codex Review on Cache Gating - Resolve the merged server config before the OAuth callback reconnects, so the connection itself uses Config-tier overlays rather than only the subsequent cache write. - Pass Config-tier candidates into the panel's request-scoped discovery, matching the reinitialize route: reinitMCPServer forwards configServers (not the provided serverConfig) to its OAuth discovery fallback. - Document the accepted read-path trade-off: the gate resolver sees base configs only, all writers pass merged configs, so a pre-gating or overlay-divergent entry survives at most one cache TTL. * 🚏 chore: Rework Cache Gating for BODY-Only Request Scoping After #13673 narrowed requiresEphemeralUserConnection to BODY placeholders, the central gate follows the predicate unchanged, but the panel's active discovery no longer serves a purpose: the only remaining request-scoped class cannot connect outside a chat turn, so the reinitialization attempt would always fail at the missing-body check. Remove that path; OpenID/Graph servers are persistent user-scoped again and flow through the stored-connection and cache lookups as before. Flip test fixtures that used OPENID placeholders to denote request-scoped configs over to BODY placeholders. * 🪟 fix: Check Config Overlays in Agent-Loading Cache Reads The cache service's registry resolver sees only base YAML/DB configs, so a BODY placeholder introduced by a request-tier Config overlay was invisible to the gate on the agent-loading read path: model-spec and ephemeral-agent expansion could read a leftover persistent entry and pin stale concrete tool names instead of the mcp_all fresh-discovery path. Check the raw overlay candidate inline in loadEphemeralAgent and loadAddedAgent — a pure placeholder scan with no extra IO — and skip the cache read when the overlay makes the server request-scoped. Widen UserScopedConnectionConfig so raw (pre-inspection) configs qualify for the scoping predicates, which only check key presence. * 🧪 test: Guard Run-Scoped MCP Definition Handoff Boundaries The original ClickHouse breaker storm regressed precisely at field pass-through boundaries that unit tests of each end could not see: initializeAgent dropping mcpAvailableTools from its destructure, and the agent tool context losing it on the way into ON_TOOL_EXECUTE. Add direct guards on both hops: the loadTools result must surface on the initialized agent, and the captured toolExecuteOptions closure must forward it to loadToolsForExecution. |
||
|
Danny Avila
|
c27d6b85a4
|
🤫 refactor: Silent MCP OAuth Refresh on Mid-Session 401 (#13369)
* 🤫 fix: Silent MCP OAuth Refresh on Mid-Session 401 Avoids the hourly interactive re-auth prompt when an MCP server (e.g. Azure Entra ID) returns 401 mid-session by attempting a refresh token exchange first, and only falling back to the interactive OAuth flow when no refresh token is stored or the refresh server rejects it. Resolves #13364. * fix: Use distinct flow type for silent token refresh to avoid cache hit Addresses the Codex review on PR #13369: `attemptSilentTokenRefresh` was reusing the `'mcp_get_tokens'` flow type, so `FlowStateManager.createFlowWithHandler` would short-circuit and return the same tokens cached by an earlier `getOAuthTokens` call — the very tokens the server just rejected — without executing the forced-refresh handler. Switch silent refresh to the distinct `'mcp_force_refresh_tokens'` flow type so coalescing still works but stale `mcp_get_tokens` cache entries are not reused. After a successful refresh, invalidate the `mcp_get_tokens` flow cache so the next `getOAuthTokens` call reads the freshly persisted tokens from storage rather than the stale cached value. Add a regression test that simulates the real `FlowStateManager.createFlowWithHandler` cache-hit behavior for `mcp_get_tokens` and verifies the silent refresh handler still runs and returns the freshly refreshed tokens. * fix: Address Codex round-2 review on silent MCP OAuth refresh Three follow-up findings from Codex on PR #13369: 1. The new `mcp_force_refresh_tokens` flow type was itself cached by `FlowStateManager.createFlowWithHandler`, so a subsequent 401 within the refreshed token's `expires_at` could re-serve the just-rejected token without ever re-running the refresh handler. 2. The factory's `oauthRequired` listener was removed immediately after the initial `attemptToConnect` succeeded, so a real mid-session 401 emitted by `MCPConnection.connectClient` during transport recovery had no listener — the OAuth handled-promise would simply time out instead of triggering the silent refresh. 3. Routing the silent refresh through a distinct flow type broke coalescing with the `mcp_get_tokens` lock used by `getOAuthTokens`, letting two paths concurrently redeem the same stored refresh token. For providers that rotate refresh tokens (e.g. Azure Entra) the second redemption is rejected, kicking the user back into interactive OAuth despite a successful refresh elsewhere. Resolution: - Drop `FlowStateManager` from the silent-refresh path entirely. Replace with a process-local `inflightSilentRefreshes` Map keyed by `userId:serverName` that holds only the in-flight Promise (no cached result), so every fresh 401 after settlement triggers a fresh redemption while concurrent 401s for the same user/server still share one redemption. - Stop calling `cleanupOAuthHandlers()` on successful initial connect, keeping the OAuth handler attached for the connection's lifetime so mid-session 401s actually reach `attemptSilentTokenRefresh`. - Add a regression test reproducing the stale-cache scenario by faking the `mcp_get_tokens` cache hit and asserting silent refresh still runs against storage and returns the fresh tokens. - Add a coalescing test asserting two concurrent oauthRequired events for the same user/server result in a single `forceRefreshTokens` call. - Clear `inflightSilentRefreshes` in `beforeEach` to prevent cross-test leakage; switch the silent-refresh test mocks to `mockResolvedValueOnce` / `mockImplementationOnce` so leftover mock state cannot leak into later test cases. Acknowledged remaining gap: the silent refresh still races `getOAuthTokens`'s `mcp_get_tokens` flow when both run concurrently (narrow window when an existing connection's local `expires_at` is still valid but the server invalidated the token, and a new connection is being created in parallel). The race is self-healing on the next 401 and documented inline. * fix: Address Codex round-3 review on silent MCP OAuth refresh Three more findings from Codex on PR #13369: 1. The in-flight silent-refresh promise was unbounded. If `forceRefreshTokens()` ever hung (slow provider, dropped TCP), the `inflightSilentRefreshes` lock stayed occupied forever and every later 401 for the same user/server joined the stuck promise instead of starting a fresh attempt or falling back to interactive OAuth. 2. The interactive-OAuth fallback didn't invalidate the `mcp_get_tokens` flow cache after persisting fresh tokens. For providers that don't issue refresh tokens (so silent refresh returns null), the old cache could still feed stale access tokens to the next `getOAuthTokens` call until its TTL expired — causing an immediate reconnect with the same just-rejected token. 3. When silent refresh failed, the handler fell through to `handleOAuthRequired()` whose recent-completion fast path can reuse a COMPLETED `mcp_oauth` flow within `PENDING_STALE_MS`. Those cached tokens are exactly the ones the server just rejected, so the connection would keep adopting them and looping on 401s until the cache aged out. Resolution: - Wrap `runSilentRefresh()` with a 60-second `withTimeout` (well under `connectClient`'s 120s OAuth timeout). On timeout the `.catch` resolves to null and the `finally` clears the in-flight entry, so the next 401 starts fresh and falls through to interactive OAuth. - Extract two helpers — `invalidateGetTokensFlow` and `invalidateCompletedOAuthFlow` — and call them from the right branches: clear `mcp_get_tokens` after silent-refresh success AND after interactive-OAuth `storeTokens`; clear the COMPLETED `mcp_oauth` state (plus its CSRF mapping) before falling through to interactive OAuth so the fast-reuse path can't re-serve the rejected tokens. - Add three regression tests: hung refresh release-the-lock under fake timers, completed-OAuth cache invalidation pre-fallback, and `mcp_get_tokens` invalidation after interactive token store. * fix: Address Codex round-4 review on silent MCP OAuth refresh Three more findings from Codex on PR #13369: 1. (P1) The silent-refresh in-flight lock keyed only by `userId:serverName`. In multi-tenant setups where two tenants share a userId (e.g. username-based IDs) and the same MCP server name, a concurrent mid-session 401 from tenant B would join tenant A's in-flight refresh and adopt tenant A's freshly minted tokens onto a tenant-B connection — a cross-tenant credential leak. 2. (P2) `invalidateGetTokensFlow` deleted the `mcp_get_tokens` flow state regardless of its status. When another connection was currently in `getOAuthTokens()` (PENDING flow) and joiners were monitoring it, the unconditional delete made those waiters see "Flow state not found" and unnecessarily fall back to interactive OAuth — even though fresh tokens were already being written. 3. (P2) The 60s `withTimeout` wrapping `runSilentRefresh()` only races the promise; it does not cancel the underlying `forceRefreshTokens` / refresh-token HTTP request. If the request returned after a subsequent interactive OAuth had stored newer tokens, the late completion would `storeTokens` over the newer state. This requires a provider that doesn't rotate refresh tokens AND a refresh slower than 60s AND a successful interactive OAuth in that window — narrow but real. Resolution: - Capture `getTenantId()` into a new `factory.tenantId` field at factory construction time (before the OAuth handler closes over it outside the original request's async context) and include it in the silent-refresh lock key as `tenantId:userId:serverName`. - `invalidateGetTokensFlow` now calls `getFlowState` first and only deletes when `status === 'COMPLETED'`. PENDING lookups are left alone so concurrent `getOAuthTokens` waiters via `monitorFlow` can still settle. - For (3), document the race as a known limitation inline. Fully closing it requires threading an `AbortSignal` through `MCPTokenStorage.forceRefreshTokens` and the OAuth refresh handler to skip the late `storeTokens` after timeout — out of scope for this PR's surgical change. - Add `getTenantId` to the `MCPOAuthConnectionEvents` test's `@librechat/data-schemas` mock so the factory constructor doesn't blow up under that suite. - Add three regression tests: per-tenant lock isolation, PENDING-state preservation under `invalidateGetTokensFlow`, and (reused) the existing interactive-store invalidation test now driven through `getFlowState` returning the COMPLETED state. * fix: Address silent MCP OAuth refresh review Restore captured tenant context around token storage and OAuth fallback paths so mid-session callbacks do not lose tenant scope. Thread AbortSignal through forced refresh and OAuth token requests, cap silent refresh by the connection OAuth timeout, and prevent timed-out refreshes from writing stale credentials after fallback. Complete pending mcp_get_tokens flows with fresh tokens, add missing FlowState createdAt test fixtures, and cover the new tenant/abort/cache behaviors. * fix: Tighten tenant-scoped MCP token refresh Cap silent refresh by both the factory connect timeout and the connection OAuth wait timeout so fallback OAuth wins before the outer connect attempt expires. Tenant-scope mcp_get_tokens flow ids for both token lookup and refresh invalidation, preventing cross-tenant flow completion or cache deletion when tenants share user ids and server names. Add regression tests for the omitted initTimeout budget and tenant-prefixed token flow locks. * fix: Reserve MCP OAuth fallback budget * fix: Harden MCP OAuth refresh races * fix: Keep MCP OAuth fallback route-compatible * test: Add SDK MCP OAuth refresh repro * fix: Address MCP OAuth refresh review findings * fix: Address MCP OAuth tenant review findings * fix: Close MCP OAuth route tenant gaps * fix: Preserve MCP OAuth refresh flow guards * fix: Avoid reprocessing MCP OAuth reauth config * fix: Release timed-out MCP refresh locks * fix: Release MCP OAuth request callbacks * fix: Tenant-scope remaining MCP OAuth flow lookups * ci: Sort imports in MCP OAuth test suites |