mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-04 05:28:30 +00:00
63 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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
|
||
|
|
547bd8c4bf
|
🧵 feat: Persist View-Only Subagent Threads (#14957) | ||
|
|
aa35cd42b1
|
📬 feat: Add Durable Agent Trigger Delivery (#14925)
* feat: wire trusted agent trigger dispatch * feat: add durable agent trigger delivery * fix: annotate trigger envelope byte limit * test: isolate trigger startup in server specs * fix: fence trigger delivery during account deletion * test: isolate trigger service in user controller specs * fix: close trigger deletion admission race * fix: harden account deletion fences * fix: close durable trigger review gaps * fix: require offline stale-fence recovery * fix: type trigger lane sequence ids * fix: fence admin user deletion triggers * fix: make trigger deletion recovery durable * fix: harden offline user deletion * fix: serialize trigger lane publication * style: sort trigger delivery imports * fix: recover orphaned trigger publications * fix: preserve trigger recovery ordering * fix: fence trigger publication during purge * fix: defer remote trigger deletion fences * fix: close durable delivery cleanup races * fix: drain CLI generation owners before deletion |
||
|
|
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> |
||
|
|
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 |
||
|
|
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. |
||
|
|
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 |
||
|
|
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. |
||
|
|
b84e26671e
|
🕒 feat: Track Terms Acceptance Timestamp (#10810)
* feat: add terms acceptance timestamp tracking and migration script * feat: update migration script to use countUsers method for user count * Update config/migrate-terms-timestamp.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: enhance terms acceptance response to include acceptance timestamp * fix: make terms acceptance idempotent and fail migration on partial errors Preserve the original termsAcceptedAt on repeat accepts within a terms cycle so retried or duplicate requests no longer overwrite the first acceptance time. Exit the migration script with a non-zero status when any per-user update fails so partial failures are not reported as successful. * style: fix import ordering in data-provider mutations * refactor: record terms acceptance atomically to preserve first-accept time Replace the read-then-write in acceptTermsController with a single atomic acceptTerms method that conditionally stamps termsAcceptedAt via an $ifNull aggregation update. This removes the TOCTOU window where two concurrent first-time accepts could overwrite the earlier acceptance timestamp, while still preserving an existing timestamp and backfilling legacy accepted users. * fix: run terms timestamp migration under system tenant context Wrap the count, cursor scan, and per-user updates in runAsSystem so the tenant isolation plugin does not throw under TENANT_ISOLATION_STRICT or scope the cross-tenant migration to a non-existent tenant, matching the other maintenance migrations. * fix: guard terms backfill against concurrent acceptances Add the missing-timestamp predicate to the per-user updateOne filter so a user who accepts through the API between the cursor read and the write keeps their real acceptance time instead of being overwritten with createdAt. Track modified vs skipped so the summary reflects skips. * fix: scope terms backfill to still-accepted users Add termsAccepted: true to the per-user updateOne filter so a reset that clears acceptance between the cursor read and the write is not re-stamped with createdAt, which would otherwise poison the next acceptance cycle through the $ifNull preserve in acceptTerms. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
|
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. |
||
|
|
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 |
||
|
|
8c71dbcb32
|
🛂 fix: Normalize Verification Flow Error Responses (#13558)
* fix: normalize verification flow responses * fix: keep verification responses consistent |
||
|
|
86fe79c37d
|
🔗 feat: Add Granular Access Control to Shared Links via ACL System (#13051)
* feat: Add granular access control to shared links via ACL system * fix(shared-links): preserve isPublic on failed migration grants Transient ACL failures during auto-migration permanently stranded links — $unset ran unconditionally, removing the legacy flag that triggers retry. Now only $unset isPublic after all grants succeed. * fix(config): skip isPublic unset for failed ACL grants Bulk migration unconditionally removed isPublic from all links, even those whose ACL writes failed. Failed links then lost the legacy marker needed for auto-migration retry. Now tracks failed link IDs per-batch and excludes them from the $unset step. Also adds sharedLink to AccessRole resourceType schema enum — was missing, only worked because seedDefaultRoles uses findOneAndUpdate which bypasses validation. * ci(config): add jest config and PR workflow for migration tests config/__tests__/ specs depend on api/jest.config.js module mappings but had no dedicated runner. Adds config/jest.config.js extending api config with absolutized paths, npm test:config script, and a GitHub Actions workflow triggered by changes to config/, api/models/, api/db/, or packages/ ACL code. * fix(permissions): honor boolean sharedLinks config SHARED_LINKS has no USE permission, so boolean config produced an empty update payload — gate conditions only matched object form, making `sharedLinks: false` a no-op on existing perms. * fix(share): resolve role before creating shared link Role lookup between create and grant left an orphaned link without ACL entries if getRoleByName threw — retry then hit "Share already exists" with no recovery path. * fix: Restore Public ACL Access Checks * fix: Type Public ACL Lookup * fix: Preserve Private Legacy Shared Links * chore: Promote Shared Link Permission Migration * fix: Address Shared Link Review Findings * fix: Repair Shared Link CI Follow-Up * fix: Narrow Shared Link Mongoose Test Mock * fix: Address Shared Link Review Follow-Ups * fix: Close Shared Link Review Gaps * fix: Guard Missing Shared Link Permission Backfill * test: Add Shared Link Mock E2E * test: Stabilize Shared Link Mock E2E --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
|
75baa5b848
|
🧼 fix: Sanitize User Response Fields (#13421) | ||
|
|
c342e2345b
|
🪪 fix: Resolve Group-Scoped Config Overrides (#13176)
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
* fix: resolve group-scoped config overrides * test: fix endpoint config request mock typing * fix: keep remote agent preauth config tenant-scoped * test: align config scoping expectations * test: reproduce group endpoint override resolution |
||
|
|
4cce88be42
|
🪟 feat: Add allowedAddresses Exemption List For SSRF-Guarded Targets (#12933)
* 🪟 feat: Add allowedAddresses Exemption List For SSRF-Guarded Targets LibreChat already blocks SSRF-prone targets (private IPs, loopback, link-local, .internal/.local TLDs) at every server-side fetch site that consumes user-controllable URLs — custom-endpoint baseURLs, MCP servers, OpenAPI Actions, and OAuth endpoints. The only existing escape hatch is `allowedDomains`, but that flips the field into a strict whitelist: adding `127.0.0.1` to permit a self-hosted Ollama also blocks every public destination that isn't in the list. Introduce `allowedAddresses` as the orthogonal primitive: a private- IP-space exemption list. When a hostname or its resolved IP appears in the list, the SSRF block is bypassed for that target. Public destinations remain reachable. Operators can now run self-hosted LLMs / MCP servers / Action endpoints on private addresses without weakening the default-deny posture for everything else. Schema additions in `packages/data-provider/src/config.ts`: - `endpoints.allowedAddresses` (new — gates `validateEndpointURL`) - `mcpSettings.allowedAddresses` (parallel to `allowedDomains`) - `actions.allowedAddresses` (parallel to `allowedDomains`) Core changes in `packages/api/src/auth/`: - New `isAddressAllowed(hostnameOrIP, allowedAddresses)` — pure, case-insensitive, bracket-stripped literal match. - Threaded the list through `isSSRFTarget`, `resolveHostnameSSRF`, `isDomainAllowedCore`, `isActionDomainAllowed`, `isMCPDomainAllowed`, `isOAuthUrlAllowed`, and `validateEndpointURL`. - Extended `createSSRFSafeAgents` and `createSSRFSafeUndiciConnect` to accept the list, building an SSRF-safe DNS lookup that exempts matching hostnames/IPs at TCP connect time (TOCTOU-safe). Wiring: - Custom and OpenAI endpoint initialize sites pass `endpoints.allowedAddresses` to `validateEndpointURL`. - `MCPServersRegistry` stores `allowedAddresses` and exposes it via `getAllowedAddresses()`. The factory, connection class, manager, `UserConnectionManager`, and `ConnectionsRepository` all thread it through to the SSRF utilities. - `MCPOAuthHandler.initiateOAuthFlow`, `refreshOAuthTokens`, and `validateOAuthUrl` accept the list and consult it on every URL validation along the OAuth chain. - `ToolService`, `ActionService`, and the assistants/agents action routes pass `actions.allowedAddresses` to `isActionDomainAllowed` and to `createSSRFSafeAgents` for runtime action calls. - `initializeMCPs.js` reads `mcpSettings.allowedAddresses` from the app config and forwards it to the registry constructor. Documentation: - `librechat.example.yaml` shows the new field next to each existing `allowedDomains` block, with a note clarifying that `allowedAddresses` is an exemption list (not a whitelist). Tests: - Unit tests for `isAddressAllowed` covering literal IPs, hostnames, IPv6 brackets, case insensitivity, and partial-match rejection. - Exemption tests for every entry point: `isSSRFTarget`, `resolveHostnameSSRF`, `validateEndpointURL`, `isActionDomainAllowed`, `isMCPDomainAllowed`, `isOAuthUrlAllowed`. - Existing tests updated to reflect the new optional parameter. Default behavior is unchanged: omitted = empty list = no exemptions. * 🩹 fix: Plumb allowedAddresses Through AppConfig endpoints Type The initial PR added `endpoints.allowedAddresses` to the data-provider config schema and consumed it in the endpoint initialize sites, but the runtime `AppConfig.endpoints` shape in `@librechat/data-schemas` was a hand-maintained subset that didn't include the new field — so `tsc` rejected `appConfig.endpoints.allowedAddresses`. Add the field to `AppConfig['endpoints']` in `packages/data-schemas/src/types/app.ts` and forward it from the loaded config in `packages/data-schemas/src/app/endpoints.ts` so the runtime config carries the value. Update `initializeMCPs.spec.js` to expect the third positional argument (`allowedAddresses`) on the `createMCPServersRegistry` call. * 🩹 fix: Enforce allowedDomains Before allowedAddresses In isOAuthUrlAllowed The initial implementation checked the address exemption first, so a URL whose hostname appeared in `allowedAddresses` would return true even when the admin had configured `allowedDomains` as a strict bound on OAuth endpoints. A malicious MCP server could advertise OAuth metadata, token, or revocation URLs at any address the admin had permitted for an unrelated reason (a self-hosted LLM at `127.0.0.1`, for example) and pass validation, expanding SSRF reach beyond the configured domain whitelist. Reorder: when `allowedDomains` is set, treat it as authoritative — return true only if the URL matches a domain entry, otherwise fall through to false. The address exemption only applies when no `allowedDomains` is configured (mirrors how the downstream SSRF check in `validateOAuthUrl` consults `allowedAddresses`). Add a regression test asserting that an `allowedAddresses` entry does not broaden a configured `allowedDomains` list. Reported by chatgpt-codex-connector on PR #12933. * 🩹 fix: Forward allowedAddresses To Remaining OAuth Callers Two `MCPOAuthHandler` callers still used the pre-feature signatures and were silently dropping the new `allowedAddresses` argument: - `api/server/routes/mcp.js` invoked `initiateOAuthFlow` with the old 5-argument shape, so OAuth flows initiated through the route handler ignored the registry's `getAllowedAddresses()` and would reject any metadata/authorization/token URL on a permitted private host. - `api/server/controllers/UserController.js#maybeUninstallOAuthMCP` invoked `revokeOAuthToken` without the address exemption, so uninstalling an OAuth-backed MCP server on a permitted private host would fail at the revocation step even though the rest of the MCP connection path now permits it. Both sites now read `allowedAddresses` from the registry alongside `allowedDomains` and forward it. Reported by Copilot on PR #12933. * 🩹 fix: Update Test Mocks And Assertions For OAuth allowedAddresses The previous commit started passing `allowedAddresses` to `MCPOAuthHandler.initiateOAuthFlow` from `api/server/routes/mcp.js` and to `MCPOAuthHandler.revokeOAuthToken` from `api/server/controllers/UserController.js`, but the corresponding test files mocked the registry without `getAllowedAddresses` (causing `TypeError`s) and asserted the old positional shape on `toHaveBeenCalledWith`. Update the mocks and assertions to match the new arity: - `api/server/routes/__tests__/mcp.spec.js`: add `getAllowedDomains`/`getAllowedAddresses` to the registry mock and expect the additional positional args on `initiateOAuthFlow`. - `api/server/controllers/__tests__/maybeUninstallOAuthMCP.spec.js`: add a `getAllowedAddresses` mock alongside the existing `getAllowedDomains` and seed it in `setupOAuthServerFound`. - `api/server/controllers/__tests__/UserController.mcpOAuth.spec.js`: add `getAllowedAddresses` to the registry mock and expect the trailing `null` arg on the three `revokeOAuthToken` assertions. * 🛡️ fix: Address Comprehensive Review — Scope allowedAddresses To Private IP Space Major findings from the comprehensive PR review (severity → fix): **CRITICAL — `validateOAuthUrl` SSRF fallback bypass.** When `allowedDomains` is configured and a URL fails the whitelist, the SSRF fallback in `validateOAuthUrl` was still passing `allowedAddresses` to `isSSRFTarget` / `resolveHostnameSSRF`, letting a malicious MCP server advertise OAuth endpoints at any address the admin had permitted for an unrelated reason. Suppress `allowedAddresses` in the fallback when `allowedDomains` is active — the address exemption is opt-in for the no-whitelist mode only. **MAJOR — WebSocket transport SSRF check ignored exemptions.** The `constructTransport` WebSocket branch called `resolveHostnameSSRF(wsHostname)` without `this.allowedAddresses`, so a permitted private MCP server would pass `isMCPDomainAllowed` but be blocked at transport creation. Forward the exemption. **Scope `allowedAddresses` to private IP space only (operator directive).** The exemption list is for permitting private/internal targets; it must not be a back-door to broaden trust to public destinations. - Schema (`packages/data-provider/src/config.ts`): new `allowedAddressesSchema` rejects URLs (`://`), paths/CIDR (`/`), whitespace, and public IPv4/IPv6 literals at config-load time. Wired into `endpoints`, `mcpSettings`, and `actions`. - Runtime (`packages/api/src/auth/domain.ts`): `isAddressAllowed` now drops public-IP candidates and public-IP entries on the match path — defense in depth so a misconfigured runtime list never grants exemption. - Hot path (`packages/api/src/auth/agent.ts`): `buildSSRFSafeLookup` pre-normalizes the list into a `Set<string>` once at construction and applies the same scoping filter, so the connect-time DNS lookup is an O(1) Set membership check instead of a full re-iterate-and-normalize on every outbound request. **Test coverage for the connect-time and OAuth-fallback paths.** - `agent.spec.ts`: new describe block exercising `buildSSRFSafeLookup` and `createSSRFSafe*` with `allowedAddresses` — hostname-literal exemption, resolved-IP exemption, public-IP scoping, URL/CIDR/whitespace rejection, and the default no-list block. - `handler.allowedAddresses.test.ts` (new): integration tests for `validateOAuthUrl` — covers both the no-domains-set "permit private" path and the strict-bound regression where `allowedAddresses` must NOT bypass `allowedDomains`. **Documentation & cleanup.** - `connection.ts` redirect SSRF check: explicit comment that `allowedAddresses` is intentionally NOT consulted for redirect targets (server-controlled, must not inherit the admin's exemption). - `MCPConnectionFactory.test.ts`: replaced an `eslint-disable` with a proper `import { getTenantId } from '@librechat/data-schemas'`. The disable was added to make a pre-existing `require()` quiet — the cleaner fix is to use the existing top-level import. Updated `MCPConnectionSSRF.test.ts` WebSocket SSRF assertions to match the new two-argument call shape (`hostname, allowedAddresses`). * 🩹 fix: Require Absolute URL Before allowedAddresses Trust Bypass In isOAuthUrlAllowed `parseDomainSpec` is lenient — it silently prepends `https://` to schemeless inputs so it can match patterns like bare `example.com`. That leniency leaked into `isOAuthUrlAllowed`'s new `allowedAddresses` short-circuit: a value like `10.0.0.5/oauth` (no scheme) would parse successfully via the prepended default, hit the address-exemption path, return `true`, and skip `validateOAuthUrl`'s strict `new URL(url)` parse-or-throw — only to fail later in OAuth discovery with a less clear runtime error. Add a strict `new URL(url)` gate at the top of `isOAuthUrlAllowed`. Schemeless inputs now fall through to `validateOAuthUrl`'s explicit "Invalid OAuth <field>" rejection. Tests added in both `auth/domain.spec.ts` (unit) and the OAuth handler integration spec (end-to-end). Reported by chatgpt-codex-connector (P2) on PR #12933. * 🛡️ fix: Address Follow-Up Comprehensive Review — Schema Tests, Shared Normalization, host:port Auditing the second comprehensive review: **F1 MAJOR — schema validation untested.** `allowedAddressesSchema` had zero coverage, so a regression in the three refinement stages or the three wiring locations (`endpoints` / `mcpSettings` / `actions`) would silently let invalid entries reach the runtime. Added a dedicated `describe('allowedAddressesSchema')` block in `config.spec.ts` covering: valid private IPs (v4 + v6, including the previously-missed 192.0.0.0/24 range), accepted hostnames, all rejection categories (URLs, CIDR, paths, whitespace tabs/newlines, host:port, public IP literals), and full `configSchema.parse()` integration at each of the three nesting points. **F2 MINOR — `isPrivateIPv4Literal` divergence.** The schema reimpl in `packages/data-provider` was discarding the `c` octet, so the `192.0.0.0/24` (RFC 5736 IETF protocol assignments) range that the authoritative `isPrivateIPv4` accepts was being rejected with a misleading "public IP" error. Destructure `c` and add the missing range check; covered by the new schema tests. **F3 MINOR — DRY violation across `domain.ts` and `agent.ts`.** Both files had independent normalization implementations with a subtle whitespace-check divergence (`/\s/` vs `.includes(' ')`). Extracted the shared logic into a new `packages/api/src/auth/allowedAddresses.ts` module that both consumers import: - `normalizeAddressEntry(entry)` — single-entry shape check - `looksLikeHostPort(entry)` — host:port detector (used by F4) - `normalizeAllowedAddressesSet(list)` — pre-normalized Set for the connect-time hot path - `isAddressInAllowedSet(candidate, set)` — membership check that enforces private-IP scoping on the candidate Both `isAddressAllowed` (preflight) and `buildSSRFSafeLookup` (connect) now go through the same primitives; the whitespace divergence is gone. To break the import cycle (`allowedAddresses` needs `isPrivateIP`, `domain` previously owned it), extracted IP private-range detection into a leaf `auth/ip.ts` module. `domain.ts` re-exports `isPrivateIP` for backward compatibility with existing call sites. **F4 MINOR — `host:port` silently misclassified.** Entries like `localhost:8080` previously slipped through the URL/path guard, were mis-detected as IPv6, failed `isPrivateIP`, and were silently dropped with a misleading "public IP" schema error. Added an explicit `looksLikeHostPort` check with a clear error: "allowedAddresses entries must not include a port — list the bare hostname or IP only." Bare `::1`, `[::1]`, and other valid IPv6 literals are intentionally not matched (regex distinguishes by colon count and the bracketed `[ipv6]:port` form). **F5 MINOR — hostname-trust documentation gap.** Hostname entries short-circuit `resolveHostnameSSRF` before any DNS lookup — that's a deliberate design (admin trusts the name) but it means the exemption follows whatever the name resolves to at runtime. Added an explicit note in `librechat.example.yaml` for both `mcpSettings.allowedAddresses` and `endpoints.allowedAddresses`: "a hostname entry trusts whatever IP that name resolves to. Only list hostnames whose DNS you control. Prefer literal IPs when you can." **F6** (8 positional params) is flagged for follow-up; refactor to an options object is a breaking-API change deferred to a separate PR. **F7** (redirect/WebSocket asymmetry, NIT, conf 40) — skipping; the existing inline comment is sufficient. * 🧹 chore: Address Follow-Up NITs — Import Order And Mirror-Function Naming Three NITs from the latest comprehensive review: **NIT #1 (conf 85) — local import order.** AGENTS.md requires local imports sorted longest-to-shortest. Both `domain.ts` and `agent.ts` had `./ip` (shorter) before `./allowedAddresses` (longer). Swapped. **NIT #2 (conf 60) — missing cross-reference.** The schema-side `isHostPortShape` in `packages/data-provider/src/config.ts` had no note pointing at the canonical runtime mirror. Added a JSDoc paragraph explaining the mirror relationship and why a local copy exists (the data-provider package can't import from `@librechat/api` without creating a circular dependency). **NIT #3 (conf 50) — naming inconsistency.** Renamed `isHostPortShape` → `looksLikeHostPort` so the schema mirror matches the runtime helper exactly. Kept as a separate function (not a shared import) for the same circular-dependency reason; the matching name makes it obvious they should stay in lockstep. |
||
|
|
4e45e8e17c
|
🧹 fix: Clear MCP OAuth Tokens On Revoke
Fixes #12912.\n\n- Clear stored MCP OAuth tokens and flow state on revoke cleanup-only paths.\n- Keep provider revocation best-effort when token and client metadata are available.\n- Add controller and function coverage for stale metadata, missing config, and cleanup failure paths. |
||
|
|
84043432a5
|
🧹 fix: Graceful MCP OAuth Revoke Cleanup When Tokens Are Missing (#12825)
* fix: graceful MCP OAuth revoke cleanup when tokens are missing (#12754) `maybeUninstallOAuthMCP` in `api/server/controllers/UserController.js` aborts before the DB-delete and flow-state cleanup steps whenever `MCPTokenStorage.getTokens` throws `ReauthenticationRequiredError` — which is exactly what happens when a user clicks "Revoke" on an MCP server whose backend is already dead and whose refresh token is gone. The resulting error is both surfaced to the log as a red line and, more importantly, leaks the DB token row and OAuth flow state. Wrap the token retrieval in try/catch following the same best-effort pattern already used for the two `revokeOAuthToken` calls. On `ReauthenticationRequiredError`, skip revocation silently (info log) and continue to the cleanup steps. On any other unexpected error, log a warning and continue — cleanup must always run. Exported `maybeUninstallOAuthMCP` for direct unit testing and added `api/server/controllers/__tests__/maybeUninstallOAuthMCP.spec.js` with 8 cases: early-return guards (non-MCP key, non-OAuth server, missing client info), happy path (both tokens revoked + cleanup), both failure-to-retrieve paths (ReauthenticationRequiredError and arbitrary error — cleanup still runs in both), single-token path, and revocation-call failures (cleanup still runs). Fixes #12754. * test: use instanceof check against real ReauthenticationRequiredError Follow-up to the previous commit on this branch. Two changes: 1. `UserController.maybeUninstallOAuthMCP` now checks `error instanceof ReauthenticationRequiredError` using the real class imported from `@librechat/api`, instead of comparing `error?.name === 'ReauthenticationRequiredError'`. The name-string check matched any unrelated error that happened to have the same `.name`; the `instanceof` check is a proper identity test. 2. The accompanying spec's jest mock for `@librechat/api` now exposes a `ReauthenticationRequiredError` class, and the test imports it from that mock so the `instanceof` comparison in the production code holds during the test. Without this, the two "skips revocation ... still runs cleanup" tests threw `TypeError: Right-hand side of 'instanceof' is not an object` because the mock left the class undefined. All 8 tests in the spec pass. |
||
|
|
963068b112 |
🧬 feat: Scaffold Skills CRUD with ACL Sharing and File Schema (#12613)
* 🧬 feat: Scaffold Skills CRUD with ACL Sharing and File Schema Adds Skills as a new first-class resource modeled on Anthropic's Agent Skills, reusing the existing Prompt ACL stack for sharing. Lays the groundwork for multi-file skills (SkillFile schema + metadata routes) without wiring upload processing — single-file skills (inline SKILL.md body) work end-to-end, multi-file uploads are stubbed for phase 2. * 🔬 fix: Wire Skill Cleanup, AccessRole Enum, and Express 5 Path Params CI surfaced four follow-ups from the initial Skills scaffolding commit that local builds missed: - AccessRole's resourceType field had a hardcoded enum that didn't include `'skill'`, blocking SKILL_OWNER/EDITOR/VIEWER role creation in every test that hit the AccessRole model. - The seedDefaultRoles assertion in accessRole.spec.ts hard-listed the expected role IDs and needed the new SKILL_* entries. - deleteUserController had no cleanup for skills, and the deleteUserResourceCoverage guard test enforces every ResourceType has a documented handler — wired in db.deleteUserSkills(user._id) and added the entry to HANDLED_RESOURCE_TYPES. - Express 5's path-to-regexp v6 rejects the legacy `(*)` named-group glob syntax. The two skill file routes now use a plain `:relativePath` param; the client already encodeURIComponents the path, so a single param is sufficient and decoded server-side. * 🪡 fix: Make Skill Name Uniqueness Application-Level Resolve three more CI failures from the Skills scaffolding PR: - Mongoose creates indexes asynchronously and mongodb-memory-server tests can race ahead of the unique (name, author, tenantId) index being built, so the duplicate-name uniqueness test was flaky. Added an explicit findOne pre-check inside createSkill that throws with code 11000 (mimicking the index violation), giving deterministic behavior. The unique index stays as the persistent guarantee. - The deleteUser.spec.js and UserController.spec.js suites mock the ~/models module directly and were missing deleteUserSkills, causing deleteUserController to throw and return 500 instead of 200. - Removed two doc-comment claims that the SKILL_NAME_MAX_LENGTH and SKILL_DESCRIPTION_MAX_LENGTH constants "match Anthropic's API". The values themselves are reasonable but the comments were misleading about who enforces them. * 🪢 fix: Address Code Review Findings on Skills Scaffolding Resolve all 15 findings from the comprehensive PR review: Critical: - Rollback the created skill when grantPermission throws so a transient ACL failure cannot leave an orphaned, inaccessible skill in the DB. - Fix infinite query cache corruption in useUpdateSkillMutation helpers. setQueriesData([QueryKeys.skills]) matches useSkillsInfiniteQuery's InfiniteData cache entries, which have { pages, pageParams } shape — spreading data.skills on those would throw. Added an isInfiniteSkillData guard and per-page transform so both flat and infinite caches update correctly. Major: - Fix TUpdateSkillContext type: the public type declared previousListData but onMutate actually returns previousListSnapshots (a [key, value] tuple array). Updated the type + added TSkillCacheEntry as a shared export from data-provider. - Add cancelQueries calls before optimistic update in onMutate so in-flight refetches cannot clobber the optimistic state. - Parallelize deleteUserSkills ACL removal via Promise.allSettled instead of a sequential await loop — O(1) round-trip vs O(n). - Stub mockDeleteUserSkills in stubDeletionMocks() and assert it's called with user.id in the deleteUser.spec.js happy-path test. - Add idResolver: getSkillById to the SKILL branch in accessPermissions.js so GET /api/permissions/skill/<missing-id> returns 404 instead of 403. Minor: - Reuse resolved skill from req.resourceAccess.resourceInfo in getHandler to eliminate a redundant getSkillById call per GET /api/skills/:id. - Reject PATCH /api/skills/:id requests whose body contains only expectedVersion — previously they silently bumped version with no changes, triggering spurious 409s for collaborators. - Make TSkill.frontmatter optional (wire type) and add serializeFrontmatter / serializeSourceMetadata helpers that return undefined for empty objects instead of casting incomplete data to SkillFrontmatter. - Standardize deleteUserSkills to accept string | ObjectId and convert internally, matching deleteUserPrompts's signature; UserController now passes user.id consistently. - Replace bumpSkillVersionAndRecount (read-then-write, racy) with bumpSkillVersionAndAdjustFileCount using atomic $inc. upsertSkillFile pre-checks existence to distinguish insert (+1) from replace (0). - Add DELETE /api/skills/:id/files/:relativePath integration tests covering success, 404, and 403 paths. Nits: - Drop trivial resolveSkillId wrapper — pass getSkillById directly. - Remove dead staleTime: 1000 * 10 from useListSkillsQuery since all refetch triggers are already disabled. * 🧭 fix: Resolve Second Skills Review Pass — Cache, Gate, TOCTOU Address 13 of 14 findings from the second code review; reject #13 as misread of the AGENTS.md import-order rule (package types correctly precede local types regardless of length). Major: - Fix addSkillToCachedLists closure bug: a hoisted `prepended` flag was shared across every cache entry matched by setQueriesData, so concurrent flat + infinite caches would silently drop the prepend on whichever was processed second. Replaced the shared helper with three per-entry inline updaters that handle InfiniteData at the page level (page 0 only for prepend, all pages for replace/remove). - Tighten patchHandler's expectedVersion validation: NaN passes `typeof === 'number'` and would previously leak current skill state via a misleading 409. Now requires finite positive integer and returns 400 otherwise. - Guard decodeURIComponent in deleteFileHandler with try/catch — malformed percent encoding now returns 400 instead of 500. - Add PermissionTypes.SKILLS + skillPermissionsSchema + TSkillPermissions in data-provider; seed default SKILLS permissions for ADMIN (all true) and USER (use + create only); wire checkSkillAccess / checkSkillCreate via generateCheckAccess onto the skills router mirroring the prompts pattern. Skills route now enforces role-based capability gates alongside per-resource ACLs. Test suite adds a mocked getRoleByName returning permissive SKILLS. - Fix upsertSkillFile TOCTOU: replaced the pre-check + upsert pair with a single `findOneAndUpdate({ new: false, upsert: true })` call that atomically returns the pre-update doc (null ⇒ insert) so fileCount delta can't double-count on concurrent same-path uploads. Minor: - Add `sourceMetadata` to listSkillsByAccess .select() so summaries no longer silently drop the field for GitHub/Notion-synced skills. - Include `cursor` in useListSkillsQuery's query key so manual pagination doesn't alias across pages. - Clean up TSkillSummary to `Omit<TSkill, 'body' | 'frontmatter'>` matching what serializeSkillSummary actually emits; drop the Omit-then-re-add noise. - Skip getPublicSkillIdSet in createHandler; a newly-created skill cannot have a PUBLIC ACL entry, so pass an empty set directly instead of paying a DB round-trip. - Trim SkillMethods public surface: drop internal helpers countSkillFiles / deleteSkillFilesBySkillId / getSkillFile from the return object; inline the file cascade into deleteSkill. - Use TSkillConflictResponse at the PATCH 409 call site instead of an inline ad-hoc object literal. - Drop the now-unused EXPECTED_VERSION_ERROR module constant. * 🧩 fix: Extend Role Schema + Types with SKILLS PermissionType CI type-check and unit test failures from the PermissionTypes.SKILLS addition surfaced three unrelated places that all hardcode the permission-type set: - IRole.permissions in data-schemas/types/role.ts enumerates every PermissionTypes key as an optional field. Adding SKILLS to the enum without updating the interface caused TS7053 'expression of type PermissionTypes can't be used to index type' errors in role.methods.spec.ts (lines 407-408, 477-478) because Object.values(PermissionTypes) now yielded a value the interface didn't cover. - schema/role.ts rolePermissionsSchema mirrors the interface at the Mongoose layer; also needed SKILLS added so the persisted role document can actually store skill permissions. - data-provider/roles.spec.ts has a guard test that every permission type carrying CREATE/SHARE/SHARE_PUBLIC must be explicitly "tracked" either in RESOURCE_PERMISSION_TYPES or in the PROMPTS/AGENTS/MEMORIES exemption list. Added SKILLS to the exemption list since skills follow the same default model as prompts/agents (USE + CREATE on for USER, SHARE / SHARE_PUBLIC off). All three are additive pass-throughs with no behavior change. * 🏷️ refactor: Introduce ISkillSummary for Narrow List Projection Follow-up NITs from the second review pass on the Skills PR: - Define ISkillSummary = Omit<ISkill, 'body' | 'frontmatter'> and use it as the element type in ListSkillsByAccessResult. The list query's .select() intentionally omits body and frontmatter for payload size, but the previous type claimed both fields were present — a type lie that would mislead future readers even though serializeSkillSummary never touches those fields at runtime. handlers.ts's signature for serializeSkillSummary now accepts ISkillSummary too. - Document the intentional second-round-trip `findOne` in upsertSkillFile. Switching to `findOneAndUpdate({ new: false })` was required for TOCTOU-safe insert-vs-replace detection, which means the handler needs a follow-up query to return the post-upsert document. A comment now explains the tradeoff so future readers don't silently "optimize" it away. No behavior change. * 🌐 fix: Wire SKILL into SHARE_PUBLIC Resource Maps Address codex comment #1 — making a skill public was blocked on two hardcoded resource→permission-type maps that didn't know about SKILL: - api/server/middleware/checkSharePublicAccess.js's resourceToPermissionType map was missing ResourceType.SKILL, so PUT /api/permissions/skill/:id with { public: true } would fall through to the 400 "Unsupported resource type for public sharing" path even though PermissionTypes.SKILLS exists and ADMIN has SHARE_PUBLIC configured. Added the mapping. - client/src/hooks/Sharing/useCanSharePublic.ts has an identical client-side map used to gate the "Make Public" UI toggle. Without the SKILL mapping the hook returned false for everyone, so the toggle wouldn't render for skills once the sharing UI lands in phase 2. Added the mapping. Codex comment #2 (create/update cache writes inject skills into unrelated filtered lists) is invalid — it flags a pattern that mirrors useUpdatePromptGroup (which the PR description explicitly cites as the model) and is a deliberate optimistic-update tradeoff. Trying to match each cache key's embedded filter would couple the mutation callback to query-key internals, which is exactly what setQueriesData is designed to avoid. No change there. * 🧪 feat: Frontmatter Validation, Reserved-Name Fixes, Coaching Warnings Address the follow-up review notes on the Skills PR. This commit closes the gap between the wire-type promise and what the backend actually enforces, tightens the reserved-name rules, and adds a non-blocking coaching tier for validators. Frontmatter validation (new): - Add `validateSkillFrontmatter` in data-schemas/methods/skill.ts with strict mode — unknown keys are rejected so expanding the allowed set is an intentional code change. Known keys are type-checked against a `FrontmatterKind` table derived from Anthropic's Agent Skills spec (name, description, when-to-use, allowed-tools, arguments, argument-hint, user-invocable, disable-model-invocation, model, effort, context, agent, paths, shell, hooks, version, metadata). - `hooks` and `metadata` get a shallow JSON-safety check (max depth 4, max string 2000, max array 100) instead of a full schema, since their full shapes live outside this module. - Wired into BOTH createSkill AND updateSkill so the PATCH path can't smuggle invalid frontmatter past the validator. Validation warning tier (new): - Add optional `severity: 'error' | 'warning'` to `ValidationIssue` (defaults to error). `partitionIssues` splits an issue list into blocking errors and non-blocking warnings. - `createSkill` / `updateSkill` filter on errors for the throw check and return warnings in a new `warnings: ValidationIssue[]` field on their result objects (`CreateSkillResult` / `UpdateSkillResult`). - `validateSkillDescription` now emits a `TOO_SHORT` warning for descriptions under 20 chars — the primary triggering field, so a little coaching goes a long way. - `createHandler` / `patchHandler` in packages/api surface the warnings via a new `attachWarnings` helper that decorates the serialized response with a `warnings?: TSkillWarning[]` field. - `TSkill` gains an optional `warnings?: TSkillWarning[]` field documented as "present on POST/PATCH, never on GET". Reserved-name filter (tightened): - Replace the substring match (`.includes('anthropic')`) with prefix matching on `anthropic-` and `claude-` plus exact-match rejection of CLI slash-command collisions (help, clear, compact, model, exit, quit, settings, plus the bare `anthropic` / `claude` words). Both the pure validator (`methods/skill.ts`) and the Mongoose schema validator (`schema/skill.ts`) updated in lockstep; comments on each reference the other to prevent drift. - `research-anthropic-helper` and `about-claude` are now allowed; `anthropic-helper`, `claude-bot`, and `settings` are still rejected. Documentation: - Add docstrings on `ISkill`, `schema/skill.ts`, and `TSkill` explaining the semantics of `name` (Claude-visible identifier, kebab-case, stable), `displayTitle` (UI-only cosmetic label, NOT sent to Claude), `description` (highest-leverage trigger field), and `source` / `sourceMetadata` (reserved for phase 2+ external sync). - Add a detailed consistency comment on `bumpSkillVersionAndAdjustFileCount` explaining that it runs as a separate MongoDB operation from upsertSkillFile/deleteSkillFile, so `fileCount` can drift if the second op fails — options listed, tradeoff documented, phase 1 risk window noted as closed because upload is still stubbed. Tests: - data-schemas skill.spec.ts: destructure `{ skill, warnings }` from createSkill at every call site; add a TOO_SHORT warning test, a frontmatter strict-mode test, reserved-prefix tests (including positive cases for substring names that should pass), CLI reserved word tests, and a full `validateSkillFrontmatter` describe block covering unknown keys, type mismatches, and deep-nesting rejection. - api/server/routes/skills.test.js: bump default test description above the 20-char threshold, add a warning-emission test, add reserved-prefix + reserved-CLI-word tests, add an unknown-frontmatter- key test asserting the 400 response carries `issues` with `UNKNOWN_KEY`. * 📦 fix: Export CreateSkillResult from data-schemas Methods Index `CreateSkillResult` was defined in `methods/skill.ts` and consumed by `packages/api/src/skills/handlers.ts` but never re-exported from the methods barrel, so the type-check job failed with TS2724 "'@librechat/data-schemas' has no exported member named 'CreateSkillResult'". Rollup's bundle-mode build picked up the type via its internal resolver, but the standalone `tsc --noEmit` type-check ran against the package's public entrypoint and couldn't see it. Added the type import + export alongside the existing `UpdateSkillResult` export, which fixes the CI type-check without any runtime change. |
||
|
|
9f6d8c6e93
|
🧵 feat: ALS Context Middleware, Tenant Threading, and Config Cache Invalidation (#12407)
* feat: add tenant context middleware for ALS-based isolation Introduces tenantContextMiddleware that propagates req.user.tenantId into AsyncLocalStorage, activating the Mongoose applyTenantIsolation plugin for all downstream DB queries within a request. - Strict mode (TENANT_ISOLATION_STRICT=true) returns 403 if no tenantId - Non-strict mode passes through for backward compatibility - No-op for unauthenticated requests - Includes 6 unit tests covering all paths * feat: register tenant middleware and wrap startup/auth in runAsSystem() - Register tenantContextMiddleware in Express app after capability middleware - Wrap server startup initialization in runAsSystem() for strict mode compat - Wrap auth strategy getAppConfig() calls in runAsSystem() since they run before user context is established (LDAP, SAML, OpenID, social login, AuthService) * feat: thread tenantId through all getAppConfig callers Pass tenantId from req.user to getAppConfig() across all callers that have request context, ensuring correct per-tenant cache key resolution. Also fixes getBaseConfig admin endpoint to scope to requesting admin's tenant instead of returning the unscoped base config. Files updated: - Controllers: UserController, PluginController - Middleware: checkDomainAllowed, balance - Routes: config - Services: loadConfigModels, loadDefaultModels, getEndpointsConfig, MCP - Audio services: TTSService, STTService, getVoices, getCustomConfigSpeech - Admin: getBaseConfig endpoint * feat: add config cache invalidation on admin mutations - Add clearOverrideCache(tenantId?) to flush per-principal override caches by enumerating Keyv store keys matching _OVERRIDE_: prefix - Add invalidateConfigCaches() helper that clears base config, override caches, tool caches, and endpoint config cache in one call - Wire invalidation into all 5 admin config mutation handlers (upsert, patch, delete field, delete overrides, toggle active) - Add strict mode warning when __default__ tenant fallback is used - Add 3 new tests for clearOverrideCache (all/scoped/base-preserving) * chore: update getUserPrincipals comment to reflect ALS-based tenant filtering The TODO(#12091) about missing tenantId filtering is resolved by the tenant context middleware + applyTenantIsolation Mongoose plugin. Group queries are now automatically scoped by tenantId via ALS. * fix: replace runAsSystem with baseOnly for pre-tenant code paths App configs are tenant-owned — runAsSystem() would bypass tenant isolation and return cross-tenant DB overrides. Instead, add baseOnly option to getAppConfig() that returns YAML-derived config only, with zero DB queries. All startup code, auth strategies, and MCP initialization now use getAppConfig({ baseOnly: true }) to get the YAML config without touching the Config collection. * fix: address PR review findings — middleware ordering, types, cache safety - Chain tenantContextMiddleware inside requireJwtAuth after passport auth instead of global app.use() where req.user is always undefined (Finding 1) - Remove global tenantContextMiddleware registration from index.js - Update BalanceMiddlewareOptions to include tenantId, remove redundant cast (Finding 4) - Add warning log when clearOverrideCache cannot enumerate keys on Redis (Finding 3) - Use startsWith instead of includes for cache key filtering (Finding 12) - Use generator loop instead of Array.from for key enumeration (Finding 3) - Selective barrel export — exclude _resetTenantMiddlewareStrictCache (Finding 5) - Move isMainThread check to module level, remove per-request check (Finding 9) - Move mid-file require to top of app.js (Finding 8) - Parallelize invalidateConfigCaches with Promise.all (Finding 10) - Remove clearOverrideCache from public app.js exports (internal only) - Strengthen getUserPrincipals comment re: ALS dependency (Finding 2) * fix: restore runAsSystem for startup DB ops, consolidate require, clarify baseOnly - Restore runAsSystem() around performStartupChecks, updateInterfacePermissions, initializeMCPs, and initializeOAuthReconnectManager — these make Mongoose queries that need system context in strict tenant mode (NEW-3) - Consolidate duplicate require('@librechat/api') in requireJwtAuth.js (NEW-1) - Document that baseOnly ignores role/userId/tenantId in JSDoc (NEW-2) * test: add requireJwtAuth tenant chaining + invalidateConfigCaches tests - requireJwtAuth: 5 tests verifying ALS tenant context is set after passport auth, isolated between concurrent requests, and not set when user has no tenantId (Finding 6) - invalidateConfigCaches: 4 tests verifying all four caches are cleared, tenantId is threaded through, partial failure is handled gracefully, and operations run in parallel via Promise.all (Finding 11) * fix: address Copilot review — passport errors, namespaced cache keys, /base scoping - Forward passport errors in requireJwtAuth before entering tenant middleware — prevents silent auth failures from reaching handlers (P1) - Account for Keyv namespace prefix in clearOverrideCache — stored keys are namespaced as "APP_CONFIG:_OVERRIDE_:..." not "_OVERRIDE_:...", so override caches were never actually matched/cleared (P2) - Remove role from getBaseConfig — /base should return tenant-scoped base config, not role-merged config that drifts per admin role (P2) - Return tenantStorage.run() for cleaner async semantics - Update mock cache in service.spec.ts to simulate Keyv namespacing * fix: address second review — cache safety, code quality, test reliability - Decouple cache invalidation from mutation response: fire-and-forget with logging so DB mutation success is not masked by cache failures - Extract clearEndpointConfigCache helper from inline IIFE - Move isMainThread check to lazy once-per-process guard (no import side effect) - Memoize process.env read in overrideCacheKey to avoid per-request env lookups and log flooding in strict mode - Remove flaky timer-based parallelism assertion, use structural check - Merge orphaned double JSDoc block on getUserPrincipals - Fix stale [getAppConfig] log prefix → [ensureBaseConfig] - Fix import order in tenant.spec.ts (package types before local values) - Replace "Finding 1" reference with self-contained description - Use real tenantStorage primitives in requireJwtAuth spec mock * fix: move JSDoc to correct function after clearEndpointConfigCache extraction * refactor: remove Redis SCAN from clearOverrideCache, rely on TTL expiry Redis SCAN causes 60s+ stalls under concurrent load (see #12410). APP_CONFIG defaults to FORCED_IN_MEMORY_CACHE_NAMESPACES, so the in-memory store.keys() path handles the standard case. When APP_CONFIG is Redis-backed, overrides expire naturally via overrideCacheTtl (60s default) — an acceptable window for admin config mutations. * fix: remove return from tenantStorage.run to satisfy void middleware signature * fix: address second review — cache safety, code quality, test reliability - Switch invalidateConfigCaches from Promise.all to Promise.allSettled so partial failures are logged individually instead of producing one undifferentiated error (Finding 3) - Gate overrideCacheKey strict-mode warning behind a once-per-process flag to prevent log flooding under load (Finding 4) - Add test for passport error forwarding in requireJwtAuth — the if (err) { return next(err) } branch now has coverage (Finding 5) - Add test for real partial failure in invalidateConfigCaches where clearAppConfigCache rejects (not just the swallowed endpoint error) * chore: reorder imports in index.js and app.js for consistency - Moved logger and runAsSystem imports to maintain a consistent import order across files. - Improved code readability by ensuring related imports are grouped together. |
||
|
|
87a3b8221a
|
🧹 chore: Consolidate getSoleOwnedResourceIds into data-schemas and use db object in PermissionService
Move getSoleOwnedResourceIds from PermissionService to data-schemas aclEntry methods, update PermissionService to use the db object pattern instead of destructured imports from ~/models, and update UserController + tests accordingly. |
||
|
|
a0fed6173c
|
🗂️ refactor: Migrate S3 Storage to TypeScript in packages/api (#11947)
* Migrate S3 storage module with unit and integration tests - Migrate S3 CRUD and image operations to packages/api/src/storage/s3/ - Add S3ImageService class with dependency injection - Add unit tests using aws-sdk-client-mock - Add integration tests with real s3 bucket (condition presence of AWS_TEST_BUCKET_NAME) * AI Review Findings Fixes * chore: tests and refactor S3 storage types - Added mock implementations for the 'sharp' library in various test files to improve image processing testing. - Updated type references in S3 storage files from MongoFile to TFile for consistency and type safety. - Refactored S3 CRUD operations to ensure proper handling of file types and improve code clarity. - Enhanced integration tests to validate S3 file operations and error handling more effectively. * chore: rename test file * Remove duplicate import of refreshS3Url * chore: imports order * fix: remove duplicate imports for S3 URL handling in UserController * fix: remove duplicate import of refreshS3FileUrls in files.js * test: Add mock implementations for 'sharp' and '@librechat/api' in UserController tests - Introduced mock functions for the 'sharp' library to facilitate image processing tests, including metadata retrieval and buffer conversion. - Enhanced mocking for '@librechat/api' to ensure consistent behavior in tests, particularly for the needsRefresh and getNewS3URL functions. --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
|
8ba2bde5c1
|
📦 refactor: Consolidate DB models, encapsulating Mongoose usage in data-schemas (#11830)
* chore: move database model methods to /packages/data-schemas * chore: add TypeScript ESLint rule to warn on unused variables * refactor: model imports to streamline access - Consolidated model imports across various files to improve code organization and reduce redundancy. - Updated imports for models such as Assistant, Message, Conversation, and others to a unified import path. - Adjusted middleware and service files to reflect the new import structure, ensuring functionality remains intact. - Enhanced test files to align with the new import paths, maintaining test coverage and integrity. * chore: migrate database models to packages/data-schemas and refactor all direct Mongoose Model usage outside of data-schemas * test: update agent model mocks in unit tests - Added `getAgent` mock to `client.test.js` to enhance test coverage for agent-related functionality. - Removed redundant `getAgent` and `getAgents` mocks from `openai.spec.js` and `responses.unit.spec.js` to streamline test setup and reduce duplication. - Ensured consistency in agent mock implementations across test files. * fix: update types in data-schemas * refactor: enhance type definitions in transaction and spending methods - Updated type definitions in `checkBalance.ts` to use specific request and response types. - Refined `spendTokens.ts` to utilize a new `SpendTxData` interface for better clarity and type safety. - Improved transaction handling in `transaction.ts` by introducing `TransactionResult` and `TxData` interfaces, ensuring consistent data structures across methods. - Adjusted unit tests in `transaction.spec.ts` to accommodate new type definitions and enhance robustness. * refactor: streamline model imports and enhance code organization - Consolidated model imports across various controllers and services to a unified import path, improving code clarity and reducing redundancy. - Updated multiple files to reflect the new import structure, ensuring all functionalities remain intact. - Enhanced overall code organization by removing duplicate import statements and optimizing the usage of model methods. * feat: implement loadAddedAgent and refactor agent loading logic - Introduced `loadAddedAgent` function to handle loading agents from added conversations, supporting multi-convo parallel execution. - Created a new `load.ts` file to encapsulate agent loading functionalities, including `loadEphemeralAgent` and `loadAgent`. - Updated the `index.ts` file to export the new `load` module instead of the deprecated `loadAgent`. - Enhanced type definitions and improved error handling in the agent loading process. - Adjusted unit tests to reflect changes in the agent loading structure and ensure comprehensive coverage. * refactor: enhance balance handling with new update interface - Introduced `IBalanceUpdate` interface to streamline balance update operations across the codebase. - Updated `upsertBalanceFields` method signatures in `balance.ts`, `transaction.ts`, and related tests to utilize the new interface for improved type safety. - Adjusted type imports in `balance.spec.ts` to include `IBalanceUpdate`, ensuring consistency in balance management functionalities. - Enhanced overall code clarity and maintainability by refining type definitions related to balance operations. * feat: add unit tests for loadAgent functionality and enhance agent loading logic - Introduced comprehensive unit tests for the `loadAgent` function, covering various scenarios including null and empty agent IDs, loading of ephemeral agents, and permission checks. - Enhanced the `initializeClient` function by moving `getConvoFiles` to the correct position in the database method exports, ensuring proper functionality. - Improved test coverage for agent loading, including handling of non-existent agents and user permissions. * chore: reorder memory method exports for consistency - Moved `deleteAllUserMemories` to the correct position in the exported memory methods, ensuring a consistent and logical order of method exports in `memory.ts`. |
||
|
|
38521381f4
|
🐘 feat: FerretDB Compatibility (#11769)
* feat: replace unsupported MongoDB aggregation operators for FerretDB compatibility Replace $lookup, $unwind, $sample, $replaceRoot, and $addFields aggregation stages which are unsupported on FerretDB v2.x (postgres-documentdb backend). - Prompt.js: Replace $lookup/$unwind/$project pipelines with find().select().lean() + attachProductionPrompts() batch helper. Replace $group/$replaceRoot/$sample in getRandomPromptGroups with distinct() + Fisher-Yates shuffle. - Agent/Prompt migration scripts: Replace $lookup anti-join pattern with distinct() + $nin two-step queries for finding un-migrated resources. All replacement patterns verified against FerretDB v2.7.0. * fix: use $pullAll for simple array removals, fix memberIds type mismatches Replace $pull with $pullAll for exact-value scalar array removals. Both operators work on MongoDB and FerretDB, but $pullAll is more explicit for exact matching (no condition expressions). Fix critical type mismatch bugs where ObjectId values were used against String[] memberIds arrays in Group queries: - config/delete-user.js: use string uid instead of ObjectId user._id - e2e/setup/cleanupUser.ts: convert userId.toString() before query Harden PermissionService.bulkUpdateResourcePermissions abort handling to prevent crash when abortTransaction is called after commitTransaction. All changes verified against FerretDB v2.7.0 and MongoDB Memory Server. * fix: harden transaction support probe for FerretDB compatibility Commit the transaction before aborting in supportsTransactions probe, and wrap abortTransaction in try-catch to prevent crashes when abort is called after a successful commit (observed behavior on FerretDB). * feat: add FerretDB compatibility test suite, retry utilities, and CI config Add comprehensive FerretDB integration test suite covering: - $pullAll scalar array operations - $pull with subdocument conditions - $lookup replacement (find + manual join) - $sample replacement (distinct + Fisher-Yates) - $bit and $bitsAllSet operations - Migration anti-join pattern - Multi-tenancy (useDb, scaling, write amplification) - Sharding proof-of-concept - Production operations (backup/restore, schema migration, deadlock retry) Add production retryWithBackoff utility for deadlock recovery during concurrent index creation on FerretDB/DocumentDB backends. Add UserController.spec.js tests for deleteUserController (runs in CI). Configure jest and eslint to isolate FerretDB tests from CI pipelines: - packages/data-schemas/jest.config.mjs: ignore misc/ directory - eslint.config.mjs: ignore packages/data-schemas/misc/ Include Docker Compose config for local FerretDB v2.7 + postgres-documentdb, dedicated jest/tsconfig for the test files, and multi-tenancy findings doc. * style: brace formatting in aclEntry.ts modifyPermissionBits * refactor: reorganize retry utilities and update imports - Moved retryWithBackoff utility to a new file `retry.ts` for better structure. - Updated imports in `orgOperations.ferretdb.spec.ts` to reflect the new location of retry utilities. - Removed old import statement for retryWithBackoff from index.ts to streamline exports. * test: add $pullAll coverage for ConversationTag and PermissionService Add integration tests for deleteConversationTag verifying $pullAll removes tags from conversations correctly, and for syncUserEntraGroupMemberships verifying $pullAll removes user from non-matching Entra groups while preserving local group membership. --------- |
||
|
|
1ecff83b20
|
🪦 fix: ACL-Safe User Account Deletion for Agents, Prompts, and MCP Servers (#12314)
* fix: use ACL ownership for prompt group cleanup on user deletion deleteUserPrompts previously called getAllPromptGroups with only an author filter, which defaults to searchShared=true and drops the author filter for shared/global project entries. This caused any user deleting their account to strip shared prompt group associations and ACL entries for other users. Replace the author-based query with ACL-based ownership lookup: - Find prompt groups where the user has OWNER permission (DELETE bit) - Only delete groups where the user is the sole owner - Preserve multi-owned groups and their ACL entries for other owners * fix: use ACL ownership for agent cleanup on user deletion deleteUserAgents used the deprecated author field to find and delete agents, then unconditionally removed all ACL entries for those agents. This could destroy ACL entries for agents shared with or co-owned by other users. Replace the author-based query with ACL-based ownership lookup: - Find agents where the user has OWNER permission (DELETE bit) - Only delete agents where the user is the sole owner - Preserve multi-owned agents and their ACL entries for other owners - Also clean up handoff edges referencing deleted agents * fix: add MCP server cleanup on user deletion User deletion had no cleanup for MCP servers, leaving solely-owned servers orphaned in the database with dangling ACL entries for other users. Add deleteUserMcpServers that follows the same ACL ownership pattern as prompt groups and agents: find servers with OWNER permission, check for sole ownership, and only delete those with no other owners. * style: fix prettier formatting in Prompt.spec.js * refactor: extract getSoleOwnedResourceIds to PermissionService The ACL sole-ownership detection algorithm was duplicated across deleteUserPrompts, deleteUserAgents, and deleteUserMcpServers. Centralizes the three-step pattern (find owned entries, find other owners, compute sole-owned set) into a single reusable utility. * refactor: use getSoleOwnedResourceIds in all deletion functions - Replace inline ACL queries with the centralized utility - Remove vestigial _req parameter from deleteUserPrompts - Use Promise.all for parallel project removal instead of sequential awaits - Disconnect live MCP sessions and invalidate tool cache before deleting sole-owned MCP server documents - Export deleteUserMcpServers for testability * test: improve deletion test coverage and quality - Move deleteUserPrompts call to beforeAll to eliminate execution-order dependency between tests - Standardize on test() instead of it() for consistency in Prompt.spec.js - Add assertion for deleting user's own ACL entry preservation on multi-owned agents - Add deleteUserMcpServers integration test suite with 6 tests covering sole-owner deletion, multi-owner preservation, session disconnect, cache invalidation, model-not-registered guard, and missing MCPManager - Add PermissionService mock to existing deleteUser.spec.js to fix import chain * fix: add legacy author-based fallback for unmigrated resources Resources created before the ACL system have author set but no AclEntry records. The sole-ownership detection returns empty for these, causing deleteUserPrompts, deleteUserAgents, and deleteUserMcpServers to silently skip them — permanently orphaning data on user deletion. Add a fallback that identifies author-owned resources with zero ACL entries (truly unmigrated) and includes them in the deletion set. This preserves the multi-owner safety of the ACL path while ensuring pre-ACL resources are still cleaned up regardless of migration status. * style: fix prettier formatting across all changed files * test: add resource type coverage guard for user deletion Ensures every ResourceType in the ACL system has a corresponding cleanup handler wired into deleteUserController. When a new ResourceType is added (e.g. WORKFLOW), this test fails immediately, preventing silent data orphaning on user account deletion. * style: fix import order in PermissionService destructure * test: add opt-out set and fix test lifecycle in coverage guard Add NO_USER_CLEANUP_NEEDED set for resource types that legitimately require no per-user deletion. Move fs.readFileSync into beforeAll so path errors surface as clean test failures instead of unhandled crashes. |
||
|
|
acd07e8085
|
🗝️ fix: Exempt Admin-Trusted Domains from MCP OAuth Validation (#12255)
* fix: exempt allowedDomains from MCP OAuth SSRF checks (#12254) The SSRF guard in validateOAuthUrl was context-blind — it blocked private/internal OAuth endpoints even for admin-trusted MCP servers listed in mcpSettings.allowedDomains. Add isHostnameAllowed() to domain.ts and skip SSRF checks in validateOAuthUrl when the OAuth endpoint hostname matches an allowed domain. * refactor: thread allowedDomains through MCP connection stack Pass allowedDomains from MCPServersRegistry through BasicConnectionOptions, MCPConnectionFactory, and into MCPOAuthHandler method calls so the OAuth layer can exempt admin-trusted domains from SSRF validation. * test: add allowedDomains bypass tests and fix registry mocks Add isHostnameAllowed unit tests (exact, wildcard, case-insensitive, private IPs). Add MCPOAuthSecurity tests covering the allowedDomains bypass for initiateOAuthFlow, refreshOAuthTokens, and revokeOAuthToken. Update registry mocks to include getAllowedDomains. * fix: enforce protocol/port constraints in OAuth allowedDomains bypass Replace isHostnameAllowed (hostname-only check) with isOAuthUrlAllowed which parses the full OAuth URL and matches against allowedDomains entries including protocol and explicit port constraints — mirroring isDomainAllowedCore's allowlist logic. Prevents a port-scoped entry like 'https://auth.internal:8443' from also exempting other ports. * test: cover auto-discovery and branch-3 refresh paths with allowedDomains Add three new integration tests using a real OAuth test server: - auto-discovered OAuth endpoints allowed when server IP is in allowedDomains - auto-discovered endpoints rejected when allowedDomains doesn't match - refreshOAuthTokens branch 3 (no clientInfo/config) with allowedDomains bypass Also rename describe block from ephemeral issue number to durable name. * docs: explain intentional absence of allowedDomains in completeOAuthFlow Prevents future contributors from assuming a missing parameter during security audits — URLs are pre-validated during initiateOAuthFlow. * test: update initiateOAuthFlow assertion for allowedDomains parameter * perf: avoid redundant URL parse for admin-trusted OAuth endpoints Move isOAuthUrlAllowed check before the hostname extraction so admin-trusted URLs short-circuit with a single URL parse instead of two. The hostname extraction (new URL) is now deferred to the SSRF-check path where it's actually needed. |
||
|
|
71a3b48504
|
🔑 fix: Require OTP Verification for 2FA Re-Enrollment and Backup Code Regeneration (#12223)
* fix: require OTP verification for 2FA re-enrollment and backup code regeneration * fix: require OTP verification for account deletion when 2FA is enabled * refactor: Improve code formatting and readability in TwoFactorController and UserController - Reformatted code in TwoFactorController and UserController for better readability by aligning parameters and breaking long lines. - Updated test cases in deleteUser.spec.js and TwoFactorController.spec.js to enhance clarity by formatting object parameters consistently. * refactor: Consolidate OTP and backup code verification logic in TwoFactorController and UserController - Introduced a new `verifyOTPOrBackupCode` function to streamline the verification process for TOTP tokens and backup codes across multiple controllers. - Updated the `enable2FA`, `disable2FA`, and `deleteUserController` methods to utilize the new verification function, enhancing code reusability and readability. - Adjusted related tests to reflect the changes in verification logic, ensuring consistent behavior across different scenarios. - Improved error handling and response messages for verification failures, providing clearer feedback to users. * chore: linting * refactor: Update BackupCodesItem component to enhance OTP verification logic - Consolidated OTP input handling by moving the 2FA verification UI logic to a more consistent location within the component. - Improved the state management for OTP readiness, ensuring the regenerate button is only enabled when the OTP is ready. - Cleaned up imports by removing redundant type imports, enhancing code clarity and maintainability. * chore: lint * fix: stage 2FA re-enrollment in pending fields to prevent disarmament window enable2FA now writes to pendingTotpSecret/pendingBackupCodes instead of overwriting the live fields. confirm2FA performs the atomic swap only after the new TOTP code is verified. If the user abandons mid-flow, their existing 2FA remains active and intact. |
||
|
|
599f4a11f1
|
🛡️ fix: Secure MCP/Actions OAuth Flows, Resolve Race Condition & Tool Cache Cleanup (#11756)
* 🔧 fix: Update OAuth error message for clarity - Changed the default error message in the OAuth error route from 'Unknown error' to 'Unknown OAuth error' to provide clearer context during authentication failures. * 🔒 feat: Enhance OAuth flow with CSRF protection and session management - Implemented CSRF protection for OAuth flows by introducing `generateOAuthCsrfToken`, `setOAuthCsrfCookie`, and `validateOAuthCsrf` functions. - Added session management for OAuth with `setOAuthSession` and `validateOAuthSession` middleware. - Updated routes to bind CSRF tokens for MCP and action OAuth flows, ensuring secure authentication. - Enhanced tests to validate CSRF handling and session management in OAuth processes. * 🔧 refactor: Invalidate cached tools after user plugin disconnection - Added a call to `invalidateCachedTools` in the `updateUserPluginsController` to ensure that cached tools are refreshed when a user disconnects from an MCP server after a plugin authentication update. This change improves the accuracy of tool data for users. * chore: imports order * fix: domain separator regex usage in ToolService - Moved the declaration of `domainSeparatorRegex` to avoid redundancy in the `loadActionToolsForExecution` function, improving code clarity and performance. * chore: OAuth flow error handling and CSRF token generation - Enhanced the OAuth callback route to validate the flow ID format, ensuring proper error handling for invalid states. - Updated the CSRF token generation function to require a JWT secret, throwing an error if not provided, which improves security and clarity in token generation. - Adjusted tests to reflect changes in flow ID handling and ensure robust validation across various scenarios. |
||
|
|
6279ea8dd7
|
🛸 feat: Remote Agent Access with External API Support (#11503)
* 🪪 feat: Microsoft Graph Access Token Placeholder for MCP Servers (#10867) * feat: MCP Graph Token env var * Addressing copilot remarks * Addressed Copilot review remarks * Fixed graphtokenservice mock in MCP test suite * fix: remove unnecessary type check and cast in resolveGraphTokensInRecord * ci: add Graph Token integration tests in MCPManager * refactor: update user type definitions to use Partial<IUser> in multiple functions * test: enhance MCP tests for graph token processing and user placeholder resolution - Added comprehensive tests to validate the interaction between preProcessGraphTokens and processMCPEnv. - Ensured correct resolution of graph tokens and user placeholders in various configurations. - Mocked OIDC utilities to facilitate testing of token extraction and validation. - Verified that original options remain unchanged after processing. * chore: import order * chore: imports --------- Co-authored-by: Danny Avila <danny@librechat.ai> * WIP: OpenAI-compatible API for LibreChat agents - Added OpenAIChatCompletionController for handling chat completions. - Introduced ListModelsController and GetModelController for listing and retrieving agent details. - Created routes for OpenAI API endpoints, including /v1/chat/completions and /v1/models. - Developed event handlers for streaming responses in OpenAI format. - Implemented request validation and error handling for API interactions. - Integrated content aggregation and response formatting to align with OpenAI specifications. This commit establishes a foundational API for interacting with LibreChat agents in a manner compatible with OpenAI's chat completion interface. * refactor: OpenAI-spec content aggregation for improved performance and clarity * fix: OpenAI chat completion controller with safe user handling for correct tool loading * refactor: Remove conversation ID from OpenAI response context and related handlers * refactor: OpenAI chat completion handling with streaming support - Introduced a lightweight tracker for streaming responses, allowing for efficient tracking of emitted content and usage metadata. - Updated the OpenAIChatCompletionController to utilize the new tracker, improving the handling of streaming and non-streaming responses. - Refactored event handlers to accommodate the new streaming logic, ensuring proper management of tool calls and content aggregation. - Adjusted response handling to streamline error reporting during streaming sessions. * WIP: Open Responses API with core service, types, and handlers - Added Open Responses API module with comprehensive types and enums. - Implemented core service for processing requests, including validation and input conversion. - Developed event handlers for streaming responses and non-streaming aggregation. - Established response building logic and error handling mechanisms. - Created detailed types for input and output content, ensuring compliance with Open Responses specification. * feat: Implement response storage and retrieval in Open Responses API - Added functionality to save user input messages and assistant responses to the database when the `store` flag is set to true. - Introduced a new endpoint to retrieve stored responses by ID, allowing users to access previous interactions. - Enhanced the response creation process to include database operations for conversation and message storage. - Implemented tests to validate the storage and retrieval of responses, ensuring correct behavior for both existing and non-existent response IDs. * refactor: Open Responses API with additional token tracking and validation - Added support for tracking cached tokens in response usage, improving token management. - Updated response structure to include new properties for top log probabilities and detailed usage metrics. - Enhanced tests to validate the presence and types of new properties in API responses, ensuring compliance with updated specifications. - Refactored response handling to accommodate new fields and improve overall clarity and performance. * refactor: Update reasoning event handlers and types for consistency - Renamed reasoning text events to simplify naming conventions, changing `emitReasoningTextDelta` to `emitReasoningDelta` and `emitReasoningTextDone` to `emitReasoningDone`. - Updated event types in the API to reflect the new naming, ensuring consistency across the codebase. - Added `logprobs` property to output events for enhanced tracking of log probabilities. * feat: Add validation for streaming events in Open Responses API tests * feat: Implement response.created event in Open Responses API - Added emitResponseCreated function to emit the response.created event as the first event in the streaming sequence, adhering to the Open Responses specification. - Updated createResponse function to emit response.created followed by response.in_progress. - Enhanced tests to validate the order of emitted events, ensuring response.created is triggered before response.in_progress. * feat: Responses API with attachment event handling - Introduced `createResponsesToolEndCallback` to handle attachment events in the Responses API, emitting `librechat:attachment` events as per the Open Responses extension specification. - Updated the `createResponse` function to utilize the new callback for processing tool outputs and emitting attachments during streaming. - Added helper functions for writing attachment events and defined types for attachment data, ensuring compatibility with the Open Responses protocol. - Enhanced tests to validate the integration of attachment events within the Responses API workflow. * WIP: remote agent auth * fix: Improve loading state handling in AgentApiKeys component - Updated the rendering logic to conditionally display loading spinner and API keys based on the loading state. - Removed unnecessary imports and streamlined the component for better readability. * refactor: Update API key access handling in routes - Replaced `checkAccess` with `generateCheckAccess` for improved access control. - Consolidated access checks into a single `checkApiKeyAccess` function, enhancing code readability and maintainability. - Streamlined route definitions for creating, listing, retrieving, and deleting API keys. * fix: Add permission handling for REMOTE_AGENT resource type * feat: Enhance permission handling for REMOTE_AGENT resources - Updated the deleteAgent and deleteUserAgents functions to handle permissions for both AGENT and REMOTE_AGENT resource types. - Introduced new functions to enrich REMOTE_AGENT principals and backfill permissions for AGENT owners. - Modified createAgentHandler and duplicateAgentHandler to grant permissions for REMOTE_AGENT alongside AGENT. - Added utility functions for retrieving effective permissions for REMOTE_AGENT resources, ensuring consistent access control across the application. * refactor: Rename and update roles for remote agent access - Changed role name from API User to Editor in translation files for clarity. - Updated default editor role ID from REMOTE_AGENT_USER to REMOTE_AGENT_EDITOR in resource configurations. - Adjusted role localization to reflect the new Editor role. - Modified access permissions to align with the updated role definitions across the application. * feat: Introduce remote agent permissions and update access handling - Added support for REMOTE_AGENTS in permission schemas, including use, create, share, and share_public permissions. - Updated the interface configuration to include remote agent settings. - Modified middleware and API key access checks to align with the new remote agent permission structure. - Enhanced role defaults to incorporate remote agent permissions, ensuring consistent access control across the application. * refactor: Update AgentApiKeys component and permissions handling - Refactored the AgentApiKeys component to improve structure and readability, including the introduction of ApiKeysContent for better separation of concerns. - Updated CreateKeyDialog to accept an onKeyCreated callback, enhancing its functionality. - Adjusted permission checks in Data component to use REMOTE_AGENTS and USE permissions, aligning with recent permission schema changes. - Enhanced loading state handling and dialog management for a smoother user experience. * refactor: Update remote agent access checks in API routes - Replaced existing access checks with `generateCheckAccess` for remote agents in the API keys and agents routes. - Introduced specific permission checks for creating, listing, retrieving, and deleting API keys, enhancing access control. - Improved code structure by consolidating permission handling for remote agents across multiple routes. * fix: Correct query parameters in ApiKeysContent component - Updated the useGetAgentApiKeysQuery call to include an object for the enabled parameter, ensuring proper functionality when the component is open. - This change improves the handling of API key retrieval based on the component's open state. * feat: Implement remote agents permissions and update API routes - Added new API route for updating remote agents permissions, enhancing role management capabilities. - Introduced remote agents permissions handling in the AgentApiKeys component, including a dedicated settings dialog. - Updated localization files to include new remote agents permission labels for better user experience. - Refactored data provider to support remote agents permissions updates, ensuring consistent access control across the application. * feat: Add remote agents permissions to role schema and interface - Introduced new permissions for REMOTE_AGENTS in the role schema, including USE, CREATE, SHARE, and SHARE_PUBLIC. - Updated the IRole interface to reflect the new remote agents permissions structure, enhancing role management capabilities. * feat: Add remote agents settings button to API keys dialog * feat: Update AgentFooter to include remote agent sharing permissions - Refactored access checks to incorporate permissions for sharing remote agents. - Enhanced conditional rendering logic to allow sharing by users with remote agent permissions. - Improved loading state handling for remote agent permissions, ensuring a smoother user experience. * refactor: Update API key creation access check and localization strings - Replaced the access check for creating API keys to use the existing remote agents access check. - Updated localization strings to correct the descriptions for remote agent permissions, ensuring clarity in user interface. * fix: resource permission mapping to include remote agents - Changed the resourceToPermissionMap to use a Partial<Record> for better flexibility. - Added mapping for REMOTE_AGENT permissions, enhancing the sharing capabilities for remote agents. * feat: Implement remote access checks for agent models - Enhanced ListModelsController and GetModelController to include checks for user permissions on remote agents. - Integrated findAccessibleResources to filter agents based on VIEW permission for REMOTE_AGENT. - Updated response handling to ensure users can only access agents they have permissions for, improving security and access control. * fix: Update user parameter type in processUserPlaceholders function - Changed the user parameter type in the processUserPlaceholders function from Partial<Partial<IUser>> to Partial<IUser> for improved type clarity and consistency. * refactor: Simplify integration test structure by removing conditional describe - Replaced conditional describeWithApiKey with a standard describe for all integration tests in responses.spec.js. - This change enhances test clarity and ensures all tests are executed consistently, regardless of the SKIP_INTEGRATION_TESTS flag. * test: Update AgentFooter tests to reflect new grant access dialog ID - Changed test IDs for the grant access dialog in AgentFooter tests to include the resource type, ensuring accurate identification in the test cases. - This update improves test clarity and aligns with recent changes in the component's implementation. * test: Enhance integration tests for Open Responses API - Updated integration tests in responses.spec.js to utilize an authRequest helper for consistent authorization handling across all test cases. - Introduced a test user and API key creation to improve test setup and ensure proper permission checks for remote agents. - Added checks for existing access roles and created necessary roles if they do not exist, enhancing test reliability and coverage. * feat: Extend accessRole schema to include remoteAgent resource type - Updated the accessRole schema to add 'remoteAgent' to the resourceType enum, enhancing the flexibility of role assignments and permissions management. * test: refactored test setup to create a minimal Express app for responses routes, enhancing test structure and maintainability. * test: Enhance abort.spec.js by mocking additional modules for improved test isolation - Updated the test setup in abort.spec.js to include actual implementations of '@librechat/data-schemas' and '@librechat/api' while maintaining mock functionality. - This change improves test reliability and ensures that the tests are more representative of the actual module behavior. * refactor: Update conversation ID generation to use UUID - Replaced the nanoid with uuidv4 for generating conversation IDs in the createResponse function, enhancing uniqueness and consistency in ID generation. * test: Add remote agent access roles to AccessRole model tests - Included additional access roles for remote agents (REMOTE_AGENT_EDITOR, REMOTE_AGENT_OWNER, REMOTE_AGENT_VIEWER) in the AccessRole model tests to ensure comprehensive coverage of role assignments and permissions management. * chore: Add deletion of user agent API keys in user deletion process - Updated the user deletion process in UserController and delete-user.js to include the removal of user agent API keys, ensuring comprehensive cleanup of user data upon account deletion. * test: Add remote agents permissions to permissions.spec.ts - Enhanced the permissions tests by including comprehensive permission settings for remote agents across various scenarios, ensuring accurate validation of access controls for remote agent roles. * chore: Update remote agents translations for clarity and consistency - Removed outdated remote agents translation entries and added revised entries to improve clarity on API key creation and sharing permissions for remote agents. This enhances user understanding of the available functionalities. * feat: Add indexing and TTL for agent API keys - Introduced an index on the `key` field for improved query performance. - Added a TTL index on the `expiresAt` field to enable automatic cleanup of expired API keys, ensuring efficient management of stored keys. * chore: Update API route documentation for clarity - Revised comments in the agents route file to clarify the handling of API key authentication. - Removed outdated endpoint listings to streamline the documentation and focus on current functionality. --------- Co-authored-by: Max Sanna <max@maxsanna.com> |
||
|
|
99f8bd2ce6
|
🏗️ feat: Dynamic MCP Server Infrastructure with Access Control (#10787)
* Feature: Dynamic MCP Server with Full UI Management * 🚦 feat: Add MCP Connection Status icons to MCPBuilder panel (#10805) * feature: Add MCP server connection status icons to MCPBuilder panel * refactor: Simplify MCPConfigDialog rendering in MCPBuilderPanel --------- Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com> Co-authored-by: Danny Avila <danny@librechat.ai> * fix: address code review feedback for MCP server management - Fix OAuth secret preservation to avoid mutating input parameter by creating a merged config copy in ServerConfigsDB.update() - Improve error handling in getResourcePermissionsMap to propagate critical errors instead of silently returning empty Map - Extract duplicated MCP server filter logic by exposing selectableServers from useMCPServerManager hook and using it in MCPSelect component * test: Update PermissionService tests to throw errors on invalid resource types - Changed the test for handling invalid resource types to ensure it throws an error instead of returning an empty permissions map. - Updated the expectation to check for the specific error message when an invalid resource type is provided. * feat: Implement retry logic for MCP server creation to handle race conditions - Enhanced the createMCPServer method to include retry logic with exponential backoff for handling duplicate key errors during concurrent server creation. - Updated tests to verify that all concurrent requests succeed and that unique server names are generated. - Added a helper function to identify MongoDB duplicate key errors, improving error handling during server creation. * refactor: StatusIcon to use CircleCheck for connected status - Replaced the PlugZap icon with CircleCheck in the ConnectedStatusIcon component to better represent the connected state. - Ensured consistent icon usage across the component for improved visual clarity. * test: Update AccessControlService tests to throw errors on invalid resource types - Modified the test for invalid resource types to ensure it throws an error with a specific message instead of returning an empty permissions map. - This change enhances error handling and improves test coverage for the AccessControlService. * fix: Update error message for missing server name in MCP server retrieval - Changed the error message returned when the server name is not provided from 'MCP ID is required' to 'Server name is required' for better clarity and accuracy in the API response. --------- Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com> Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
|
04a4a2aa44
|
🧵 refactor: Migrate Endpoint Initialization to TypeScript (#10794)
* refactor: move endpoint initialization methods to typescript * refactor: move agent init to packages/api - Introduced `initialize.ts` for agent initialization, including file processing and tool loading. - Updated `resources.ts` to allow optional appConfig parameter. - Enhanced endpoint configuration handling in various initialization files to support model parameters. - Added new artifacts and prompts for React component generation. - Refactored existing code to improve type safety and maintainability. * refactor: streamline endpoint initialization and enhance type safety - Updated initialization functions across various endpoints to use a consistent request structure, replacing `unknown` types with `ServerResponse`. - Simplified request handling by directly extracting keys from the request body. - Improved type safety by ensuring user IDs are safely accessed with optional chaining. - Removed unnecessary parameters and streamlined model options handling for better clarity and maintainability. * refactor: moved ModelService and extractBaseURL to packages/api - Added comprehensive tests for the models fetching functionality, covering scenarios for OpenAI, Anthropic, Google, and Ollama models. - Updated existing endpoint index to include the new models module. - Enhanced utility functions for URL extraction and model data processing. - Improved type safety and error handling across the models fetching logic. * refactor: consolidate utility functions and remove unused files - Merged `deriveBaseURL` and `extractBaseURL` into the `@librechat/api` module for better organization. - Removed redundant utility files and their associated tests to streamline the codebase. - Updated imports across various client files to utilize the new consolidated functions. - Enhanced overall maintainability by reducing the number of utility modules. * refactor: replace ModelService references with direct imports from @librechat/api and remove ModelService file * refactor: move encrypt/decrypt methods and key db methods to data-schemas, use `getProviderConfig` from `@librechat/api` * chore: remove unused 'res' from options in AgentClient * refactor: file model imports and methods - Updated imports in various controllers and services to use the unified file model from '~/models' instead of '~/models/File'. - Consolidated file-related methods into a new file methods module in the data-schemas package. - Added comprehensive tests for file methods including creation, retrieval, updating, and deletion. - Enhanced the initializeAgent function to accept dependency injection for file-related methods. - Improved error handling and logging in file methods. * refactor: streamline database method references in agent initialization * refactor: enhance file method tests and update type references to IMongoFile * refactor: consolidate database method imports in agent client and initialization * chore: remove redundant import of initializeAgent from @librechat/api * refactor: move checkUserKeyExpiry utility to @librechat/api and update references across endpoints * refactor: move updateUserPlugins logic to user.ts and simplify UserController * refactor: update imports for user key management and remove UserService * refactor: remove unused Anthropics and Bedrock endpoint files and clean up imports * refactor: consolidate and update encryption imports across various files to use @librechat/data-schemas * chore: update file model mock to use unified import from '~/models' * chore: import order * refactor: remove migrated to TS agent.js file and its associated logic from the endpoints * chore: add reusable function to extract imports from source code in unused-packages workflow * chore: enhance unused-packages workflow to include @librechat/api dependencies and improve dependency extraction * chore: improve dependency extraction in unused-packages workflow with enhanced error handling and debugging output * chore: add detailed debugging output to unused-packages workflow for better visibility into unused dependencies and exclusion lists * chore: refine subpath handling in unused-packages workflow to correctly process scoped and non-scoped package imports * chore: clean up unused debug output in unused-packages workflow and reorganize type imports in initialize.ts |
||
|
|
ad6ba4b6d1
|
🧬 refactor: Wire Database Methods into MCP Package via Registry Pattern (#10715)
* Refactor: MCPServersRegistry Singleton Pattern with Dependency Injection for DB methods consumption * refactor: error handling in MCP initialization and improve logging for MCPServersRegistry instance creation. - Added checks for mongoose instance in ServerConfigsDB constructor and refined error messages for clarity. - Reorder and use type imports --------- Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com> Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
|
846e34b1d7
|
🗑️ fix: Remove All User Metadata on Deletion (#10534)
* remove all user metadata on deletion * chore: import order * fix: Update JSDoc types for deleteMessages function parameters and return value * fix: Enhance user deletion process by removing associated data and updating group memberships * fix: Add missing config middleware to user deletion route * fix: Refactor agent and prompt deletion processes to bulk delete and remove associated ACL entries * fix: Add deletion of OAuth tokens and ACL entries in user deletion process --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
|
ce7e6edad8
|
🔄 refactor: MCP Registry System with Distributed Caching (#10191)
* refactor: Restructure MCP registry system with caching - Split MCPServersRegistry into modular components: - MCPServerInspector: handles server inspection and health checks - MCPServersInitializer: manages server initialization logic - MCPServersRegistry: simplified registry coordination - Add distributed caching layer: - ServerConfigsCacheRedis: Redis-backed configuration cache - ServerConfigsCacheInMemory: in-memory fallback cache - RegistryStatusCache: distributed leader election state - Add promise utilities (withTimeout) replacing Promise.race patterns - Add comprehensive cache integration tests for all cache implementations - Remove unused MCPManager.getAllToolFunctions method * fix: Update OAuth flow to include user-specific headers * chore: Update Jest configuration to ignore additional test files - Added patterns to ignore files ending with .helper.ts and .helper.d.ts in testPathIgnorePatterns for cleaner test runs. * fix: oauth headers in callback * chore: Update Jest testPathIgnorePatterns to exclude helper files - Modified testPathIgnorePatterns in package.json to ignore files ending with .helper.ts and .helper.d.ts for cleaner test execution. * ci: update test mocks --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
|
5ce67b5b71
|
📮 feat: Custom OAuth Headers Support for MCP Server Config (#10014)
* add oauth_headers field to mcp options * wrap fetch to pass oauth headers * fix order * consolidate headers passing * fix tests |
||
|
|
838fb53208
|
🔃 refactor: Decouple Effects from AppService, move to data-schemas (#9974)
* chore: linting for `loadCustomConfig` * refactor: decouple CDN init and variable/health checks from AppService * refactor: move AppService to packages/data-schemas * chore: update AppConfig import path to use data-schemas * chore: update JsonSchemaType import path to use data-schemas * refactor: update UserController to import webSearchKeys and redefine FunctionTool typedef * chore: remove AppService.js * refactor: update AppConfig interface to use Partial<TCustomConfig> and make paths and fileStrategies optional * refactor: update checkConfig function to accept Partial<TCustomConfig> * chore: fix types * refactor: move handleRateLimits to startup checks as is an effect * test: remove outdated rate limit tests from AppService.spec and add new handleRateLimits tests in checks.spec |
||
|
|
5b1a31ef4d
|
🔄 refactor: Optimize MCP Tool Initialization
🔄 refactor: Optimize MCP Tool Initialization
fix: update tool caching to use separated mcp logic
refactor: Replace `req.user` with `userId` in MCP handling functions
refactor: Replace `req` parameter with `userId` in file search tool functions
fix: Update user connection parameter to use object format in reinitMCPServer
refactor: Simplify MCP tool creation logic and improve handling of tool configurations to avoid capturing too much in closures
refactor: ensure MCP available tools are fetched from cache only when needed
|
||
|
|
386900fb4f
|
🧰 refactor: Decouple MCP Tools from System Tools (#9748) | ||
|
|
04c3a5a861
|
🔌 feat: Revoke MCP OAuth Credentials (#9464)
* revocation metadata fields * store metadata * get client info and meta * revoke oauth tokens * delete flow * uninstall oauth mcp * revoke button * revoke oauth refactor, add comments, test * adjust for clarity * test deleteFlow * handle metadata type * no mutation * adjust for clarity * styling * restructure for clarity * move token-specific stuff * use mcpmanager's oauth servers * fix typo * fix addressing of oauth prop * log prefix * remove debug log |
||
|
|
49e8443ec5
|
✂️ refactor: MCP UI Separation for Agents (#9237)
* refactor: MCP UI Separation for Agents (Dustin WIP)
feat: separate MCPs into their own lists away from tools + actions and add the status indicator functionality from chat to their dropdown ui
fix: spotify mcp was not persisting on agent creation
feat: show disconnected saved servers and their tools in agent mcp list in created agents
fix: select-all regression fixed (caused by deleting tools we were drawing from for rendering list)
fix: dont show all mcps, only those installed in agent in list
feat: separate ToolSelectDialog for MCPServerTools
fix: uninitialized mcp servers not showing as added in toolselectdialog
refactor: reduce looping in AgentPanelContext for categorizing groups and mcps
refactor: split ToolSelectDialog and MCPToolSelectDialog functionality (still needs customization for custom user vars)
chore: address ESLint comments
chore: address ESLint comments
feat: one-click initialization on MCP servers in agent builder
fix: stop propagation triggering reinit on caret click
refactor: split uninitialized MCPs component from initialized MCPs
feat: new mcp tool select dialog ui with custom user vars
feat: show initialization state for CUV configurable MCPs too
chore: remove unused localization string
fix: deselecting all tools caused a re-render
fix: remove subtools so removal from MCPToolSelectDialog works more consistently
feat: added servers have all tools enabled by default
feat: mcp server list now alphabetical to prevent annoying ui behavior of servers jumping around depending on tool selection
fix: filter out placeholder group mcp tools from any actual tool calls / definitions
feat: indicator now takes you to config dialog for uninitialized servers
feat: show previously configured mcp servers that are now missing from the yaml
feat: select all enabled by default on first add to mcp server list
chore: address ESLint comments
* refactor: MCP UI Separation for Agents (Danny WIP)
chore: remove use of `{serverName}_mcp_{serverName}`
chore: import order
WIP: separate component concerns
refactor: streamline agent mcp tools
refactor: unify MCP server handling and improve tool visibility logic, remove unnecessary normalization or sorting, remove nesting button, make variable names clear
refactor: rename mcpServerIds to mcpServerNames for clarity and consistency across components
refactor: remove groupedMCPTools and toolToServerMap, streamline MCP server handling in context and components to effectively utilize mcpServersMap
refactor: optimize tool selection logic by replacing array includes with Set for improved performance
chore: add error logging for failed auth URL parsing in ToolCall component
refactor: enhance MCP tool handling by improving server name management and updating UI elements for better clarity
* refactor: decouple connection status from useMCPServerManager with useMCPConnectionStatus
* fix: improve MCP tool validation logic to handle unconfigured servers
* chore: enhance log message clarity for MCP server disconnection in updateUserPluginsController
* refactor: simplify connection status extraction in useMCPConnectionStatus hook
* refactor: improve initializing UX
* chore: replace string literal with ResourceType constant in useResourcePermissions
* refactor: cleanup code, remove redundancies, rename variables for clarity
* chore: add back filtering and sorting for mcp tools dialog
* refactor: initializeServer to return response and early return
* refactor: enhance server initialization logic and improve UI for OAuth interaction
* chore: clarify warning message for unconfigured MCP server in handleTools
* refactor: prevent CustomUserVarsSection from submitting tools dialog form
* fix: nested button of button issue in UninitializedMCPTool
* feat: add functionality to revoke custom user variables in MCPToolSelectDialog
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
|
9a210971f5
|
🛜 refactor: Streamline App Config Usage (#9234)
* WIP: app.locals refactoring
WIP: appConfig
fix: update memory configuration retrieval to use getAppConfig based on user role
fix: update comment for AppConfig interface to clarify purpose
🏷️ refactor: Update tests to use getAppConfig for endpoint configurations
ci: Update AppService tests to initialize app config instead of app.locals
ci: Integrate getAppConfig into remaining tests
refactor: Update multer storage destination to use promise-based getAppConfig and improve error handling in tests
refactor: Rename initializeAppConfig to setAppConfig and update related tests
ci: Mock getAppConfig in various tests to provide default configurations
refactor: Update convertMCPToolsToPlugins to use mcpManager for server configuration and adjust related tests
chore: rename `Config/getAppConfig` -> `Config/app`
fix: streamline OpenAI image tools configuration by removing direct appConfig dependency and using function parameters
chore: correct parameter documentation for imageOutputType in ToolService.js
refactor: remove `getCustomConfig` dependency in config route
refactor: update domain validation to use appConfig for allowed domains
refactor: use appConfig registration property
chore: remove app parameter from AppService invocation
refactor: update AppConfig interface to correct registration and turnstile configurations
refactor: remove getCustomConfig dependency and use getAppConfig in PluginController, multer, and MCP services
refactor: replace getCustomConfig with getAppConfig in STTService, TTSService, and related files
refactor: replace getCustomConfig with getAppConfig in Conversation and Message models, update tempChatRetention functions to use AppConfig type
refactor: update getAppConfig calls in Conversation and Message models to include user role for temporary chat expiration
ci: update related tests
refactor: update getAppConfig call in getCustomConfigSpeech to include user role
fix: update appConfig usage to access allowedDomains from actions instead of registration
refactor: enhance AppConfig to include fileStrategies and update related file strategy logic
refactor: update imports to use normalizeEndpointName from @librechat/api and remove redundant definitions
chore: remove deprecated unused RunManager
refactor: get balance config primarily from appConfig
refactor: remove customConfig dependency for appConfig and streamline loadConfigModels logic
refactor: remove getCustomConfig usage and use app config in file citations
refactor: consolidate endpoint loading logic into loadEndpoints function
refactor: update appConfig access to use endpoints structure across various services
refactor: implement custom endpoints configuration and streamline endpoint loading logic
refactor: update getAppConfig call to include user role parameter
refactor: streamline endpoint configuration and enhance appConfig usage across services
refactor: replace getMCPAuthMap with getUserMCPAuthMap and remove unused getCustomConfig file
refactor: add type annotation for loadedEndpoints in loadEndpoints function
refactor: move /services/Files/images/parse to TS API
chore: add missing FILE_CITATIONS permission to IRole interface
refactor: restructure toolkits to TS API
refactor: separate manifest logic into its own module
refactor: consolidate tool loading logic into a new tools module for startup logic
refactor: move interface config logic to TS API
refactor: migrate checkEmailConfig to TypeScript and update imports
refactor: add FunctionTool interface and availableTools to AppConfig
refactor: decouple caching and DB operations from AppService, make part of consolidated `getAppConfig`
WIP: fix tests
* fix: rebase conflicts
* refactor: remove app.locals references
* refactor: replace getBalanceConfig with getAppConfig in various strategies and middleware
* refactor: replace appConfig?.balance with getBalanceConfig in various controllers and clients
* test: add balance configuration to titleConvo method in AgentClient tests
* chore: remove unused `openai-chat-tokens` package
* chore: remove unused imports in initializeMCPs.js
* refactor: update balance configuration to use getAppConfig instead of getBalanceConfig
* refactor: integrate configMiddleware for centralized configuration handling
* refactor: optimize email domain validation by removing unnecessary async calls
* refactor: simplify multer storage configuration by removing async calls
* refactor: reorder imports for better readability in user.js
* refactor: replace getAppConfig calls with req.config for improved performance
* chore: replace getAppConfig calls with req.config in tests for centralized configuration handling
* chore: remove unused override config
* refactor: add configMiddleware to endpoint route and replace getAppConfig with req.config
* chore: remove customConfig parameter from TTSService constructor
* refactor: pass appConfig from request to processFileCitations for improved configuration handling
* refactor: remove configMiddleware from endpoint route and retrieve appConfig directly in getEndpointsConfig if not in `req.config`
* test: add mockAppConfig to processFileCitations tests for improved configuration handling
* fix: pass req.config to hasCustomUserVars and call without await after synchronous refactor
* fix: type safety in useExportConversation
* refactor: retrieve appConfig using getAppConfig in PluginController and remove configMiddleware from plugins route, to avoid always retrieving when plugins are cached
* chore: change `MongoUser` typedef to `IUser`
* fix: Add `user` and `config` fields to ServerRequest and update JSDoc type annotations from Express.Request to ServerRequest
* fix: remove unused setAppConfig mock from Server configuration tests
|
||
|
|
3547873bc4
|
🧑💻 refactor: Secure Field Selection for 2FA & API Build Sourcemap (#9087)
* refactor: `packages/api` build scripts for better inline debugging * refactor: Explicitly select secure fields as no longer returned by default, exclude backupCodes from user data retrieval in authentication and 2FA processes * refactor: Backup Codes UI to not expect backup codes, only regeneration * refactor: Ensure secure fields are deleted from user data in getUserController |
||
|
|
9ca1847535
|
🔧 refactor: customUserVar Error Normalization (#8950)
* fix: localization string had unused template var * fix: add normalizeHttpError to hopefully stop UI hangs when an error is returned in UserController - Ensures updateUserPluginsController always returns valid HTTP status codes instead of undefined - Add normalizeHttpError() helper to safely extract status/message from errors - Default to 400 status code when Error.status is undefined/invalid * refactor: move normalizeHttpError to packages/api |
||
|
|
74d8a3824c
|
🔌 feat: MCP Reinitialization and OAuth in UI (#8598)
* ✨ feat: Add connection status endpoint for MCP servers
- Implemented a new endpoint to retrieve the connection status of all MCP servers without disconnecting idle connections.
- Enhanced MCPManager class with a method to get all user-specific connections.
* feat: add silencer arg to loadCustomConfig function to conditionally print config details
- Modified loadCustomConfig to accept a printConfig parameter that allows me to prevent the entire custom config being printed every time it is called
* fix: new status endpoint actually works now, changes to manager.ts to support it
- Updated the connection status endpoint to utilize Maps for app and user connections, rather than incorrectly treating them as objects.
- Introduced a new method + variable in MCPManager to track servers requiring OAuth discovered at startup.
- Stopped OAuth flow from continuing once detected during startup for a new connection
* refactor: Remove hasAuthConfig since we can get that on the frontend without needing to use the endpoint
* feat: Add MCP connection status query and query key for new endpoint
- Introduced a new query hook `useMCPConnectionStatusQuery` to fetch the connection status of MCP servers.
- Added request in data-service
- Defined the API endpoint for retrieving MCP connection status in api-endpoints.ts.
- Defined new types for MCP connection status responses in the types module.
- Added mcpConnectionStatus key
* feat: Enhance MCPSelect component with connection status and server configuration
- Added connection status handling for MCP servers using the new `useMCPConnectionStatusQuery` hook.
- Implemented logic to display appropriate status icons based on connection state and authentication configuration.
- Updated the server selection logic to utilize configured MCP servers from the startup configuration.
- Refactored the rendering of configuration buttons and status indicators for improved user interaction.
* refactor: move MCPConfigDialog to its own MCP subdir in ui and update import
* refactor: silence loadCustomConfig in status endpoint
* feat: Add optional pluginKey parameter to getUserPluginAuthValue
* feat: Add MCP authentication values endpoint and related queries
- Implemented a new endpoint to check authentication value flags for specific MCP servers, returning boolean indicators for each custom user variable.
- Added a corresponding query hook `useMCPAuthValuesQuery` to fetch authentication values from the frontend.
- Defined the API endpoint for retrieving MCP authentication values in api-endpoints.ts.
- Updated data-service to include a method for fetching MCP authentication values.
- Introduced new types for MCP authentication values responses in the types module.
- Added a new query key for MCP authentication values.
* feat: Localize MCPSelect component status labels and aria attributes
- Updated the MCPSelect component to use localized strings for connection status labels and aria attributes, enhancing accessibility and internationalization support.
- Added new translation keys for various connection states in the translation.json file.
* feat: Implement filtered MCP values selection based on connection status in MCPSelect
- Added a new `filteredSetMCPValues` function to ensure only connected servers are selectable in the MCPSelect component.
- Updated the rendering logic to visually indicate the connection status of servers by adjusting opacity.
- Enhanced accessibility by localizing the aria-label for the configuration button.
* feat: Add CustomUserVarsSection component for managing user variables
- Introduced a new `CustomUserVarsSection` component to allow users to configure custom variables for MCP servers.
- Integrated localization for user interface elements and added new translation keys for variable management.
- Added functionality to save and revoke user variables, with visual indicators for set/unset states.
* feat: Enhance MCPSelect and MCPConfigDialog with improved state management and UI updates
- Integrated `useQueryClient` to refetch queries for tools, authentication values, and connection status upon successful plugin updates in MCPSelect.
- Simplified plugin key handling by directly using the formatted plugin key in save and revoke operations.
- Updated MCPConfigDialog to include server status indicators and improved dialog content structure for better user experience.
- Added new translation key for active status in the localization files.
* feat: Enhance MCPConfigDialog with dynamic server status badges and localization updates
- Added a helper function to render status badges based on the connection state of the MCP server, improving user feedback on connection status.
- Updated the localization files to include new translation keys for connection states such as "Connecting" and "Offline".
- Refactored the dialog to utilize the new status rendering function for better code organization and readability.
* feat: Implement OAuth handling and server initialization in MCP reinitialize flow
- Added OAuth handling to the MCP reinitialize endpoint, allowing the server to capture and return OAuth URLs when required.
- Updated the MCPConfigDialog to include a new ServerInitializationSection for managing server initialization and OAuth flow.
- Enhanced the user experience by providing feedback on server status and OAuth requirements through localized messages.
- Introduced new translation keys for OAuth-related messages in the localization files.
- Refactored the MCPSelect component to remove unused authentication configuration props.
* feat: Make OAuth actually work / update after OAuth link authorized
- Improved the handling of OAuth flows in the MCP reinitialize process, allowing for immediate return when OAuth is initiated.
- Updated the UserController to extract server names from plugin keys for better logging and connection management.
- Enhanced the MCPSelect component to reflect authentication status based on OAuth requirements.
- Implemented polling for OAuth completion in the ServerInitializationSection to improve user feedback during the connection process.
- Refactored MCPManager to support new OAuth flow initiation logic and connection handling.
* refactor: Simplify MCPPanel component and enhance server status display
- Removed unused imports and state management related to user plugins and server reinitialization.
- Integrated connection status handling directly into the MCPPanel for improved user feedback.
- Updated the rendering logic to display server connection states with visual indicators.
- Refactored the editing view to utilize new components for server initialization and custom user variables management.
* chore: remove comments
* chore: remove unused translation key for MCP panel
* refactor: Rename returnOnOAuthInitiated to returnOnOAuth for clarity
* refactor: attempt initialize on server click
* feat: add cancel OAuth flow functionality and related UI updates
* refactor: move server status icon logic into its own component
* chore: remove old localization strings (makes more sense for icon labels to just use configure stirng since thats where it leads to)
* fix: fix accessibility issues with MCPSelect
* fix: add missing save/revoke mutation logic to MCPPanel
* styling: add margin to checkmark in MultiSelect
* fix: add back in customUserVars check to hide gear config icon for servers without customUserVars
---------
Co-authored-by: Dustin Healy <dustinhealy1@gmail.com>
Co-authored-by: Dustin Healy <54083382+dustinhealy@users.noreply.github.com>
|
||
|
|
d0c958ba33
|
🔥 feat: Add Firecrawl Scraper Configurability (#8495)
- Added firecrawlOptions configuration field to librechat.yaml - Refactored web.ts to live in packages/api rather than data-provider - Updated imports from web.ts to reflect new location - Added firecrawlOptions to FirecrawlConfig interface - Added firecrawlOptions to authResult of loadWebSearchAuth so it gets properly passed to agents to be built into firecrawl payload - Added tests for firecrawlOptions to web.spec.ts |
||
|
|
3e4b01de82
|
🗝️ feat: User Provided Credentials for MCP Servers (#7980)
* 🗝️ feat: Per-User Credentials for MCP Servers
chore: add aider to gitignore
feat: fill custom variables to MCP server
feat: replace placeholders with custom user MCP variables
feat: handle MCP install/uninstall (uses pluginauths)
feat: add MCP custom variables dialog to MCPSelect
feat: add MCP custom variables dialog to the side panel
feat: do not require to fill MCP credentials for in tools dialog
feat: add translations keys (en+cs) for custom MCP variables
fix: handle LIBRECHAT_USER_ID correctly during MCP var replacement
style: remove unused MCP translation keys
style: fix eslint for MCP custom vars
chore: move aider gitignore to AI section
* feat: Add Plugin Authentication Methods to data-schemas
* refactor: Replace PluginAuth model methods with new utility functions for improved code organization and maintainability
* refactor: Move IPluginAuth interface to types directory for better organization and update pluginAuth schema to use the new import
* refactor: Remove unused getUsersPluginsAuthValuesMap function and streamline PluginService.js; add new getPluginAuthMap function for improved plugin authentication handling
* chore: fix typing for optional tools property with GenericTool[] type
* chore: update librechat-data-provider version to 0.7.88
* refactor: optimize getUserMCPAuthMap function by reducing variable usage and improving server key collection logic
* refactor: streamline MCP tool creation by removing customUserVars parameter and enhancing user-specific authentication handling to avoid closure encapsulation
* refactor: extract processSingleValue function to streamline MCP environment variable processing and enhance readability
* refactor: enhance MCP tool processing logic by simplifying conditions and improving authentication handling for custom user variables
* ci: fix action tests
* chore: fix imports, remove comments
* chore: remove non-english translations
* fix: remove newline at end of translation.json file
---------
Co-authored-by: Aleš Kůtek <kutekales@gmail.com>
|
||
|
|
3af2666890
|
🪐 refactor: Migrate Share Functionality to Type-Safe Methods (#7903)
* chore: Update import for isEnabled utility in convoAccess middleware * refactor: Migrate Share functionality to new methods structure in `@librechat/data-schemas` - Deleted the old Share.js model and moved its functionality to a new share.ts file within the data-schemas package. - Updated imports across the codebase to reflect the new structure. - Enhanced error handling and logging in shared link operations. - Introduced TypeScript types for shared links and related operations to improve type safety and maintainability. * chore: Update promptGroupSchema validation with typing * fix: error handling and logging in createSharedLink * fix: don't allow empty shared link or shared link without messages * ci: add tests for shared link methods * chore: Bump version of @librechat/data-schemas to 0.0.9 in package.json and package-lock.json * chore: Add nanoid as peer dependency - Introduced `nanoid` as a dependency in `package.json` and `package-lock.json`. - Replaced UUID generation with `nanoid` for creating unique conversation and message IDs in share methods tests. |
||
|
|
be4cf5846c
|
📧 feat: Mailgun API Email Configuration (#7742)
* fix: add undefined password check in local user authentication * fix: edge case - issue deleting user when no conversations in deleteUserController * feat: Integrate Mailgun API for email sending functionality * fix: undefined SESSION_EXPIRY handling and add tests * fix: update import path for isEnabled utility in azureUtils.js to resolve circular dep. |
||
|
|
a2fc7d312a
|
🏗️ refactor: Extract DB layers to data-schemas for shared use (#7650)
* refactor: move model definitions and database-related methods to packages/data-schemas * ci: update tests due to new DB structure fix: disable mocking `librechat-data-provider` feat: Add schema exports to data-schemas package - Introduced a new schema module that exports various schemas including action, agent, and user schemas. - Updated index.ts to include the new schema exports for better modularity and organization. ci: fix appleStrategy tests fix: Agent.spec.js ci: refactor handleTools tests to use MongoMemoryServer for in-memory database fix: getLogStores imports ci: update banViolation tests to use MongoMemoryServer and improve session mocking test: refactor samlStrategy tests to improve mock configurations and user handling ci: fix crypto mock in handleText tests for improved accuracy ci: refactor spendTokens tests to improve model imports and setup ci: refactor Message model tests to use MongoMemoryServer and improve database interactions * refactor: streamline IMessage interface and move feedback properties to types/message.ts * refactor: use exported initializeRoles from `data-schemas`, remove api workspace version (this serves as an example of future migrations that still need to happen) * refactor: update model imports to use destructuring from `~/db/models` for consistency and clarity * refactor: remove unused mongoose imports from model files for cleaner code * refactor: remove unused mongoose imports from Share, Prompt, and Transaction model files for cleaner code * refactor: remove unused import in Transaction model for cleaner code * ci: update deploy workflow to reference new Docker Dev Branch Images Build and add new workflow for building Docker images on dev branch * chore: cleanup imports |
||
|
|
0dbbf7de04
|
🔎 feat: Native Web Search with Citation References (#7516)
* WIP: search tool integration * WIP: Add web search capabilities and API key management to agent actions * WIP: web search capability to agent configuration and selection * WIP: Add web search capability to backend agent configuration * WIP: add web search option to default agent form values * WIP: add attachments for web search * feat: add plugin for processing web search citations * WIP: first pass, Citation UI * chore: remove console.log * feat: Add AnimatedTabs component for tabbed UI functionality * refactor: AnimatedTabs component with CSS animations and stable ID generation * WIP example content * feat: SearchContext for managing search results apart from MessageContext * feat: Enhance AnimatedTabs with underline animation and state management * WIP: first pass, Implement dynamic tab functionality in Sources component with search results integration * fix: Update class names for improved styling in Sources and AnimatedTabs components * feat: Improve styling and layout in Sources component with enhanced button and item designs * feat: Refactor Sources component to integrate OGDialog for source display and improve layout * style: Update background color in SourceItem and SourcesGroup components for improved visibility * refactor: Sources component to enhance SourceItem structure and improve favicon handling * style: Adjust font size of domain text in SourceItem for better readability * feat: Add localization for citation source and details in CompositeCitation component * style: add theming to Citation components * feat: Enhance SourceItem component with dialog support and improved hovercard functionality * feat: Add localization for sources tab and image alt text in Sources component * style: Replace divs with spans for better semantic structure in CompositeCitation and Citation components * refactor: Sources component to use useMemo for tab generation and improve performance * chore: bump @librechat/agents to v2.4.318 * chore: update search result types * fix: search results retrieval in ContentParts component, re-render attachments when expected * feat: update sources style/types to use latest search result structure * style: enhance Dialog (expanded) SourceItem component with link wrapping and improved styling * style: update ImageItem component styling for improved title visibility * refactor: remove SourceItemBase component and adjust SourceItem layout for improved styling * chore: linting twcss order * fix: prevent FileAttachment from rendering search attachments * fix: append underscore to responseMessageId for unique identification to prevent mapping of previous latest message's attachments * chore: remove unused parameter 'useSpecs' from loadTools function * chore: twcss order * WIP: WebSearch Tool UI * refactor: add limit parameter to StackedFavicons for customizable source display * refactor: optimize search results memoization by making more granular and separate conerns * refactor: integrated StackedFavicons to WebSearch mid-run * chore: bump @librechat/agents to expose handleToolCallChunks * chore: use typedefs from dedicated file instead of defining them in AgentClient module * WIP: first pass, search progress results * refactor: move createOnSearchResults function to a dedicated search module * chore: bump @librechat/agents to v2.4.320 * WIP: first pass, search results processed UX * refactor: consolidate context variables in createOnSearchResults function * chore: bump @librechat/agents to v2.4.321 * feat: add guidelines for web search tool response formatting in loadTools function * feat: add isLast prop to Part component and update WebSearch logic for improved state handling * style: update Hovercard styles for improved UI consistency * feat: export FaviconImage component for improved accessibility in other modules * refactor: export getCleanDomain function and use FaviconImage in Citation component for improved source representation * refactor: implement SourceHovercard component for consistency and DRY compliance * fix: replace <p> with <span> for snippet and title in SourceItem and SourceHovercard for consistency * style: `not-prose` * style: remove 'not-prose' class for consistency in SourceItem, Citation, and SourceHovercard components, adjust style classes * refactor: `imageUrl` on hover and prevent duplicate sources * refactor: enhance SourcesGroup dialog layout and improve source item presentation * refactor: reorganize Web Components, save in same directory * feat: add 'news' refType to refTypeMap for citation sources * style: adjust Hovercard width for improved layout * refactor: update tool usage guidelines for improved clarity and execution * chore: linting * feat: add Web Search badge with initial permissions and local storage logic * feat: add webSearch support to interface and permissions schemas * feat: implement Web Search API key management and localization updates * feat: refactor Web Search API key handling and integrate new search API key form * fix: remove unnecessary visibility state from FileAttachment component * feat: update WebSearch component to use Globe icon and localized search label * feat: enhance ApiKeyDialog with dropdown for reranker selection and update translations * feat: implement dropdown menus for engine, scraper, and reranker selection in ApiKeyDialog * chore: linting and add unknown instead of `any` type * feat: refactor ApiKeyDialog and useAuthSearchTool for improved API key management * refactor: update ocrSchema to use template literals for default apiKey and baseURL * feat: add web search configuration and utility functions for environment variable extraction * fix: ensure filepath is defined before checking its prefix in useAttachmentHandler * feat: enhance web search functionality with improved configuration and environment variable extraction for authFields * fix: update auth type in TPluginAction and TUpdateUserPlugins to use Partial<Record<string, string>> * feat: implement web search authentication verification and enhance webSearchAuth structure * feat: enhance ephemeral agent handling with new web search capability and type definition * feat: enhance isEphemeralAgent function to include web search selection * feat: refactor verifyWebSearchAuth to improve key handling and authentication checks * feat: implement loadWebSearchAuth function for improved web search authentication handling * feat: enhance web search authentication with new configuration options and refactor related types * refactor: rename search engine to search provider and update related localization keys * feat: update verifyWebSearchAuth to handle multiple authentication types and improve error handling * feat: update ApiKeyDialog to accept authTypes prop and remove isUserProvided check * feat: add tests for extractWebSearchEnvVars and loadWebSearchAuth functions * feat: enhance loadWebSearchAuth to support specific service checks for providers, scrapers, and rerankers * fix: update web search configuration key and adjust auth result handling in loadTools function * feat: add new progress key for repeated web searching and update localization * chore: bump @librechat/agents to 2.4.322 * feat: enhance loadTools function to include ISO time and improve search tool logging * feat: update StackedFavicons to handle negative start index and improve citation attribution styling and text * chore: update .gitignore to categorize AI-related files * fix: mobile responsiveness of sources/citations hovercards * feat: enhance source display with improved line clamping for better readability * chore: bump @librechat/agents to v2.4.33 * feat: add handling for image sources in references mapping * chore: bump librechat-data-provider version to 0.7.84 * chore: bump @librechat/agents version to 2.4.34 * fix: update auth handling to support multiple auth types in tools and allow key configuration in agent panel * chore: remove redundant agent attribution text from search form * fix: web search auth uninstall * refactor: convert CheckboxButton to a forwardRef component and update setValue callback signature * feat: add triggerRef prop to ApiKeyDialog components for improved dialog control * feat: integrate triggerRef in CodeInterpreter and WebSearch components for enhanced dialog management * feat: enhance ApiKeyDialog with additional links for Firecrawl and Jina API key guidance * feat: implement web search configuration handling in ApiKeyDialog and add tests for dropdown visibility * fix: update webSearchConfig reference in config route for correct payload assignment * feat: update ApiKeyDialog to conditionally render sections based on authTypes and modify loadWebSearchAuth to correctly categorize authentication types * feat: refactor ApiKeyDialog and related tests to use SearchCategories and RerankerTypes enums and remove nested ternaries * refactor: move ThinkingButton rendering to improve layout consistency in ContentParts * feat: integrate search context into Markdown component to conditionally include unicodeCitation plugin * chore: bump @librechat/agents to v2.4.35 * chore: remove unused 18n key * ci: add WEB_SEARCH permission testing and update AppService tests for new webSearch configuration * ci: add more comprehensive tests for loadWebSearchAuth to validate authentication handling and authTypes structure * chore: remove debugging console log from web.spec.ts to clean up test output |