mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-11-01 19:46:50 +00:00
180 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ravi Kumar L
|
000349681e
|
🩻 feat(insights): Scope Insights to Authorized Agents (#15549)
* feat(insights): add agent-scoped access * fix(insights): exclude unattributed assistant messages * fix(sharing): key role insight toggles by principal * fix(sharing): preserve insight grant snapshot * fix(permissions): preserve insight bits atomically * fix(permissions): guard insight audit rollback * fix(insights): keep admin grant controls accessible * fix(permissions): reconcile duplicate principal updates * fix(insights): show automatic admin access * fix(permissions): harden insights access updates * fix(insights): simplify authorization and preserve client state |
||
Danny Avila
|
57c52de33c
|
🛎️ fix: Start Code Environment Reconciliation After Mongo Connects (#15620) | ||
|
Danny Avila
|
d346e5ab5e
|
🚰 fix: Record the Provider Drain When Shutdown Cuts a Generation Short (#15532)
* 🚰 fix: Record the Provider Drain When Shutdown Cuts a Generation Short
* fix: Await generation settlement on shutdown instead of publishing the drain
* fix: Spend the real shutdown budget and cover the initialization window
* fix: Keep the shutdown registration literal the index spec asserts on
* fix: Keep draining settlements registered while shutdown is waiting
* fix: Track provider executions at the begin/drain chokepoints instead of per caller
* fix: Open the provider tracker before the begin CAS and cap cluster workers to the primary deadline
* fix: Propagate the cluster deadline and release trackers on reconfigure and abandonment
* fix: Hold SIGTERM until the worker acknowledges the cluster deadline
* fix: Refuse provider starts after shutdown begins and harden the cluster deadline handoff
* test: Assert the multi-execution shutdown wait with both executions begun before shutdown
|
||
|
Danny Avila
|
fcae1025c0
|
🧳 feat: Register Principal-Owned Code Environments (#15365)
* feat: add principal-owned code environments * fix: address code environment CI coverage * fix: expose principal code environments in endpoint config * fix: reject missing code environment bodies * fix: harden code environment lifecycle * fix: revalidate principal code environments * fix: synchronize code environment authorization * fix: bind code environments to current principals * fix: fail closed on code ACL cache errors * fix: prevent code environment override shadowing * fix: preserve principal environment defaults * fix: suppress revoked code environment aliases * fix: fail closed on environment augmentation * fix: narrow code environment defaults * fix: isolate code environment fallback * style: sort code config imports |
||
|
Danny Avila
|
a9ccac8656
|
🧲 feat: Enable Secure Attached Environment Pairing (#15355)
* feat: add secure code environment pairing * fix: satisfy code environment type checks * fix: secure code environment administration * fix: isolate code pairing control plane * fix: validate code pairing control responses * fix: secure code pairing transport * fix: validate code pairing wire format * fix: harden pairing secret lookup |
||
|
Danny Avila
|
487984193a
|
🪂 feat: Make Event Actor Detached Actions Durable (#15307)
Some checks are pending
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
* feat: make Event Actor detached actions durable * fix: align detached actor checks with merged base * fix: close detached actor lifecycle gaps * fix: break detached action outcome cycle * fix: deepen detached action ownership * fix: fence detached launch handoffs * fix: preserve detached rollout fencing * test: type detached recovery failure * fix: close detached ownership handoffs * fix: stage detached action activation * fix: persist detached terminal retries * fix: fence detached durability ownership * fix: fence trigger lane publication * fix: fence detached transition generations * fix: retire failed detached predecessors * fix: close detached rollout consumers * test: type detached retry turns * fix: close detached ownership gaps * fix: fence detached action resumes * fix: fence prior-head event actor resumes * fix: preserve legacy event actor resumes * fix: align detached resume boundaries |
||
|
Danny Avila
|
8ba76507ae
|
🧷 fix: Verify Scheduled HITL State Before Pausing (#15284)
Some checks failed
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
Publish `librechat-data-provider` to NPM / pack (push) Has been cancelled
Publish `librechat-data-provider` to NPM / publish-npm (push) Has been cancelled
* fix: require durable storage for scheduled HITL * fix: verify scheduled HITL checkpoints before pause * fix: narrow scheduled HITL capability checks * fix: honor ask tool filters in schedule preflight * fix: enforce durability across scheduled HITL resumes * fix: scope scheduled HITL admission to active tools * refactor: move scheduled HITL admission to TypeScript * fix: close scheduled HITL admission gaps * fix: bind scheduled pauses to current checkpoint * fix: cap pending actions at binding deadline * fix: reject expired pending actions * fix: guard approval deadlines atomically |
||
|
Danny Avila
|
de59da9636
|
🎟️ refactor: Require Credentials for Local Image Access by Default (#15252)
* 🔐 fix: Protect Local Image Access by Default * 🔐 fix: Scope Image Authorization to Active Sessions * 🧹 style: Format Image Authorization Checks * 🛡️ fix: Harden Image Avatar Authorization * 🧭 style: Sort Image Authorization Imports * 🔐 fix: Close Image Authorization Review Gaps * 🧭 fix: Normalize Stored Avatar Base Paths * 🏢 fix: Resolve Tenant Assistant Image Policy * 🛂 fix: Enforce Effective Image Access Policy * 🧹 style: Flatten Assistant Config Selection * 🧷 fix: Preserve Image Access Compatibility * 🪪 fix: Make Image Sessions Revocable * 🏗️ fix: Move Image Session Policy Into API |
||
|
Danny Avila
|
44edcbe014
|
🧾 feat: Store Durable Event Actor Receipts (#15265)
* 🧾 feat: Store durable event actor receipts * 📊 fix: Scope actor delivery metrics * 🧱 fix: Close durable receipt recovery gaps * fix: serialize actor action admission * fix: retire terminal batch members * fix: close actor terminal recovery races * fix: reclaim lanes after terminal receipts * fix: close actor receipt recovery gaps * fix: preserve ambiguous legacy handling * fix: recover legacy actor outcomes * fix: admit leased actor actions * fix: serialize actor admission recovery * fix: close actor recovery races * fix: token-fence actor admissions * fix: preserve mixed-version actor fences * fix: harden actor rollout and metrics * fix: gate durable actor receipt rollout * fix: enforce base actor receipt rollout * fix: align experimental actor rollout |
||
|
Danny Avila
|
f34a49007d
|
🔣 fix: Escape SPA Language Attribute (#15248) | ||
|
Danny Avila
|
290b8664d9
|
🧾 feat: Track Authoritative Agent Event Outcomes (#15213)
* feat: track authoritative agent event outcomes * fix: isolate agent event outcome types * fix: declare agent event handler result * fix: simplify agent event status selection * fix: preserve authoritative event outcomes * fix: preserve terminal event evidence * test: use completed run-step envelope * test: scope deferred HITL question locator * fix: settle every agent event terminal path * style: sort terminal host action imports * fix: fence agent event terminal evidence * fix: recover agent event terminal settlement * fix: scope terminal retry hints by generation * fix: settle terminal host actions exactly |
||
|
Danny Avila
|
877b9b2f1a
|
🐚 feat: Nonce-Based Content Security Policy for the SPA Shell (#14446)
* 🛡️ feat: Configurable Baseline HTTP Security Headers Adds helmet's CSP-independent headers (HSTS, X-Frame-Options, X-Content-Type-Options, COOP, CORP, Referrer-Policy) on every response, with contentSecurityPolicy explicitly disabled. Every header that can break a deployment is configurable, so there is no allow-list to go stale the way #7377's hardcoded CSP directives did. HSTS includeSubDomains defaults off rather than matching helmet's on-by-default: it would otherwise pin every sibling subdomain to HTTPS for a year in every visitor's browser, and undoing that requires serving max-age=0 from each affected host. * 🛡️ feat: Nonce-Based Content Security Policy for the SPA Shell Adds an opt-in, per-response nonce CSP on the HTML response, resolved once at startup so each request only mints a nonce and concatenates the header. Report-only by default, since that is the rollout step #7377 skipped. Rebase and correctness pass over #13226: - Styles carry no nonce. A nonce in style-src makes browsers ignore 'unsafe-inline', which would have blocked the <style> element the theme script injects at runtime, plus every style third-party components inject. - frame-ancestors 'self' is now a default rather than opt-in, so enabling CSP actually covers the clickjacking half of #7110. - CSP_SCRIPT_SRC_EXTRA now drops 'strict-dynamic', which would otherwise make browsers ignore the very hosts the operator configured. - Nonce stamping runs after the query-devtools bootstrap injection so that injected script is covered too. * fix: replace frame-ancestors instead of merging it Merging the configured value into the default turned a deliberate CSP_FRAME_ANCESTORS='none' into `frame-ancestors 'self' 'none'`, which browsers resolve back to 'self'. Also bail out if the serialized policy somehow lacks the nonce slot rather than emitting a header the shell cannot match. * fix: address Codex review findings on the CSP defaults All five were real against LibreChat's actual runtime: - CSP_REPORT_ONLY now only enforces on an explicit false/off/0/no. A typo or `1` previously fell through isEnabled() to enforcing, turning a config slip into a blocked SPA. Shares the parse helper with headers.ts via a new security/env.ts. - Module preloads are stamped. A production client/dist/index.html carries 32 parser-inserted `<link rel="modulepreload">` tags, which 'strict-dynamic' does not cover and 'self' cannot rescue. - Stale nonce attributes are replaced rather than preserved; only the current response's nonce is authorized. - worker-src allows data:, which Monaco's default CDN loader needs to bootstrap its workers (there is no loader.config() in the client). - script-src allows 'wasm-unsafe-eval' for the HEIC upload path, which compiles WebAssembly through heic-to. Narrower than 'unsafe-eval'. Verified against the real built shell: 4 scripts and all 32 preloads nonced, stylesheets/icons/manifest and <style> untouched. * fix: address second Codex round on CSP rollout controls - SECURITY_HEADERS=false now disables CSP too. It is documented as the global kill switch, and an operator reaching for it to recover a shell broken by an enforcing policy must not be left with that policy on. - The SPA shell is forced to `no-store` while CSP is enabled, ignoring INDEX_CACHE_CONTROL/INDEX_PRAGMA/INDEX_EXPIRES and warning when they are set. A cacheable shell pins one nonce across page loads and users, which is the whole thing a nonce policy defends against. - Added CSP_ALLOW_WASM and CSP_ALLOW_DATA_WORKERS. The previous commit's .env.example claimed CSP_ADDITIONAL_DIRECTIVES could drop 'wasm-unsafe-eval' and data:, but merging only ever appends sources, so the documented hardening step was impossible. These toggles make it real. |
||
|
Danny Avila
|
2ef12b1e1d
|
🦺 feat: Configurable Baseline HTTP Security Headers (#14445)
Adds helmet's CSP-independent headers (HSTS, X-Frame-Options, X-Content-Type-Options, COOP, CORP, Referrer-Policy) on every response, with contentSecurityPolicy explicitly disabled. Every header that can break a deployment is configurable, so there is no allow-list to go stale the way #7377's hardcoded CSP directives did. HSTS includeSubDomains defaults off rather than matching helmet's on-by-default: it would otherwise pin every sibling subdomain to HTTPS for a year in every visitor's browser, and undoing that requires serving max-age=0 from each affected host. |
||
|
Danny Avila
|
8969ee4b18
|
🎚️ feat: Configure Agent Event Runtime in YAML (#15128) | ||
|
Danny Avila
|
c7e355b219
|
🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup (#15051)
* 🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup Fixes #15042, fixes #15043. `resume.js` inferred a confirmed stop from the ABSENCE of `failureReason`, but `abortJob` had four `success: false` paths that returned no reason at all. Those settled the occurrence as `interrupted` and pruned the checkpoint on aborts that never landed — including one where a REPLACEMENT generation owned the conversation, which pruned the successor's checkpoint. Every `success: false` return now names itself (`job_not_found`, `already_settled` added alongside the existing `generation_replaced` / `job_still_active`), and a single canonical `isStopConfirmed` predicate decides whether durable state may be settled. `already_settled` confirms a stop — `awaitProviderDrain` has proven the provider segment can no longer persist — so a permanently terminal generation is not answered with a retry loop. Separately, a schedule engine that failed to arm advertised its permanent outage as a transient 503 with `Retry-After`, so a client obeying it would poll forever. Readiness is now tri-state (`starting` / `armed` / `unavailable`): the retry contract applies only while arming is genuinely pending, and a failed arm returns a terminal `SCHEDULES_UNAVAILABLE` with no `Retry-After` and an error-level log. * 🏷️ fix: Declare Schedule Write Gate Return Types `--isolatedDeclarations` requires an explicit return type on the exported factory and on the middleware it returns (TS9007). Adds a named `ScheduleWriteGate` type matching the existing `ShareMiddleware` shape. |
||
|
Danny Avila
|
c5276fc63d
|
⏱️ feat: Run Scheduled Chats Through Durable Agent Triggers (#14939)
* feat: Scheduled Chats — agent-centric scheduled runs creating real conversations
Squash of the full review-hardened branch (PR 14540, supersedes 14373) onto
latest dev, preserving the exact verified tree. History prior to this commit
lived on the pre-squash branch; every invariant below survived 25 Codex review
rounds plus two external audit rounds (R26) with regression tests that fail
without their fixes.
Feature:
- Schedules CRUD + side-panel UI (cadence dialog, run cards, Run Now), roles/
permissions (SCHEDULES:USE), interface.schedules availability, per-user limits
and capacity slots, timezone-aware cadence with DST-conservative floors and
misfire grace.
- Engine: single-process claim/fire loop with leases, loopback POST dispatch
(signed schedule-fire JWT claims, per-occurrence idempotency key inside the
route's clientRequestId charset), overlap/balance/capacity/duplicate skip
policies with auto-disable streaks (too_many_failures, insufficient_balance),
reconciliation from retained terminal-job evidence, erasure sweep.
- Scheduled runs create real conversations through the resumable agents chat
path: HITL pauses surface on the card (requires_action), resumes re-apply the
fire boundary's admission policy (revision fence, enabled, global kill switch,
SCHEDULES:USE, availability) before continuing a billed generation.
Correctness invariants (the audit surface):
- Settlement discipline: every persistence-producing write happens-before a
run's terminal outcome write; Stop/complete/pause race through single-winner
terminal CAS claims (dev's TerminalJobClaim substrate) with retained,
completedAt-less evidence for scheduled fires plus an owner-intended outcome
stamp (scheduleOutcome) the reconciler prefers over re-derived success —
round-tripped through the Redis hash mapper.
- Swallowed generation failures (client error content parts) classify to
error/skipped_balance instead of success on both initial and resumed paths;
stale stamps are refreshed evidence-first when persistence plus the Mongo
outcome write both fail.
- Abort honesty: delivery judged by generation ownership and the CAS's actual
from-status; republication escalates to the transport's acknowledged variant;
the Stop route settles only genuinely paused runs.
- Account deletion: one-way barrier (deletionRequestedAt) with auth-cache
tombstone-before-stamp, boundary rechecks across all auth strategies, quiesce
of scheduled + interactive work with durable per-stream abort fences
(positive-evidence acknowledgement only), owner-side finalization markers for
post-terminal billed writes, deferred-deletion sweep (explicitly ensured
partial index) that completes cascades autonomously. Remote OpenAI-compatible/
Responses requests are documented as outside the quiesce and tracked in issue
14594.
- Store compatibility: finalization markers optional on the legacy IJobStore
contract with coherent degradation (registration fails -> synchronous title
fallback; count reads 0).
* fix: fence terminal response persistence from deletion; retain stale-pause evidence
Two blockers from the third external review round.
Terminal persistence visible to account deletion:
- The finalization marker was registered only when post-terminal TITLE work was
possible, but every persistence-owning terminal CAS opens the same window: the
claim drops the job out of the active set BEFORE the response save (and the
background user-message/convo saves), so a deletion quiesce landing there saw
neither an active job nor a marker and could cascade while the admitted request
could still recreate messages. Both controllers now register the marker before
every persistence-owning claim — the fresh-turn path and the HITL resume — and
release it only after their pending saves (and any post-terminal title) have
landed, on success and failure paths alike. The TTL bounds a crash.
- settleAbortFence no longer clears a complete/error fence while
`terminalPersistencePending` is set: terminal at the CAS is not settled while
the owner is still persisting. The job facade now surfaces the flag.
- The marker trio is REQUIRED by the runtime store contract (assertJobStoreV2
refuses a store without it at configure time, keeping the failure loud and
deterministic) while remaining optional on the legacy public IJobStore type for
source compatibility. The silent degrade path from the previous round is gone —
it was not deletion-safe.
Stale-pause recovery retains scheduled evidence:
- Three crash/timeout recovery paths — ApprovalLifecycle.failStalePausePersistence
and the InMemory/Redis stale-pause cleanups — unconditionally stamped
`completedAt`, putting a scheduled fire's failed-pause error terminal on the
short completed TTL. A Mongo outage longer than that TTL erased the evidence and
the reconciler recovered the run as `interrupted` instead of `error`. All three
now follow the controller-observed path from the previous round: scheduled jobs
omit `completedAt` (retained-evidence TTL) and stamp the error outcome.
The PR description now explicitly narrows the deletion guarantee for the remote
OpenAI-compatible/Responses paths (tracked in issue 14594).
* fix: deletion-fence marker protocol — generation-scoped, atomic, fail-closed, all terminal paths
One consolidated pass over the finalization-marker mechanism, per the fourth
external review round. The invariant it establishes: NO persistence-owning
terminal CAS runs without a durable, generation-qualified marker covering the
window it opens, and every consumer treats a pending terminal as unsettled.
- Generation-scoped markers. Entries were keyed (userId, streamId), so a
Stop-superseded generation finishing late could clear the marker its
replacement registered on the same conversation. Marker fields are now
qualified by the generation's createdAt; clears must present the same
identity, and an unqualified legacy clear cannot drop a qualified entry.
- Atomic Redis registration. HSET-then-EXPIRE loses the fresh marker when the
user's existing hash expires between the two commands (or the process dies
there); registration is now a single Lua script carrying both.
- Fail closed everywhere. Registration failure (after one retry) now REFUSES
the terminal CAS instead of proceeding uncovered: the completion claim throws
into the error path, the error path skips completeJob and leaves the job
ACTIVE — deletion-visible by itself, recovered by the stale-running reaper —
and abortJob returns a new retryable `fence_unavailable` failure the Stop
route answers with 503 and the deletion quiesce treats as an unacknowledged
stop (fence kept). The previous round's log-and-proceed is gone.
- Every terminal path enrolled. abortJob now owns its window (register before
the abort CAS, clear in its finally — the Stop route's checkpoint prune and
partial save run inside beforePublish, between CAS and publication); the
interactive and background generation-error paths register before their
completeJob; the resume controller's error finalization registers before its
completeJob. A lost or thrown claim releases the marker after pending saves
flush instead of holding the user's deletion behind the TTL.
- Every pending terminal unsettled. settleAbortFence defers on
terminalPersistencePending for ALL statuses — including `aborted`, whose
route-side persistence the previous guard missed.
Also: a direct Redis regression for scheduled stale-pause retention (the P2
test gap), and the PR description no longer claims to carry every commit.
* fix: lease-token lifecycle fences — same-generation isolation, admission fence, undelivered-Stop retention, legacy abort enrollment
Fifth external review round; four P1s, handled as the requested consolidated
lifecycle-fence pass.
- Lease tokens. Marker fields were (streamId, createdAt), shared by every
contender on the same generation — completion and Stop, or two racing Stops —
so a losing contender's cleanup erased the winner's still-live marker. Every
registrant now carries a unique lease token in the field and may only ever
clear its own lease; unqualified legacy clears cannot touch qualified entries.
- Admission fence. Authentication can pass before the deletion barrier goes up,
and the durable createJob is several async steps later — a deletion quiesce in
that window saw neither an active job nor a marker and could cascade before
the admitted request created its job. The controller now registers an
admission lease and THEN rereads the deletion barrier: the ordering guarantees
either this request observes the barrier (403, lease released, slot/claim
cleanup) or the quiesce observes the lease and defers. Held until createJob is
durable; released on every refusal and initialization-error path. Fail closed
when the lease itself cannot be registered (503 retryable).
- Undelivered-Stop retention. abortJob released its lease in a finally even when
delivery AND publication had provably failed — the job reads terminal
(invisible to active-set scans), a user Stop writes no durable abort fence,
and the remote owner keeps generating and will persist its abort-catch writes
whenever the signal finally lands. The lease is now retained in exactly that
case, and each resignal attempt heartbeats a fresh lease so the fence outlives
the TTL for as long as delivery is still being driven. The abort-winning
turn's own loser-side pending saves are additionally fenced in the controller
catch (best-effort — those writes are already in flight).
- Legacy abort enrollment. abortMiddleware (assistants abort route fallback for
non-assistants endpoints) awaited abortJob and then spent usage and saved the
stopped response AFTER the abort's lease was released. Both writes now run
inside `beforePublish`, between the abort CAS and publication, covered by the
same lease as every other abort.
Barrier tests, each verified to fail without its fix: same-generation lease
isolation (store), racing two-Stop loser cleanup (manager, stale-read forced
CAS race), undelivered-Stop lease retention, resignal heartbeat, admission
refusal with lease-before-reread ordering plus release-on-durable-create, and
legacy-abort persistence inside beforePublish.
* fix: heartbeat-backed owner-lifecycle leases close the settlement handoff races
Sixth external review round: the remaining P1 interleavings were one structural
problem — lease handoffs that were not atomic — resolved as the requested
consolidated lifecycle-lease pass.
- Quiesce reads leases BEFORE the active-job scan. The admission-lease -> durable
-job handoff is only atomic against a reader in the OPPOSITE order of the
writer: writers hold the lease strictly until the job is active-set visible,
so leases-first shows every interleaving either the lease or the job.
Jobs-first allowed a request to create its job after the scan and release its
lease before the count — hiding both, cascading, and letting the new
generation persist into a deleted account.
- The abort acknowledgement is fenced by the owner-lifecycle lease. Redis ACKed
the moment the owner's AbortController tripped; the stopping side released its
lease on that ACK while the owner's asynchronous abort-catch persistence was
still ahead. The transport now awaits a manager-installed pre-ACK hook that
registers a DETERMINISTIC owner lease (exactly one owner exists per
generation, and determinism is what lets the signal-time registrant and the
owner's catch-side release agree across processes) before the acknowledgement
is persisted or published; an owned same-replica abort bridges to the same
lease before tripping its local controller. Both generation-owner catches
(fresh turn, resume) release it once their writes land.
- A failed replacement handoff no longer orphans the predecessor. The atomic
replacement removes it from active storage, and an unconfirmed handoff
terminalizes the replacement too — leaving nothing a quiesce could discover
while the predecessor's provider may still be generating. Its owner lease is
now retained at the point the receipt fails delivery; the owner replica renews
it through the pre-ACK fence when the signal finally lands.
- Leases HEARTBEAT while held. The five-minute store TTL only bounds a crashed
holder; live persistence — a stalled save, a long deferred title — must never
outlive its own fence. holdUserFinalization registers and renews every minute
until released; the controllers' completion/error/admission leases all hold.
The undelivered-Stop retention moved to a deterministic `stop` lease that
every resignal attempt renews and the first successful one clears (no more
opaque leases accumulating to TTL), and a THROWN abort transition releases the
contender lease instead of leaking it.
- The user-document abort-fence mutations now invalidate the auth user-doc
cache, matching every other user-doc write.
Barrier tests, each verified to fail without its fix: quiesce lease-scan
ordering (plus the observed-lease defer), pre-ACK fence ordering at the
transport, owned-abort owner-lease bridging, replacement-handoff predecessor
retention, held-lease heartbeat past the TTL, and failed-then-successful
resignal reaping the retained stop lease.
* fix: one manager-owned owner-lease span across every abort delivery path
Seventh external review round; four lifecycle-fence gaps, closed by making the
owner-lifecycle lease a single manager-owned, heartbeat-held span.
- Fail-closed acknowledgements. The pre-ACK hook registered a one-shot lease and
the transport ACKed even when it failed; a same-replica owned abort likewise
swallowed registration failure. The hook now acquires a HELD owner lease
(heartbeat until the owner's catch releases it via releaseOwnerLease) and a
rejection SUPPRESSES the acknowledgement — the stopping side stays retryable
behind its retention lease, and every resignal re-drives the handler. The hook
also stopped gating on `job.createdAt === generationId`: during a replacement
handoff the store holds the replacement while the abort targets the
predecessor, and that gate silently skipped exactly the generation being
acknowledged (the store job is owner identity, never a generation gate). A
local owned abort acquires the same held lease before tripping its provider;
post-CAS the trip cannot be withheld, so acquisition failure downgrades
delivery and the retention handoff keeps the user fenced.
- Committed-but-lost-reply disambiguation. A thrown abort transition released
the contender lease as if nothing had happened, but a Lua CAS can commit and
lose its reply — an aborted job invisible to active-set scans whose provider
was never signalled, with no fence left. The throw path now re-reads the exact
generation: only a job still live under the caller's identity proves no
commit; committed or ambiguous outcomes hand the fence to the deterministic
stop lease (kept on the contender lease if even that fails) before rethrowing.
- Replacement handoff covered end to end. A LOCAL replacement abort acquires the
predecessor's held owner lease before the trip (failure reports the receipt
undelivered, engaging retention). Failed-handoff retention is no longer a
swallowed one-shot: it heartbeats with the durable acknowledgement proof as
its renewal predicate — acquisition failures keep retrying for as long as the
fence is needed, and the retainer stands down (without clearing the shared
field) once the remote owner ACKs and thereby holds its own lease.
- A LOCAL resignal delivery hands off to the owner lease BEFORE clearing the
retained stop lease, and keeps the stop lease when that handoff fails.
Barrier tests: hook rejection suppressing the ACK, commit-then-lost-reply
retention with its provably-uncommitted counterpart, local-resignal owner
handoff ordering, pre-ACK owner lease held past the store TTL until release
(and provably stopped after), and failed-handoff retention retrying on its
heartbeat — verified fail-before/pass-after by stashing the fixes.
* fix: finish scheduled chat lifecycle hardening
* fix: close scheduled chat review follow-ups
* fix: generation-fence abort recovery evidence
* test: wait for settled approval tool output
* test: preserve scheduled init reconciliation option
* refactor: rebuild scheduled chats on durable agent triggers
* test: reset MCP cache mock between cases
* test: isolate scheduler startup in server specs
* fix: harden scheduled run lifecycle
* fix: normalize schedule capacity conflicts
* test: type schedule collision fixture
* fix: re-fence scheduled resume and expiry
* fix: fence scheduled resume handoffs
* fix: release superseded manual schedule leases
* fix: release failed run-now claims
* fix: release superseded engine claims
* fix: repair schedule dialog interaction and rework its form
The agent picker was unusable: ControlCombobox portals its popover to the
body by default, which lands it outside the dialog's Radix focus trap. Clicks
passed through it, it could not be tabbed into, and the trap fighting Ariakit
for focus locked the page up on selection. The prop is documented for exactly
this case — pass `portal={false}` and give the dialog `overflow-visible`, as
ProjectButton already does. The time and day dropdowns defaulted the same way.
Alongside that:
- Extract the agent builder's instructions editor (special-variable menu plus
expand-to-fullscreen) into a controlled `VariableEditor` and use it for the
schedule prompt. Insertions now route through `onChange`, so react-hook-form
sees them — the schedule PATCH is built from `dirtyFields`, and a `setValue`
that skipped dirty tracking would have dropped an inserted variable silently.
- Replace the hand-rolled frequency buttons with the shared `Radio`. They marked
the selection with `bg-surface-hover` on an outline button whose hover is the
same token, so the selected option was indistinguishable from a hovered one;
`Radio` is also a real radiogroup rather than four `aria-pressed` toggles.
- Wrap the fields in a real `<form>` and associate the footer button by id, so
Enter submits. Group the frequency, day and time controls in fieldsets.
- Add placeholders for name and prompt, match the textarea fill to the other
fields, and label the hourly case as minutes past the hour.
- Widen the dialog to `md:max-w-3xl` and pair name with agent so the form fits
without scrolling on desktop.
- Move scheduled chats below skills and above prompts in the side nav.
The new dialog spec fails when the portal fix is reverted.
* test: cover scheduled and subagent deletion drains
* refactor: own the form-control appearance in the client primitives
Addresses the codex finding on ScheduleDialog: a feature-local `FIELD_CLASS`
restated the `Input` primitive's border, radius, height and background so it
could be pasted onto the schedule dropdowns, leaving those controls with no
connection to the primitive they were imitating.
Move that appearance into `packages/client/src/components/Field.ts` as the
single source `Input` and `Textarea` now compose, and give `Dropdown` and
`ControlCombobox` a `variant="field"` that applies it. The schedule dialog
passes the variant and carries no class strings of its own.
This also repairs a break the dev merge would otherwise have introduced: the
newer `Dropdown` splits `className` (wrapper) from `triggerClassName`, so the
old pasted classes would have landed on the wrapper and left the triggers
unstyled.
The semantic-token guard now watches the shared module and asserts each
primitive still composes it, which covers more than the two files it read
before.
* fix: keep an explicit schedules disable from becoming an opt-in
`use` is two things at once for a dual-purpose runtime interface field: a
permission bit, which DB overrides strip, and the runtime disable signal that
`getLimits` reads. Stripping it from `{ use: false, maxPerUser: 2 }` leaves an
object, and `getLimits` treats any object without `use: false` as enabled — so
an admin override written to stop scheduled billing for a role or user started
it instead.
Collapse an explicit disable to the boolean form before the strip, on both
paths that accept it: the `interface.schedules` field patch, which admitted the
object wholesale because bare runtime paths deliberately bypass the permission
gate, and the overrides merge, which reached the composite-field branch and
kept `maxPerUser`. Objects that only narrow limits are untouched, so a
principal can still be given a smaller cap.
Both regressions fail without the normalizer.
* fix(schedules): Wave A — null-balance CAS, atomic paused-card clear, clustered erasure sweep
Slice 1 (thread r3804518381): route the existing-null balance initialization
through a { user, tokenCredits: null } compare-and-set instead of a blind $set,
so a concurrent initializer/charge landing between the preflight read and the
write is never handed back its spent starting balance. On a CAS miss the
preflight re-reads the winner. The absent-record $setOnInsert path and the
credited-record refill-config sync are unchanged. Adds the initializeNullBalance
adapter (no upsert) and regression coverage for winner/miss/sync cases.
Slice 2 (thread r3804518388): updateScheduleById now drops a `requires_action`
lastRun projection atomically with the configRevision bump. Any pause present at
edit time was projected under the pre-edit revision and can never be replaced by
its own revision-fenced terminal outcome — a disabling edit would strand the
card on "Needs approval" forever. Implemented as classic-operator CAS branches
(DocumentDB rules out a conditional pipeline $unset), fenced on the card STILL
being the pause so a terminal outcome or newer occurrence that races in is
preserved. Terminal history survives untouched.
Slice 4 (thread r3803826204): expose initializeScheduleErasureSweep from the
schedule runtime facade and start it in every clustered (experimental) worker
after Mongo is up. It re-drives eraseScheduleIfDrained for soft-deleted rows so
a hidden prompt cannot outlive its drain when the delete/erase-on-settle
attempts miss. It arms nothing else and never infers owner death from a
process-local missing job (isTopologySafeToArm gates that).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.1 — reversible account-deletion schedule suspension
Thread r3804518383. Account-deletion quiesce marked every schedule `deleting`,
disabled it, and cleared nextRunAt destructively. When a later cascade step (or
the drain itself) failed, the controller cancelled the user-deletion fence —
restoring the user — but the schedules stayed `deleting` and were erased by the
sweep, silently losing all of a live user's scheduled prompts.
Replace the destructive marking with a REVERSIBLE, token-fenced suspension:
- suspendUserSchedulesForDeletion(userId, token) snapshots each schedule's prior
enabled/nextRunAt under a per-attempt token, then fences firing (disable, clear
nextRunAt, rotate claimToken). It never sets `deleting`, so a suspended row is
not erasure-eligible. Snapshotting reads then bulkWrites (a classic update
cannot copy field values under DocumentDB), fenced per row so an already-
suspended/soft-deleted/edited row is left alone; idempotent per token.
- restoreUserSchedulesFromDeletion(userId, token) reverses it, re-enabling and
re-arming only rows still carrying the exact attempt token and not independently
deleted — so an owner-deleted or newer-attempt-suspended schedule is never
resurrected.
- deleteUserController generates the attempt token, passes it to quiesce, and on
any failure that cancels the user-deletion fence restores the suspended rows. A
successful deletion hard-deletes them (and their snapshots) via the existing
cascade and never restores.
Adds a `deletionSuspension` embedded field (excluded from the wire schedule),
data-method regression tests (suspend/restore/fence/idempotency/no-resurrect),
and controller tests (restore on drain-false and post-quiesce cascade failure,
no restore on success).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.2 — wire the interactive Stop persistence protocol
Thread r3804255932. The durable Stop primitives (requestRunAbort 'stop',
getScheduleRunAbortState, markRunAbortPersisted) already existed but production
only used requestRunAbort(..., 'deletion'). An interactive Stop flipped the job
to `aborted` and then persisted its partial message + checkpoint inside
`beforePublish`, without ever stamping the schedule Stop or acknowledging it —
so reconciliation, the generation owner, or a concurrent schedule/account
deletion could terminalize the run and release its capacity (and erase data)
mid-write.
Wire the request -> persist -> acknowledge -> settle barrier through the
schedule runtime (the route never touches raw Mongo):
- Expose beginScheduledStop / acknowledgeScheduledStopPersistence on the service.
- The abort route stamps the Stop BEFORE signalling abortJob (a serialized
'in_progress' loser returns 409 STOP_IN_PROGRESS without a second abort),
acknowledges only after beforePublish persistence succeeds, and on a
persistence failure leaves the barrier unresolved so the run stays preserved
(client retries; stale-owner timeout is the bounded recovery). A failed abort
releases the stamp it placed so a replacement/retry is never blocked through
its predecessor.
- recordScheduleOutcome (the owner settlement path) now waits, bounded, for the
Stop acknowledgement before terminalizing; a resolved/non-stop/stale marker
proceeds immediately. The paused Stop settles only after its own ack.
Adds service-layer barrier tests and route-level ordering/persistence-failure/
in-progress tests; the data-layer serialization is already covered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave C.1 — reconcile durable trigger delivery with the reservation
Threads r3803826192 (manual limiter) and r3804255924 (PII/moderation), plus the
independently-found long-Retry-After race. fireSchedule reserves a `started` run
and a global capacity slot BEFORE the durable trigger delivery reaches the chat
route, where an interactive limiter (manual Run Now), PII, or moderation can
reject it before any generation job exists — dead-lettering the delivery while
the run sat `started` until the 30-minute orphan sweep mislabeled it interrupted.
And a valid delivery deferred by Retry-After (up to 24h) could be orphan-settled
and have its capacity released, then fire anyway.
Translate durable delivery state into the schedule outcome:
- Store the deterministic trigger deliveryKey on the ScheduleRun reservation
(computed from the envelope BEFORE enqueue, so an ambiguous commit still has it).
- Add a getTriggerDelivery engine dep (wired to the merged trigger service's
getDelivery) that reads the durable delivery by key.
- Schedule reconciliation, for a jobless `started` run: staging/pending/leased →
admission is live, never orphan; dead → record `error` from the durable
lastError and release capacity promptly (no 30-minute wait), through the
ordinary outcome/auto-disable path; succeeded or no record → the existing
legacy orphan policy (interrupted only past the cutoff); a delivery lookup
failure defers rather than orphaning a possibly-live delivery. Limiter/PII/
moderation middleware writes no schedule state.
Adds reconcile state-mapping tests (dead/pending/leased/staging/succeeded/none/
lookup-failure) and a fire test that the reservation's deliveryKey equals the
enqueued delivery's idempotency key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): Wave C.2 — durable retry/ack for terminal host lifecycle actions
Thread r3804518375. Approval expiry won the `requires_action → aborted` CAS and
then invoked the host hook best-effort: `runApprovalExpiredHandler` swallowed a
failure, and because later sweeps enumerate only `requires_action` jobs, the now-
aborted job was never offered again. In the clustered entrypoint (no schedule
reconciler) the ScheduleRun stayed `requires_action` and its retained job
persisted indefinitely.
Make the host lifecycle work durable rather than schedule-specific:
- Add a generic `terminalHostActionPending` marker, set ATOMICALLY in the same
terminal transition (ApprovalLifecycle.expireWithIdentity), only when a host
adapter is installed.
- Retain and index such jobs: both stores keep them out of terminal reaping and
expose getTerminalHostActionJobs(); Redis adds a set + extended (24h-bounded)
TTL, in-memory a bounded 24h retention so a permanently-failing hook cannot leak.
- The manager clears the marker only after the adapter acknowledges success,
fenced by generation identity (clearTerminalHostAction), so a replacement
generation can neither clear nor execute its predecessor's action.
- cleanup()/expireStaleApprovals() enumerates unacknowledged terminal host actions
across restarts and replicas and retries the idempotent hook; the relay only
re-invokes while the marker is unacknowledged, so a successful ack prevents
duplicate work. Store-won expiry marks it too, so a loser-replica relay still
crosses the hook.
- Terminal SSE notification continues regardless of host-hook outcome.
Covers in-memory behavior (retry after failure, restart/other-replica retry, ack
prevents duplicates, identity fence, terminal notification on failure, no marker
accumulation for non-scheduled jobs) and updates the Redis cluster-membership
contract test for the new index.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* style(schedules): satisfy import sorting in fire.ts and fire.spec.ts
CI "Static checks" failed on IMPORT_SORT for the two files Wave C.1 added imports
to (the AgentTriggerEnvelope type import and getAgentTriggerIdempotencyKey).
Applied scripts/sort-imports.mts to exactly those files — imports-only reordering,
no behavior change. Other files reported by a repo-wide check are pre-existing on
dev and deliberately left untouched so this PR is not widened.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): repair the deletion CLI and order restore before the fence release
Addresses three findings from the fresh Codex review.
P1 — config/delete-user.js called methods.disableUserSchedulesForDeletion, which
Wave B removed in favor of suspendUserSchedulesForDeletion. The file is
@ts-nocheck and its spec mocked the removed name, so neither typecheck nor tests
caught it; the real CLI would throw a TypeError before deleting anything and then
only unwind the fence. The CLI now uses the tokenized protocol: it mints a
suspension token, suspends with it, and restores that exact attempt's rows in its
finally block when the deletion does not commit. Its spec mocks the real methods,
so the breakage can no longer hide.
P2 — both the HTTP controller and the CLI released the user-deletion fence BEFORE
restoring schedules. That fence is what refuses new schedule writes/claims, so the
gap let an owner PATCH edit a still-suspended row and have its enabled/next-run
state overwritten by the older snapshot, and let a second deletion attempt
re-suspend under a new token — making the first restore a no-op and stranding the
disabled snapshot permanently. Restore now runs first, while writes are still
fenced.
Hardening for the same defect class across a crash: suspendUserSchedulesForDeletion
now ADOPTS an existing suspension's snapshot when re-suspending a row abandoned by
an earlier attempt, instead of re-capturing the row's current (already-suspended)
state. Without this, an attempt that died before restoring would have its
successor snapshot "disabled, no next run" and permanently strand the schedule.
Tests: CLI restore-before-fence ordering and no-restore-on-success; the same
ordering assertion on both controller post-quiesce failure paths; a data-method
regression that a second attempt adopts the abandoned snapshot and restores the
original enabled/next-run state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge dead deliveries in topology-safe maintenance
Codex finding: the `dead` delivery mapping added in Wave C.1 lives only inside
startScheduleEngine's reconciler, but the clustered entrypoint arms no engine — it
runs erasure-only maintenance. A delivery queued before a restart into clustered
mode and then rejected before generation creation (interactive limiter, PII,
moderation) dead-letters while its ScheduleRun stays `started`, holding a global
capacity slot indefinitely for an ordinary non-deleting schedule.
Add a dead-delivery convergence pass to the erasure sweep, so every topology that
runs schedule maintenance settles it. The pass is POSITIVE-EVIDENCE-ONLY and is
therefore safe where absence-based reconciliation is not: a `dead` delivery is
durable shared state proving no generation owns the reservation. It settles only
when the job is confirmed absent or identity-mismatched (an identity-matched job
still owns the run), defers on an unknown job lookup, on an in-flight abort, and
on an in-flight resume hand-off, ignores legacy reservations with no deliveryKey,
and applies a short grace so an accepted delivery still creating its generation is
never settled mid-handoff. Auto-disable policy is deliberately left to the armed
engine; this path records the failure and frees the slot.
Deliberately does NOT touch api/server/experimental.js — the clustered entrypoint
already starts this sweep, so the convergence arrives through the existing
initializer and the shared-file footprint stays as-is.
Tests: settles a dead delivery as error under an explicitly UNSAFE topology,
leaves live deliveries alone, never settles under an identity-matched running
generation (delivery is not even consulted), defers an in-flight abort, and
ignores a reservation with no deliveryKey.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): refresh host-action retention on each retry attempt
Codex finding: unacknowledged terminal host-action evidence was capped at the
24h pause TTL, so a host dependency (Mongo) unreachable for longer than that let
the Redis key — and its pending marker — expire with no generation-fenced
acknowledgement, stranding the ScheduleRun where no reconciler is armed.
Measure retention from the LAST retry rather than from the terminal transition:
enumerating a pending host action IS the retry attempt, so both stores refresh
its retention as they hand it to the hook (Redis re-EXPIREs the job key; the
in-memory store stamps terminalHostActionRefreshedAt and bounds from it). Evidence
therefore survives as long as some replica is still actively retrying, while a
deployment that stops sweeping entirely still lets it age out — so this does not
reintroduce the unbounded leak the cap existed to prevent.
Test: after a failed hook, a later cleanup pass keeps the marker pending and moves
its retention basis forward.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): defer settlement when the Stop barrier times out
Codex finding: waitForStopPersistence returned after its 5s poll budget even when
the Stop was still fresh and unacknowledged, and recordScheduleOutcome then went
straight on to terminalize the run — releasing its capacity, deletion, and erasure
barriers while beforePublish may still have been writing. Slow checkpoint cleanup
is indistinguishable from a dead route on that signal, so the timeout was being
treated as if the barrier had been satisfied.
The poll budget now means "undecided", not "clear". On timeout with a fresh,
unacknowledged Stop the barrier DEFERS: recordScheduleOutcome returns false
without recording, leaving the run active/preserved. Settlement then happens
either when the route acknowledges, or once the existing stale-owner cutoff
(ABORT_OWNER_PRESUMED_ALIVE_MS) authorizes a later attempt — which the loop
already treats as clear-to-settle. Callers with durable retry (the approval-expiry
host action, reconciliation) re-drive it, so a deferral converges rather than
stranding the run.
Test: a fresh Stop that never acknowledges within the budget reports not-settled
and records no outcome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): require definite delivery failure, extract its message, converge deferred Stops
Third Codex round. Three findings in code from this closeout, plus one pre-existing
P1 that is a one-line operator-safety fix.
P1 — dead-delivery settlement demanded too little evidence. `dead` does not prove a
request was rejected: the trigger host marks response timeouts and invalid success
responses `certainty: 'ambiguous'`, and the engine dead-letters those once retries
are exhausted. The erasure sweep treated every dead letter as positive evidence, so
an ambiguous one sitting over a generation a peer had accepted could terminalize the
run and release its capacity mid-flight. It now settles only on a DEFINITE rejection,
unless job absence is deployment-authoritative (safe topology), where the
confirmed-absent job is itself the evidence.
P1 — `lastError` is an `AgentTriggerDeliveryFailure` object, not a string. A
duplicated local interface declared it `string` (against CLAUDE.md's no-duplicate-
types rule), so both the sweep and the engine reconciler passed the object into the
String-typed run/schedule `error` fields; Mongoose would reject the cast, the per-row
catch would swallow it, and the run would keep its global capacity slot. The dep type
now reuses the canonical `AgentTriggerDeliveryFailure` and both call sites pass
`.message`. Re-typing immediately surfaced a stale test that had asserted a string.
P1 (pre-existing) — the base-config global stop is honored in `getLimits` via
`isRuntimeDisabled` rather than a literal `=== false`. The stop has two shapes, and
deepMerge turns base `{ use: false }` plus a principal override of `true` into
`{ use: true }`, so the literal check reported the feature enabled and Run Now
dispatched straight through fireSchedule, bypassing the operator's emergency stop.
Now the same predicate the engine gate already uses.
P2 — a Stop whose settlement DEFERRED past the poll budget had no convergence path
where no reconciler is armed. `acknowledgeScheduledStopPersistence` now optionally
re-drives the terminal outcome once the barrier clears; `recordRunOutcome` is
match-guarded and idempotent, so an owner that already settled makes it a no-op. The
abort route passes it for a running generation; a paused job still settles explicitly.
Tests: ambiguous dead letters refused under unsafe topology but settled when absence
is authoritative, definite rejections settled either way, the failure message carried
through, and the abort route's re-drive present for running / absent for paused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge terminal runs in clustered workers, retry suspension restore
Fourth Codex round on
|
||
|
Danny Avila
|
259f1e0c32
|
🛰️ feat: Route Live Subagent Controls Across Replicas (#14971)
* feat: route live subagent controls across replicas * fix: initialize task routing in cluster workers * fix: harden cross-replica task routing * fix: expire routed task owners independently * fix: close cross-replica routing edge cases * fix: bound owner refresh and close routed cancellation gaps Refresh owned task registrations in bounded parallel batches so a full heartbeat pass stays well inside the 30-second directory lease instead of serializing one Redis EVAL per registration. Route conversation-deletion cancellation through a dedicated owner-side scope operation. The owner applies the deletion predicate to its complete local task set, so a scope holding more children than the model-facing list cap no longer leaves live executors running after their parent is removed. Key a consumed claim's retained response by its operation rather than by one caller's correlation id, so a later poll recovers a terminal result whose responses were all lost. Live claim statuses stay uncached so a poll always observes the task's current state. Type the model-facing `maxLength` bounds with a narrow local string schema; the SDK's JsonSchemaType does not declare the keyword, and the runtime checks continue to enforce the same limits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: retain claimed results apart from control replays A consumed claim is the only routed response whose loss destroys data, so it no longer shares one bounded cache with control replays that unrelated command traffic can evict. Claims are retained under their own budget, and the requester acknowledges a result it received so the owner releases the copy immediately instead of holding it for the full replay window. Resolve the post-delete cancellation pass from durable leases. The deleted conversations cannot be read back, so re-reading each one only scaled the cascade while probing the owner directory once per removed id; one lease read now resolves every live child address instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: never consume a result the owner cannot replay Retention for consumed claims is bounded, so a burst of undelivered results could evict an earlier one and lose it for good. The owner now admits a claim only while it can retain a worst-case result, and refuses the routed claim otherwise instead of consuming it, leaving the result on the task for a later poll. Retained claims are never displaced; control replays keep evicting. Key a control replay by the command itself rather than by one caller's correlation id. The transport's own retry reuses a single envelope, but a caller that saw the owner as unavailable reissues the command under a new id, which steered, queued, or interrupted the child a second time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: own a claimed result until it is acknowledged A consumed terminal result is task-owned state, not a cache entry. It now carries no expiry at all: the owner holds it until a caller acknowledges receipt, and only then is it released. Retention stays bounded by the existing admission gate, which refuses a claim the owner could not keep rather than consuming a result it might drop. Identify a control by the caller's invocation instead of by its content. The tool mints one id per invocation and routing carries it, so a routed retransmission of that invocation replays the owner's result while two deliberate identical commands arrive under distinct ids and both apply. Content-derived identity could not tell those apart and would have answered the second from a stale snapshot. Wait for the dpkg frontend lock in the best-effort Playwright font step. Its timeout kills npx while the apt-get it spawned keeps the lock, which then failed the fatal Redis install and ended the MCP replica jobs before any test ran (#14983). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: treat acknowledgement as part of delivering a result Publishing an acknowledgement once and ignoring the outcome meant a result could be reported as delivered while the owner never learned it could let go, and since that retention neither expires nor evicts, enough lost acknowledgements would fill it and refuse every later remote claim. An acknowledgement is now confirmed: publishing to zero subscribers is not success, it retries inside the ordinary request window, and a claim whose acknowledgement cannot be confirmed reports the retryable unavailable path instead of handing back a result the owner still holds. A later poll recovers that result and acknowledges it, and releasing is idempotent. Owner registration also outlives the task while a result is unacknowledged, so the retained result cannot become unreachable. Take the control invocation identity from the provider's tool-call id rather than minting one per execution, so replaying the same tool call stays idempotent while two distinct calls with identical payloads both apply. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * style: sort the widened node:crypto import Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: own control invocations and cancellation plans at the task seam Applies one logical control exactly once for its owning task rather than in the transport, so a local caller and a routed caller of the same invocation agree, and reusing an invocation id for different content is refused instead of silently applied. Invocation identity now comes from the run, agent, and provider tool-call id hashed to a bounded 32 characters, so a repeated `call_0` never bleeds across tasks and no id can overrun the routed bound. Cancellation for conversation deletion is now resolved into a plan while those rows are still readable, then replayed against the owner directory after the cascade is deleted. Owner registration is awaited before any provider work, so a child that cannot be addressed never starts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * style: separate the control invocation map from the next member Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: close subagent deletion, claim, and control invocation gaps Bulk conversation deletion now runs behind a durable owner admission fence. Draining alone could not close the race: a child admitted on another replica after the drain read its leases would start provider work against a parent about to disappear. The fence is written before any lease is read and each child revalidates it after its own lease is written, so one of the two always observes the other. It expires on its own, so a process lost mid-deletion cannot leave an account unable to run subagents. A terminal child result is no longer kept alive in the owning replica's memory until someone acknowledges it. Collection is recorded durably on the child's own message against the polling invocation, so the poll whose response was lost recovers its own result while a different invocation is told the result was already collected. Owner-side retention returns to an ordinary bounded cache that expires, which is what abandoned polls needed: they can no longer occupy claim capacity until the process restarts. The deletion drain now cancels each task under one invocation held for the whole drain, stops re-sending once the owner answers, and retries only deliveries it could not confirm. A routed control replay also validates the command fingerprint, so one invocation id carrying different content reaches the owner to be refused instead of collecting the earlier command's success. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: assert the drain's calls before restoring its spies Restoring a spy also clears its recorded calls, so the drain assertions ran against an emptied mock. Formats the durable claim method tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: close the follow-on gaps in the deletion fence and result claim The admission fence now carries an ownership token, so an overlapping deletion's fence is never lifted by the one that finishes first, and both fence writes invalidate the cached auth user document. It also covers the other bulk-delete path: `DELETE /` with no conversation filter removes every conversation, so it runs behind the same fence rather than a bare drain. The durable record now decides who holds a one-shot result. An owner replaying a retained response could hand the same terminal claim to a second invocation; that invocation is told the result was already collected, while the one that consumed it still recovers its own. A task with no durable record to arbitrate keeps whatever the owner answered. Drain cancellation treats `not_found` as unconfirmed: a missing registration while the durable lease is still live means the child may be running, so the command is retried under its invocation once the owner republishes itself. Control fingerprints are hashed, so retaining one per invocation costs a fixed few bytes instead of a bounded message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: hold every deletion fence and keep live idempotency records An owner now holds one admission fence per concurrent bulk deletion instead of one at a time, so admission reopens only when the last deletion finishes regardless of completion order. Expired fences are pruned as new ones arrive and the set is bounded, so an abandoned fence cannot accumulate or lock an account out. A failed durable claim write is no longer read as an absent record. Handing a terminal result over without recording its claimant would let another invocation collect the same one-shot output once the database recovered, so the collection reports the retryable unavailable path and leaves the result for a later poll. Control invocation records now evict tasks the store no longer holds before live ones, over a bounded scan. Dropping a live task's record would let a caller retry apply its queue, steer, or interrupt a second time once the transport replay had also expired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: keep the deletion fence portable and never drop a live record The admission fence is written with plain update operators again. DocumentDB rejects pipeline-form updates, and this runs before any deletion, so the pipeline form would have failed both bulk-delete endpoints outright on a supported database target. An excess deletion is now refused rather than silently displacing the oldest active fence, which would have reopened admission for a deletion still running. Expired fences are pruned before the cap is tested, so only genuinely concurrent deletions count against it. Control invocation records now sweep every settled task's entry when the window fills, and a window of entirely live records refuses the new control before touching the child instead of evicting one. Applying a command with no room to record it would let the caller's own retry apply it twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: hold the fence, bound recovered results, and expire stale commands The admission fence is renewed for as long as its deletion runs, so a very large account or a stalled database cannot let it lapse while conversations are still being removed. Only the deletion's own fence is renewed, and the renewal stops with the operation. Cancellation now covers every conversation the cascade removed, not only the ones a plan named: a grandchild lives in its own parent's scope, which a plan naming the deleted root never reaches. A routed request carries the deadline its caller waits for, and an owner drops one that arrives past it. A publisher disconnected mid-request queues the envelope offline and delivers it after the caller was told the owner was unavailable, which would otherwise steer a child the caller believes untouched. A result recovered from its durable child message is bounded like a routed one. The message keeps the child's untruncated output, so recovery could otherwise return far more than the routed result limit allows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: size the fence window so a renewal can be observed The renewal test set a 30ms drain timeout but the five-minute grace window dominates it, so the interval was 100 seconds and no renewal could fire inside the test's deletion. The grace window is an option now, matching the store's other timings, and the test sizes the window to 90ms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: wire the durable claim method and close the fence follow-ons The production store never received `claimSubagentTaskResult`, so every terminal result would have surfaced as unavailable once a task settled. The host wires that object from JavaScript, where the factory's parameter type checks nothing, so the factory now refuses a store missing any method it calls rather than failing at the first claim. The routing transport takes a dedicated publisher with the offline queue disabled. The shared client held commands issued during a disconnect and delivered them after the caller had given up, which the request deadline narrowed but could not close inside the clock-skew allowance. Fence renewal invalidates the cached auth document like the fence and release paths, and a renewal reporting its entry gone re-takes the fence instead of letting the deletion run on unfenced. The post-delete cancellation retries a transiently unreachable owner: the conversations are already gone, so it is the only pass that can still stop a late-admitted child. A replaced replay entry no longer leaves its bytes counted, which would have inflated the cache's total until unrelated responses were evicted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: wait on observed lease renewal instead of a fixed delay The shared-lease renewal test held a 60ms lease and slept 100ms before asserting an overlapping worker was refused, so a loaded runner that starved the 10ms heartbeat past the TTL let the lease lapse and the second worker run. Spy on acquisition and renewal, then wait until a renewal succeeds past the acquired lease's own deadline — direct evidence the heartbeat carried it past expiry, with no timing assumption — and give the lease enough headroom that a stalled timer no longer decides the outcome. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): close the routing, fence, and cache gaps found in review Five separate seams, each with its own failure: `Cluster.duplicate` reads its first argument as a startup-node list and its second as the overrides, unlike `Redis.duplicate`, so the publisher's `enableOfflineQueue: false` was silently dropped under `USE_REDIS_CLUSTER` and a command issued mid-disconnect could still reach a child after its caller was told `unavailable`. Route both through `duplicateIoRedisClient`. The control window's capacity refusal ran before the store knew whether it owned the task, so unrelated local load could veto a cancellation bound for another replica. Establish that the task is local first and leave a remote one to its owner's window. `clearInterval` stops only future fence renewals. One already waiting on the database could resolve after the release, read its own lifted fence as expiry, and write a replacement that nothing remained to lift — closing subagent admission for the account until it aged out. Track the in-flight renewal, refuse overlapping passes, and await it before releasing. Every owner bounds its own task list, but the aggregation appended each batch whole, so the model-facing list grew with the number of replicas holding the scope. Cap the merged list while still reading every reply for the stale-registration sweep. The admission-fence prune commits independently of the fence that follows it, so a refused or failed push left the cached auth document describing entries the collection no longer held. Invalidate whichever way the second write goes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): cap the merged task list the poll tool actually reads Each owner bounds its own reply and the remote aggregation bounds their sum, but `listTasks` merged that bounded remote list with however many children this replica owns and returned it whole. `check_background_task` could therefore still receive roughly twice the advertised cap. Bound the deduplicated, sorted result and export the cap so both seams share one number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: admit every task the merged-list cap test starts The base store admits ten concurrent runs per scope by default, so starting 150 at once left most refused for capacity and the assertion never reached the merge it was written to check. Raise the cap for this store only; admission is a different invariant with its own tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): let a deletion notice its admission fence lapsing Renewal failures were logged and swallowed, so a run of rejected writes let the last confirmed `fencedUntil` pass while the deletion carried on believing admission was still closed — long enough for another replica to admit a child against conversations about to be removed. Track the deadline only a confirmed write advances, and check it after the drain, before anything is deleted: nothing has been removed at that point, so the operation fails closed and the caller retries once the fence can be held. A lapse detected after the rows are gone is logged instead, since reporting failure there would invite a retry against conversations that no longer exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: raise both concurrency caps the merged-list test trips Raising the per-scope limit left the store-wide `maxRunningTotal` at its default hundred, so fifty of the hundred and fifty starts were still refused. Verified against the base store directly this time: with only the per-scope cap raised it admits a hundred, and with both raised it admits all hundred and fifty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): close the fence renewal gap and keep running tasks listed A renewal that started before its deadline but landed after it was still credited with extending the fence from its own start time, so a window in which admission stood open was papered over: a child could take a lease the drain had already read past and the deletion would proceed without cancelling it. The deadline now only advances when the write lands while the previous one still holds; anything later records a lapse the fence cannot be restored backwards over. The model-facing cap sorted oldest-first and sliced, which dropped the newest tasks — including children that had only just started running, and which the poll tool offers no other way to discover. Bound by status instead: running children first, then the most recent settled results. Both caps share one helper, and the routed aggregation now bounds after its loop so the choice is made across every owner's reply rather than by whichever answered first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): finish the cap and the fence at the seams they still missed The status-aware cap only reached the requester: an owner's own reply still sliced positionally, so a replica holding more than the cap dropped its running children before the requester could bound anything. Both sides now share `boundedTaskList`. A fence that lapsed during the deletion itself was only logged. The rows are gone by then, so failing is still wrong, but the child another replica admitted while the fence was down is not: the fence is retaken and the drain repeated to cancel it. A child's lease renewal had the same retroactive hole the admission fence had — Mongo filters on the `now` captured before the call, so a write landing after the lease expired still moves the row forward, while an owner drain reading active leases in that gap saw the thread as free. The lease now carries its own deadline and a late renewal stops the executor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: cover the lease lapse and the post-deletion re-drain The owner-side cap shipped with a regression test; these two did not. One drives a lease renewal that succeeds only after the lease it was extending had expired and asserts the executor stops; the other lets the fence lapse during the deletion itself and asserts a second drain runs while the request still reports success. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix(agents): close live-task lifecycle gaps * test(redis): exercise cluster node discovery * fix(test): type cluster discovery seam * fix(ci): wait for orphaned apt processes * fix(ci): reserve time for apt drain * fix(ci): skip optional fonts in MCP jobs * fix(agents): recover tasks after owner loss * fix(agents): preserve local task discovery * fix(agents): initialize fail-fast cluster publisher * style(agents): sort routing test imports --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Ravi Kumar L
|
006e421cd2
|
💡 feat: add DB-backed admin insights (#14898)
* feat: add Mongo-backed admin insights * feat: gate insights with environment variable * fix: tighten insights access and activity metrics * fix: preserve insights date selections * perf: parallelize insights search aggregation * test: wait for MCP conflict recovery * test: satisfy strict MCP recovery typing * fix: disable insights pagination while loading * fix: localize insights range shortcuts * fix: bound insights search input |
||
|
Danny Avila
|
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 |
||
|
Danny Avila
|
f829aca9fb
|
🧩 fix: Align Tenant and MCP Configuration Resolution (#14904)
* fix: Align Tenant and MCP Configuration Resolution * fix: Preserve Operator-Owned MCP Entries * fix: Preserve Configuration Source Ownership * style: Normalize Middleware Import Order * fix: Preserve Process Server Precedence * test: Align Tenant-Aware E2E Setup |
||
|
Danny Avila
|
ee8c0abe2d
|
🪝 feat: Execute Agent Plugin Command Hooks (#14755)
* 🪝 feat: Execute Agent Plugin Command Hooks Implement the missing PluginHookExecutor boundary so deployment plugins' ai.librechat/hooks/hooks.json documents execute instead of loading inert: - Command executor runs handlers as child processes outside the API process: Claude-shaped JSON payload on stdin, exit 0 + JSON stdout as sanitized hook output, exit 2 blocks with stderr as the reason, minimal allowlisted environment plus PLUGIN_ROOT/PLUGIN_DATA, abort-signal kill - Plugin loading carries the parsed hooks document on the contribution and threads hookCapabilities from startup, gated on the operator opt-in DEPLOYMENT_PLUGIN_HOOKS (off by default: parsed-but-inert with warning) - Runs register every ready plugin hook onto the per-run HookRegistry after internal policy hooks, with once-per-conversation SessionStart dedup Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Harden Plugin Hook Execution Boundary Address CI and Codex/Copilot review findings on #14755: - Break the agents -> plugins import cycle: the run seam now reads a PluginHookSource wired at startup (mirrors the tool-approval registry) - Tighten plugin ask decisions to deny unless the run has HITL wiring, so an un-resumable interrupt can never strand OpenAI-compatible callers - Scope cross-run dedup keys by authenticated user and handler identity: caller-supplied conversation ids cannot collide across principals, and sibling SessionStart handlers all fire; once handlers persist across runs - Replace a literal NUL byte in source with an escape (file diffed binary) - Kill the whole detached process group on abort, not just the shell - Map exit 2 on events without a decision channel to preventContinuation - Reserve PLUGIN_ROOT/PLUGIN_DATA against allowlist overrides, quote PowerShell args, cap captured output by bytes with one-pass decoding, and serialize payloads inside the executor's error boundary - Fix import ordering flagged by the static checks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Close Plugin Hook Policy and Namespace Gaps Address the second Codex review round on #14755: - Drop updatedInput from plugin command outputs: hooks in one dispatch all receive the original arguments, so a plugin rewrite would reach the tool without the approval policy re-evaluating it (host-only now) - Translate Claude tool aliases (Bash/Write/Edit/Read) to LibreChat runtime names in matchers, with reverse payload mapping, so Claude-authored guards fire instead of planning ready and never matching - Key once-only state by declaration position as well as handler contents, so sibling declarations with identical handlers stay independent - Thread sessionStartSource through createRun and mark the HITL resume rebuild as 'resume', so SessionStart matchers see the real lifecycle Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Translate Regex-Form Claude Tool Aliases Address the third Codex review round on #14755: alias translation now substitutes word-bounded tokens, covering regex matchers like ^Bash$ and ^(Write|Edit)$ that the exact-token pass left registered against Claude names and silently never firing. A regex whose alias sits inside a character class or escape is rejected as unmapped so it fails loudly at plan time instead of never running. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Alias Translation and Reuse Load-Time Plans Address the fourth Codex review round on #14755: - Add the WebSearch -> web_search alias so Claude-authored web-search guards fire against the LibreChat built-in - Apply alias translation only to tool-name events; a StopFailure matcher like ^Bash failed$ stays untouched and keeps matching the error text - Reuse each plugin's load-time hook plan at run registration instead of re-planning up to 512 handlers on every chat turn Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Translate Aliased Tool Inputs and Harden Hook Domains - Present aliased tool inputs under Claude field names (file_path, old_string, new_string, including nested edits), so Write/Edit/Read guards see the fields they check instead of silently allowing - Derive the alias table from canonical tool-name definitions (BashExecutionToolDefinition, CREATE_FILE_TOOL_NAME, Tools.web_search) instead of a parallel hand-authored table - Reject matchers naming Claude built-ins with no runtime equivalent (Task, Glob, Grep, WebFetch, ...) as unmapped at plan time instead of registering guards that never fire - Replace per-event Sets and Stop special-cases with an exhaustive EVENT_TRAITS record over HookEvent, so new engine events demand explicit semantics at compile time - Move cross-run once-state behind a PluginHookOnceStore seam with a least-recently-marked memory default: active conversations refresh their keys each turn, so capacity eviction can no longer re-fire a conversation that is still in use; the seam admits a shared-cache store for multi-replica deployments - Gate portable-only command handlers at plan time on Windows via a new supportsHandler capability (commandWindows or shell powershell required) instead of spawning bash that cannot exist - Kill Windows hook process trees with taskkill /t on abort - Require declaration indices on execution requests, stamped from the plan instead of defaulted at execution time Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Keep Group SIGKILL Escalation Armed After Wrapper Exit An aborted hook whose descendant ignores SIGTERM could leak that descendant: the wrapper shell's exit fired close, which cancelled the scheduled group SIGKILL. The escalation timer is now never cancelled — it is unref'd and killTree already tolerates a vanished process group, so a redundant late sweep is harmless while a surviving descendant is reliably killed at the grace deadline. killGraceMs is configurable on CommandExecutorOptions, with a regression test driving a trap-protected descendant past the wrapper's exit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Once Retention by Conversation and Reject Clear Source - Restructure the once store around conversation scopes: registration touches the scope every run, so rarely-matching once handlers keep their keys while the conversation is active; eviction removes whole idle conversations (capacity counts conversations, not keys) - Reject SessionStart matchers naming the clear lifecycle source at plan time — no LibreChat run-construction path emits clear, so the handler would plan ready and never fire; wildcard warning text now reflects the sources that actually occur - Make the SIGKILL-escalation regression test real: the surviving descendant redirects its stdio away from the captured pipes so the wrapper's close fires while it is still alive, exercising the window a close-time cancellation would leak Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Bound Alias Tokens by Tool-Name Characters and Host Shells - Translate Claude aliases (and reject unsupported built-ins) only when delimited by characters that cannot appear in a runtime tool name: action tool names preserve hyphens, so an alias embedded in a longer name like deploy-Bash-v2_action_example_com stays the literal tool name instead of being rewritten into a matcher that never fires - Reject PowerShell-only command handlers on POSIX hosts at plan time (and skip them at runtime): bash cannot run PowerShell syntax, so the guard would fail open; a handler with both variants still runs its portable command - Handle rejected asynchronous once-store calls: a failed touch logs instead of raising an unhandled rejection during run construction, and a failed markOnce lookup fails open per the store's documented over-fire direction Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Probe Group Liveness Before Cancelled or Delivered SIGKILL The never-cancelled escalation timer could signal a recycled process-group id when an aborted hook's whole tree exits early in the grace window. Escalation now probes the group with signal 0: close cancels the timer only when the group is verifiably empty, and the deadline re-probes before delivering the group SIGKILL, so surviving descendants are still reaped while a fully-dead group never receives a blind late signal. The residual probe-to-signal race is documented as irreducible without pidfd support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Gate Windows Escalation on Root-Process Liveness Windows taskkill /t walks the tree from the root process, so once Node observes the root's exit an escalation pass can reap nothing and a late forced taskkill could only hit a recycled PID. The liveness gate is now platform-aware in one helper: POSIX probes the process group with signal 0, Windows checks the root's observed exit state, and both the close-time cancellation and the deadline delivery consult it — no platform retains a blind late signal. Orphaned SIGTERM-ignoring descendants on Windows are documented as the platform limitation they are without Job Objects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Payload Namespace to Declarations and Reap Stray Workers - Reverse name/input translation now applies only to declarations whose matcher actually required Claude-alias translation: the plan records requiresToolNameTranslation per entry, so a native-authored matcher like ^create_file$ receives native tool names and fields instead of Claude-shaped payloads its guard never expected - Coordinate the two dedup layers via a shouldExecute gate on the executor: a declaration suppressed by spent once-state declines before claiming the per-input dedup slot, so an identical handler under an overlapping matcher can still claim it and fire its own independent once-key instead of being permanently shadowed - Reap process groups that outlive a successful hook: a backgrounded worker left running after normal wrapper exit gets the same term-then-escalate sequence an abort uses, since unsupported async handlers mean no lifecycle owns such processes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🧰 chore: Vendor Pocock Codebase-Design and Architecture Skills Adds mattpocock/skills engineering/codebase-design and engineering/improve-codebase-architecture (MIT, license included) under .claude/skills so future sessions share the deep-module vocabulary (module, interface, depth, seam, adapter, leverage, locality) and the architecture-review process. Force-added past the /.claude/ gitignore deliberately; relocate if project skills should live elsewhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 refactor: Extract Process-Tree Reaping Into a Reaper Module Tree lifecycle — five of the last seven review findings — lived as event-handler wiring inside runCommand with its invariants in comments. It now sits behind a two-method seam: createReaper(child, graceMs) exposes reap() and onClose(), hiding the term-grace-escalate state machine, the per-platform liveness gates, the recycled-id guards, and the clean-exit sweep. The executor shrinks to capture-and-parse, and the reaper is unit-tested directly with real process trees through its own interface instead of only via whole-executor integration runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Translation Per Alternative and Sweep at Root Exit - Track which runtime tool names alias translation produced, so a mixed-namespace matcher like Bash|create_file presents Claude-shaped payloads only for bash_tool invocations while the natively-authored create_file alternative keeps native names and fields; a capability omitting the produced-names list keeps declaration-wide translation - Sweep the process tree at root exit as well as close: a backgrounded descendant holding the captured pipes delays close until it dies, so the exit-time sweep terminates it promptly instead of stalling the hook until its timeout aborts - Pass the primary agent's resolved model and identity into the plugin hook context, so SessionStart payloads carry model and agent_type instead of always omitting them Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Default Wildcard Declarations to the Document Namespace - Matcherless (or wildcard) tool-payload declarations now inherit the hook document's Claude namespace: with no alternatives to carry namespace evidence, the plan marks them for declaration-wide reverse translation, so a wildcard guard inspecting standard Claude names and fields sees Write/file_path instead of silently failing open on native payloads; PostToolBatch entries translate the same way - Recognize aliases delimited by regex metacharacters: dots leave the tool-name boundary class (runtime names never contain them — action ids underscore domain dots), so ^Bash.*$ translates to ^bash_tool.*$ instead of registering a guard that never fires - Expand Claude's ${CLAUDE_PLUGIN_ROOT} spelling in hook commands and export it in the child environment alongside PLUGIN_ROOT - Scope SessionStart once-keys by lifecycle source, so a startup firing no longer suppresses the conversation's resume rebuild Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Normalize Claude Structured Hook Output Stock Claude hooks return decisions under hookSpecificOutput (permissionDecision/permissionDecisionReason), surface context there, and use continue:false plus the legacy approve/block decisions — none of which the sanitizer's native field names recognized, so a guard that works in Claude silently allowed in LibreChat. Parsed JSON now passes through a dialect normalizer first: hookSpecificOutput fields map to decision/reason/additionalContext, continue:false becomes preventContinuation, approve becomes allow, and block becomes deny on events that block by denying. Native fields win when both dialects appear, and the ask-to-deny gate applies to the Claude dialect too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Validate Native Decisions and Slim Once Keys - Strip malformed native output fields before the dialect merge, so a placeholder like {"decision":null} can no longer suppress a valid Claude permissionDecision into a silent allow; only recognized decision tokens take precedence - Preserve the caller's working directory in hook payloads: cwd now reports the run's session context instead of the plugin installation path, which commands already receive as PLUGIN_ROOT and which the executor still uses as each process's working directory - Store a compact sha256 digest instead of the full serialized handler in once keys: declarations may carry 32 KB commands and 256 args, and the previous key embedded them in every retained conversation scope Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Validate Decisions Per Event Channel and Control Post-Tool Blocks - Accept native decision tokens only from the target event's own vocabulary: "continue" is valid on Stop but malformed on a tool event, where it previously survived validation, blocked the Claude dialect merge, and was then dropped by sanitization into a silent allow - Translate a structured "block" on events with no deny channel (PostToolUse, PostToolUseFailure, and the other prevent-trait events) into preventContinuation with the block reason as stopReason, instead of discarding it and returning a reason that controls nothing - Document why LibreChat runs supply no payload cwd: tool paths address a remote code-execution sandbox rather than the API host where hook commands run, so no host directory describes the run Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Danny Avila
|
5c939d129b
|
🔌 feat: Add Agent Plugins (Experimental) (#14704)
* 🔌 feat: Add Agent Plugins v1.0.0 Support Implements the Agent Plugins 1.0.0 specification so LibreChat can load portable plugin packages: a `plugin.json` manifest, `skills/` holding Agent Skills, `mcp.json` describing MCP servers, and reverse-domain extension directories. - Validate the closed `plugin.json` schema, selecting rules from `$schema` without retrieving it. Unknown top-level fields and a non-object `extensions` field are reported and ignored; every other violation rejects the plugin. - Enforce plugin-root containment through realpath, including for paths whose leaf does not exist, and apply the narrowest failure boundary per component. - Map `mcp.json` onto LibreChat MCP options across stdio, Streamable HTTP, and legacy HTTP+SSE, bypassing the config loader's `${VAR}` process-env expansion so plugin values never resolve against the server environment. - Expand only `${PLUGIN_ROOT}` and `${PLUGIN_DATA}`, once and non-recursively, in `args`, `env` values, and `cwd`; supply both variables to the subprocess after configured `env`, and reject entries that declare them. - Discover skills from the immediate children of `skills/` only, reusing the deployment skill loader so plugin skills are ordinary deployment skills with a distinct id namespace. - Read LibreChat's `ai.librechat` extension directory and hand `hooks/hooks.json` to the Claude hook compatibility layer. - Load operator-installed plugins from `DEPLOYMENT_PLUGINS_DIR` at startup, merging their skills into the deployment skill registry and their MCP servers into the app config. Plugins never displace a configured server or deployment skill. - Add `cwd` to the stdio MCP transport, which the specification requires and LibreChat did not previously support. Component failures stay isolated: a malformed `mcp.json`, an invalid skill, or a bad hooks document never prevents the rest of a plugin from loading. * 🔒 fix: Contain Agent Plugins config at the runtime boundary Review of #14704 surfaced that every real finding sat where the loader's output crosses into LibreChat's existing runtime, not in the specification logic. The loader deliberately left plugin placeholders literal, but downstream layers re-processed the same fields and undid it. - Mark plugin MCP configuration with `source: 'plugin'` and return it verbatim from `processMCPEnv`. Without this a remote plugin could declare `Authorization: Bearer ${OPENAI_API_KEY}` and receive host credentials at its own origin. The gate reads the configuration rather than a caller-supplied flag, so no future call site can reintroduce the leak by omitting it. - Skip `preProcessGraphTokens` for plugin configuration as well; it resolves placeholders into headers, url, and args on the same path. - Reject plugin server names that change under `normalizeServerName`. Tool keys embed the normalized name while request-time resolution uses the raw name, so an unstable name published tools that nothing could resolve. - Reject `__proto__`, `constructor`, and `prototype` as server names, and merge plugin servers with `Object.defineProperty` and an own-property conflict check, so a package cannot reach a prototype setter or collide with an inherited member. - Enforce manifest-name uniqueness before components are accepted; two packages sharing a name would share one `PLUGIN_DATA` directory. - Isolate a failed data-directory creation to the single plugin instead of rejecting the whole scan. - Prefix rejected-plugin diagnostics with the directory, which is the only identifier a package without a valid manifest has. - Type extension namespace contents as JSON rather than `unknown`, and correct the header field-value comment to name obs-text. Verified end to end from the built package: a plugin declaring an environment placeholder in a header reaches the transport with the placeholder intact while operator-authored configuration still resolves normally. * 🔇 fix: Report Agent Plugin hooks that will not run The loader reads `ai.librechat/hooks/hooks.json`, but nothing registers the resulting plan, and startup supplies no hook capabilities. A package declaring hooks was therefore accepted in silence, leaving an operator to believe the hooks ran. Detect the document when no capabilities are registered and report it as unsupported, so the limitation is visible in startup diagnostics rather than inferred from behavior that never happens. * 🧯 test: Restore MCP startup test mocks Carries the two mock additions from #14711 so this branch can prove itself green. `initializeMCPs` now calls `syncStaticTools`, which the server startup specs do not stub, so they fail on every branch that has not picked this up. Drops out of the rebase once #14711 lands. Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Danny Avila
|
1bd4455c2d
|
🧭 fix: Make MCP Catalog Redis Cluster-Safe (#14717)
* fix: make MCP catalog Redis startup cluster-safe * fix: stabilize Redis readiness gate * fix: type Redis readiness export * style: apply canonical import order |
||
|
Danny Avila
|
1596df724a
|
chore: Remove Published Credential Defaults (#14680) | ||
Dustin Healy
|
0e14d91ed9
|
⏱️ fix: Compile admin file-config MIME patterns on a linear-time engine (ReDoS) (#14555)
* ⏱️ fix: Compile admin file-config MIME patterns on a linear-time engine convertStringsToRegex compiled admin-configured supportedMimeTypes with the native RegExp engine, and checkType runs those patterns against an uploaded file's Content-Type on the server event loop, so a catastrophic-backtracking pattern in fileConfig could ReDoS the whole process on upload. The MIME-pattern compiler is now swappable. It defaults to native RegExp, which browser builds keep so no engine is added to the client bundle, and the server injects a linear-time engine (RE2JS) at startup. Only test is ever called on these matchers, so the shared type widens to a structural RegexLike with no behavior change for valid patterns. The browser stays on native because a client-side stall would only affect that one tab. * ⏱️ fix: Wire the linear MIME compiler in the experimental entry point api/server/experimental.js mounts the same upload routes and calls mergeFileConfig but never set the linear-time compiler, so admin MIME patterns still compiled with native RegExp there. Mirror the setup, and widen the client-side supportedMimeTypes type to the shared RegexLike so the browser typechecks against the same structural matcher. * 🧹 refactor: Configure the file-config linear engine from a shared helper Move the RE2 wiring out of both JS server entry points into a single configureFileConfigRegexEngine helper exported from @librechat/api, so /api stays a thin caller and the setup no longer has to be kept in sync across index.js and experimental.js. Also warn loudly when compiling an endpoint's supportedMimeTypes drops every pattern (an empty allowlist would reject all uploads), and correct the isMimeTypeSupported docstring to say RegexLike rather than RegExp. * fix: fail closed when every MIME pattern fails to compile convertStringsToRegex returned [] when all configured patterns failed to compile, and filter.ts reads an empty allowlist as no restriction, so a restrictive config whose patterns all fail allowed every attachment. Return a single reject-all matcher instead so every consumer fails closed. |
||
|
Danny Avila
|
dd159c4566
|
🔐 fix: Preserve Structured JWT Auth Context (#14652)
* 🔐 fix: Preserve structured JWT auth context
* fix: Omit identity from auth correlation logs
* fix: Isolate pre-auth request context
* style: Sort auth context imports
* fix: Preserve structured auth metadata
* fix: Narrow structured log formatter types
* fix: Annotate structured log context keys
* test: Fix request fixture typing
* fix: Namespace request path log context
* fix: Namespace request method log context
* fix: Preserve captured tenant error paths
* style: Sort tenant error imports
* fix: Classify bulk tenant isolation failures
* fix: Enforce safe request correlation invariants
|
||
|
Dustin Healy
|
3f0a1ec8d9
|
🛡️ fix: Run message-filter PII patterns on a linear-time regex engine (ReDoS) (#14554)
* 🛡️ fix: Run message-filter PII patterns on a linear-time regex engine The messageFilter.pii middleware compiled admin-configured customPatterns with the native RegExp engine and ran them synchronously against every message on the shared event loop, so a catastrophic-backtracking pattern such as (a+)+$ could stall the entire process (native RegExp takes tens of seconds at roughly 32 characters) and take the instance down for every user. Compile these patterns with RE2JS, a linear-time RE2 port with no native addon, so catastrophic backtracking is impossible regardless of the pattern rather than something the code tries to detect. Patterns using features RE2 does not support, such as backreferences, fail to compile and are dropped and logged exactly as an invalid pattern already is. The filter only tests for a match, so this is a drop-in engine swap with no behavior change for valid patterns. * 🛡️ fix: Reject RE2-incompatible messageFilter patterns at config load The customPatterns regex was validated with native RegExp at config load, but the runtime now compiles it with a linear-time engine (RE2) that does not support backreferences or lookaround. Such a pattern passed validation, then failed to compile and was silently dropped at request time, quietly removing PII protection after upgrade. Reject backreferences and lookaround during config validation with an explicit message, and document RE2 syntax in the example config instead of "JavaScript-flavor". The runtime engine remains the authoritative boundary and still drops-and-logs anything this load-time check misses. * 🧹 test: Use direct MessageFilterPiiConfig annotations in the PII specs The added ReDoS cases satisfy the exported MessageFilterPiiConfig type directly, so the `as unknown as` assertions were unnecessary. Annotate the config objects directly, matching the repo's type-safety guidance. * 🧹 fix: Reject named backreferences in messageFilter patterns at config load Extend the config-load check to also reject named backreferences (\k<name>), which are valid JavaScript regex but unsupported by the linear-time runtime engine, so they surface at load rather than being dropped at request time. Together with the existing numeric-backreference and lookaround checks this covers the RE2-incompatible construct set; the runtime engine remains authoritative. * 🛡️ fix: Preserve Unicode whitespace matching in messageFilter starter patterns RE2's \s is ASCII-only, so after the engine swap the built-in api-key and Bearer starters no longer matched a secret separated by non-ASCII whitespace (e.g. a non-breaking space), which native RegExp did match. Broaden the whitespace classes to [\s\p{Zs}] so those patterns keep their original coverage, and add a regression test for a non-breaking-space separator. * 🛡️ fix: Validate messageFilter patterns with the RE2 engine at config load Replace the syntax blacklist (numeric/named backreferences, lookaround) with authoritative validation: config load now compiles each custom pattern with the same linear-time engine the runtime uses, so any RE2-incompatible construct (including control escapes like \cA) is rejected at load with a clear error instead of being silently dropped at request time. The validator is swappable and defaults to native RegExp so browser builds add no engine; the server wires the RE2-backed check at startup via configureMessageFilterRegexValidator in both entry points. * 🛡️ fix: Match the full whitespace set in messageFilter starter patterns RE2's `\s` omits the vertical tab and `\p{Zs}` omits U+2028, U+2029, and U+FEFF, so a separator built from one of those characters slipped past the `api-key` and `Bearer` starter patterns and reached the model. Broaden the starter whitespace class to the full JavaScript whitespace set so those separators are covered again. * fix: fail closed when messageFilter.pii compiles to zero patterns DB and admin config overrides bypass the RE2 schema validation (it only runs at YAML load), so an override whose only pattern is RE2-incompatible was dropped at compile time, left zero patterns, and let the request through. compile() now returns a failClosed flag when a config declared patterns but every one failed to compile; the middleware returns 400 and findPiiMatchInMessages returns a distinct misconfigured match that the OpenAI and Responses controllers surface with an admin-facing message. * 🛡️ fix: Fail closed when any messageFilter.pii custom pattern drops compile() previously set failClosed only when every pattern dropped (patterns.length === 0 && dropped > 0). With the default starters present, a single RE2-incompatible custom override incremented dropped but left patterns.length > 0, so the filter silently enforced only the surviving subset and text matching only the dropped rule passed. failClosed now keys off dropped > 0, so any dropped custom pattern blocks with the misconfigured 400. YAML patterns are RE2-validated at load, so dropped stays 0 for valid configs and only unvalidated DB or admin overrides can trip it. Reframed the two keeps-others-active specs to assert fail-closed and added a default-starters partial-drop regression. * 🧹 fix: Correct the misconfigured JSDoc and drop redundant casts in the PII specs The misconfigured flag now means any configured custom pattern failed to compile, not that every pattern failed, so its JSDoc on PiiMatch is updated to match. The partial-drop regressions now use direct MessageFilterPiiConfig annotations instead of as-unknown-as casts, keeping the specs type-checked, consistent with the rest of the suite. |
||
Dustin Healy
|
af795be0c2
|
🪢 feat: Langfuse Fanout Connection Setting (#14108)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* feat: encrypt tenant Langfuse secret in admin config Add generic per-field secret encryption to the admin config layer: registered secret paths (langfuse.secretKey) are encrypted with encryptV3 on write and a non-secret fingerprint companion is stored. Admin config reads (base + per principal) redact registered secrets so they are never returned; the fingerprint is kept so the UI can show which key is configured. The Langfuse fanout read path decrypts the tenant secret before export. Adds secretKeyFingerprint to langfuseConfigSchema and tests for the encrypt/redact policy. * fix(api): secure admin config secret handling * fix(api): preserve encrypted langfuse config secrets * fix(api): couple config secret fingerprint deletion * fix(api): read langfuse fanout collector url from env * fix(api): display langfuse secret key hint * fix(api): remove langfuse secret fingerprint breadcrumbs * fix(api): use langfuse destination keys for tenant config * fix(api): remove langfuse config compatibility fallbacks * refactor(api): simplify langfuse secret helpers * refactor(api): simplify langfuse config secret handling * feat: in-app Langfuse connection settings panel Add a discoverable, admin-gated Langfuse connection panel inside LibreChat Settings (Dify-style): enable toggle, host, public key, masked write-only secret, configured-key fingerprint, and a test-connection action. Backed by a dedicated /api/admin/langfuse/connection endpoint that encrypts the secret at rest, returns metadata plus fingerprint on read, and validates credentials. Builds on the per-field encryption and fanout decrypt from the langfuse-config-encryption branch. * refactor: align Langfuse secret field to CustomUserVars pattern Use the established SecretInput plus Set/Unset state pill (com_ui_set/com_ui_unset) from the MCP CustomUserVars UI for the saved-secret state, instead of a bespoke masked input. * fix: drop em dash from saved-secret placeholder * feat: show loading state on Langfuse test connection button * feat: gate in-app Langfuse settings on fanout config and admin role * test: align Langfuse connection spec with SecretInput refactor * feat(langfuse): refine tenant connection controls * fix(admin): refine Langfuse connection verification * fix(langfuse): refine tenant connection settings * fix(langfuse): simplify export enablement controls * fix(langfuse): validate tenant export configuration * fix(langfuse): align startup fanout gate * fix(admin): time out Langfuse verification * fix(ui): rename Langfuse connection setting * fix(admin): enforce Langfuse config capability * feat(langfuse): require explicit tenant export activation * feat(langfuse): support single-tenant connection settings * fix(i18n): remove obsolete integrations label * fix(langfuse): authenticate ingestion verification * fix(langfuse): validate public key independently * fix(langfuse): localize connection errors * perf(config): skip Langfuse checks for non-admins * fix(langfuse): preserve trace sampling for feedback * test(langfuse): fix feedback sampling fixture * fix(langfuse): align secret preview field * fix(langfuse): harden connection settings state * fix(langfuse): preserve trace destination state * fix(langfuse): enforce tenant-wide routing invariants * fix(langfuse): preserve verified connection invariants * fix(langfuse): preserve stable project identity * fix(langfuse): warm project identity asynchronously --------- Co-authored-by: Ravi Kumar L <ravi.lazar@clickhouse.com> Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Danny Avila
|
324584552c
|
⏱️ feat: Configurable HTTP Server Timeouts (#14481)
* http server config added
* Fix TypeScript compatibility by accepting NodeJS.ProcessEnv directly when applying optional HTTP server timeout configuration.
* fix(api): configure HTTP server timeouts for clustered workers
* 🕰️ fix: Warn When HTTP Timeouts Are Not Enforced
Codex review of the rebased contributor work surfaced two ways these settings
silently do nothing. Both reproduce, and neither was reported to the operator.
Bun accepts the four property assignments and reflects them back, but does not
enforce them: with keepAliveTimeout=100 and buffer=1000, Bun 1.3.13 held a
keep-alive connection past 3s where Node 24 closed it at 1101ms. Since `b:api`
runs the server under Bun, the existing info log confirmed a configuration that
was not in effect. Warn instead.
Node sweeps header/request timeouts on `connectionsCheckingInterval`, a
createServer option that `app.listen()` leaves at 30s, so sub-30s values round
up to it: headersTimeout=2000 returned 408 at 30004ms by default versus 2010ms
with a 250ms interval. Warn on values below the sweep interval rather than
restructure server construction, since every documented value and both Node
defaults already sit well above it. keepAliveTimeout is socket-driven and stays
exact, so it is excluded.
Both caveats documented in .env.example.
* 🩹 fix: Inject Runtime Versions Instead of Mutating `process.versions`
The spec deleted `process.versions.bun` to reset between cases, which failed
typecheck with TS2790: `@types/bun` is a packages/api dependency and augments
NodeJS.ProcessVersions with a required `bun: string`, so the property is not
optional and cannot be deleted. Assigning undefined would fail for the same
reason.
That augmentation also made the production check dishonest: TypeScript saw
`process.versions.bun` as always a string, so `!= null` read as a no-op branch
even though it is correct at runtime under Node.
Both resolved by taking runtime versions as a third injectable parameter,
matching the existing `environment` parameter. Callers in api/server are
unchanged, the narrow `{ bun?: string }` type restores honest narrowing, and
the tests no longer mutate global state, so they assert the same behavior
whether the suite runs under Node or `bun jest`.
* 📏 fix: Stop Claiming a Ceiling on Sweep-Delayed Timeouts
The warning added in
|
||
|
Danny Avila
|
73699b5c25
|
⚡ perf: Reduce Agent Chat Startup Latency (#14423)
* perf: reduce agent chat startup latency * test: align Redis stream readiness assertions * perf: overlap remaining agent startup work * perf: persist initial agent job metadata atomically * test: add agent startup latency benchmark * fix: harden resumable agent stream lifecycle * fix: isolate replacement stream lifecycles * fix: preserve terminal stream epochs |
||
|
Danny Avila
|
84fa6aa820
|
🧹 feat: Eager HITL Checkpoint Cleanup (Expiry + Deletion) & Full-Wiring E2E (#14123)
* feat: eager HITL checkpoint cleanup on expiry + deletion, full-wiring e2e Follow-up to the lazy checkpointer (#14024): two paths still left a paused run's durable checkpoint to the 24h Mongo TTL, and no test exercised the whole HITL seam with real components. 1. Approval expiry: GenerationJobManager.setApprovalExpiredHandler(fn) — a non-destructive host hook fired after expireApproval's CAS succeeds (periodic sweeper AND stale-submit path), safe on startups that run constructor defaults. Both startups (index.js configureGenerationStreams, experimental.js) register a handler that prunes the checkpoint, resolving config lazily per expiry (streamId === conversationId === thread_id). 2. Conversation deletion: deleteConvos now returns the deleted conversationIds; the three deletion paths (DELETE /convos, DELETE /convos/all, account deletion) prune them via the new bulk deleteAgentCheckpoints (one $in deleteMany per collection). The delete routes gain configMiddleware for the checkpointer config. 3. Full-wiring e2e (hitlCheckpoint.e2e.spec.js): real SDK Run driven by FakeChatModel calling a gated tool, real PreToolUse/humanInTheLoop wiring, real LazyMongoSaver over mongodb-memory-server, real GenerationJobManager, real /resume controller via supertest. Asserts: clean turn persists nothing; error turn persists nothing; pause -> HTTP approve -> gated tool executes exactly once -> finalize prunes the checkpoint; expiry prunes the abandoned pause eagerly. Tests: 3 handler unit tests (pendingAction.spec), 2 bulk-prune integration tests (checkpointer.integration.spec), convos route + deleteUser specs updated, 4 e2e scenarios. 207 tests green across changed areas. * fix: tenant-scoped expiry prune, store-won expiry relay, resilient deleteConvos ids Codex round 1 on #14123 — all three valid: 1. The approval-expired handler now receives the expired JOB so both startups resolve config in the paused job's tenant/user scope (getAppConfig({userId, tenantId})) — a tenant checkpointer override no longer sends the prune to the base config's collections. expireApproval fetches the job best-effort. 2. Multi-replica: when RedisJobStore.cleanupRequiresActionIndex wins the expiry CAS on another replica, this replica's sweeper relay branch now runs the approval-expired cleanup too (prune is idempotent) — store-driven expiry no longer bypasses the hook. 3. deleteConvos: post-delete cleanup (deleteMessages, project stats refresh) is now best-effort — the conversations are already gone, so throwing hid the deletion and dropped the conversationIds the checkpoint prune needs, unrecoverable on retry. Updated the existing tag-decrement-on-failure test to the new contract (ids still returned). Tests: handler-receives-job, store-won relay path, ids-survive-cleanup-failure. 134 tests green across changed suites. * fix: relay cleanup independent of cached errorEvent; enter tenant ALS context Codex round 2 on #14123: 1. The sweeper's relay branch gated BOTH the terminal-error emit and the new checkpoint cleanup on !runtime.errorEvent — but a reconnect seeds errorEvent from the aborted job (runtime-state creation), which then suppressed the cleanup entirely. The emit stays gated; the idempotent cleanup now runs independent of the cached error, once per runtime lifetime (approvalCleanupRan flag — the aborted job is swept repeatedly). 2. Passing userId/tenantId to getAppConfig only keys the config cache; the Config query is ALS-scoped by the tenant-isolation plugin. Both startup handlers now ENTER the paused job's tenant context via tenantStorage.run before resolving config + pruning, so a tenant checkpointer override is honored in strict and non-strict modes. Tests: relay-cleanup-with-cached-error (reconnect simulation), repeated sweeps run the cleanup once. 27+4 green. * fix: dedup expiry cleanup across winner and relay paths Codex round 3 (P3): expireApproval ran the handler without marking the runtime's approvalCleanupRan flag, so the next sweep's relay branch (the aborted job outlives expiry for the completed-job TTL) ran the cleanup a second time. The dedup now lives inside runApprovalExpiredHandler — the single choke point both paths call — set-before-run, once per runtime lifetime. Test: local expiry followed by a sweep fires the handler once. |
||
|
Danny Avila
|
424ccffd83
|
🪝 feat: Configurable Tool-Approval Policy via Programmatic Hooks (#14025)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Has been cancelled
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
* 🪝 feat: Programmatic tool-approval hook seam (configurable beyond on/off) Adds a process-wide registry so host code can plug context-aware PreToolUse decision hooks into the tool-approval policy, composing with the static `endpoints.agents.toolApproval` config instead of replacing it. - `registerToolApprovalHook(factory, { matcher? })` — register a factory that builds a PreToolUse hook per run from a ToolApprovalHookContext (userId, conversationId, tenantId, appConfig); return undefined to opt the run out. Returns an unregister fn. - `buildHITLRunWiring(policy, context)` now registers the static-config policy hook as the baseline, then layers each resolved host hook after it. Decisions fold in the SDK as deny > ask > allow, so a host hook can only TIGHTEN a configured ask/deny — it can never silently auto-approve past policy (to loosen, change the static policy). updatedInput / allowedDecisions follow the SDK's last-writer-wins, so host hooks win over the baseline. - `createRun` threads the per-run context (user / conversation / tenant / appConfig) into the wiring; non-HITL and HITL-disabled runs never invoke any factory. This unlocks dynamic policy the static name-lists can't express — per-args (e.g. ask before write_file outside a workspace, the SDK's createWorkspacePolicyHook shape), per-agent, per-user. Inert until tool approval is enabled and the caller is hitlCapable. Tests: registry register/unregister/opt-out/order (hooks.spec.ts) + wiring composition, context passthrough, and disabled-path inertness (runtime.spec.ts). Full HITL suite green. * 🪝 feat: Config-driven tool-approval hook loader (librechat.yaml → hook modules) Lets operators declare programmatic tool-approval hooks in config instead of code, so the registerToolApprovalHook seam is usable without a custom build. - Config (data-provider): `endpoints.agents.toolApproval.hooks[]`, each entry `{ module, matcher?, options? }`. `module` is a bare package name or a path (resolved against the app root); its default export is a builder `(options?) => ToolApprovalHookFactory`. - Loader (@librechat/api `loadToolApprovalHooks`): imports each module, builds the factory with the entry's options, and registers it (with its optional tool-name matcher). Reload- safe (each call first unregisters its previous batch, leaving code-registered hooks alone) and robust — an unimportable module / non-function export / throwing builder is logged and skipped, never crashing startup or blocking the other hooks. Importer is injectable for tests. - Startup (api/server/index.js): loads the configured hooks once after appConfig resolves. SECURITY: modules are dynamically imported + executed in-process; this is admin-level config, documented as trusted-code-only. Tests: 9 loader cases (default/no-default export, options passthrough, bad-export skip, builder-returns-non-function skip, import-failure resilience, continue-past-bad-entry, reload de-dup). Full HITL suite green (80). * 💄 style: Sort imports in HITL hook spec files (CI sort-imports:check) * 🛡️ fix: Harden tool-approval hook loader (Codex review) Six P2 findings on the hook loader / startup wiring: - CJS/transpiled interop: unwrap a nested `default` (TS/Babel `exports.default = fn` surfaces through import() as `{ default: { default: fn } }`) before rejecting a module, so documented default-export hook modules actually load. - Validate the matcher regex at load time and skip invalid ones — the SDK compiles it with `new RegExp` at run-build time, where a bad pattern would throw out of buildHITLRunWiring and break EVERY HITL run instead of just skipping the one bad hook. - Honor the `enabled` kill switch: startup now passes hooks to the loader only when toolApproval is enabled, so a disabled endpoint imports/runs nothing (and unregisters any prior batch). - Resolve app-root-relative paths without a leading dot: a bare specifier that is a real file under basePath (e.g. `config/hooks/workspace.js`) resolves as a path; scoped/other bare names still import as packages. - Base-config-only: documented that hooks register once process-wide at startup and are NOT reloaded from per-role/user/tenant overrides — encode per-tenant logic inside the hook. - Wire the loader into the clustered startup path (api/server/experimental.js) too, not just the standard server. Tests: CJS-interop unwrap, invalid-matcher skip (+ sibling still loads), and specifier resolution (app-root file / bare package / ./relative). Full HITL suite green. * 🛡️ fix: Read tool-approval hooks from base config in clustered startup (Codex) The clustered experimental.js path read toolApproval from getAppConfig() (which merges DB __base__ overrides with no principal), so a DB override could enable/disable/replace toolApproval.hooks and import those modules in every worker — violating the base-config-only contract and diverging from the standard server. Fetch getAppConfig({ baseOnly: true }) specifically for the hook loader, matching api/server/index.js. |
||
|
Dustin Healy
|
6a63531eb4
|
📒 feat: Audit Log Backend for SystemGrant Assign and Revoke Events (#13087)
* 🛡️ feat: Audit log backend for SystemGrants changes
Add an AuditLog Mongoose collection that records every grant assign/revoke as an append-only entry capturing the actor, target principal, capability, timestamp, and tenant scope. Wire the entry-write into the existing admin assignGrant and revokeGrant handlers so the admin panel's audit-log tab populates as grants happen.
The data-schemas package gains the IAuditLog type, a Mongoose schema with tenant + target compound indexes for keyset pagination, a model factory wired through createModels, and an AuditLog methods factory exposing recordAuditEntry, listAuditLogPage (cursor-paginated, faceted, search-aware), findAuditLogEntry, and streamAuditLogEntries.
The packages/api admin layer adds createAdminAuditLogHandlers with three handlers backing the routes the admin panel already consumes: GET /api/admin/audit-log returns paginated entries, GET /api/admin/audit-log/:id returns a single entry for the permalink drawer, and GET /api/admin/audit-log/export.csv streams CSV with formula-injection defang plus UTF-8 BOM.
The Express layer mounts the new router at /api/admin/audit-log behind requireJwtAuth and the ACCESS_ADMIN capability, matching the existing admin route pattern. The audit emission failure is logged via logger.error but never rolls back the grant.
* 🧹 chore: Audit log backend cleanup — offset pagination, name-based filters, type tightening
Switch listAuditLogPage from cursor-based to offset-based pagination with skip().limit() + parallel countDocuments, returning { entries, total } instead of { entries, nextCursor }; the cursor encode and decode helpers are no longer needed and have been removed.
Interpret the actorId and targetPrincipalId filter parameters as case-insensitive partial regex against the denormalized actorName and targetName fields rather than exact-match against the underlying ObjectId. Admin panel users naturally filter by human name, not by Mongo identifier.
Replace the broad Record<string, unknown> casts on req.query with a typed AuditLogQuery shape, drop two unused exported types and the now-unused mongoose Types import, and fix the streamAuditLogEntries Omit literal to match the interface and the offset-based design.
* 🛠️ fix: Address audit log review feedback (CI typecheck, ISO offsets, no-op revoke, deps surface, schema, backpressure, tests)
Resolve the duplicate AuditAction export that broke the data-schemas TypeScript check by importing the canonical declaration from types/admin instead of re-declaring it in types/auditLog.
Accept timezone-offset ISO 8601 timestamps such as 2026-05-01T09:30:00+02:00 in the from and to filter params and reject local-time strings without a zone so every request resolves to an unambiguous instant.
Skip the audit emission on no-op revokes: revokeCapability now returns deletedCount so the admin handler can omit the grant_removed entry when the target grant did not exist, keeping the audit trail factually accurate. Mocks in the existing grants.spec.ts updated to the new return shape.
Drop the required recordAuditEntry from AdminAuditLogDeps since the audit-log handler factory never consumes it; the grants handler factory keeps its optional dep for the write path.
Tighten the tenantId validator on the audit log schema to require a non-empty trimmed string, and rewrite the listing-index comment to describe deterministic offset sort instead of keyset pagination.
Stream the CSV export with explicit backpressure (await drain when res.write returns false) and abort on client disconnect so a cancelled download no longer pins a Mongo cursor or buffers unbounded data in memory.
Add packages/data-schemas/src/methods/auditLog.spec.ts covering tenant and platform scoping, single and multi action filtering, partial-name filtering for actor and capability, the createdAt window, offset pagination with total, ObjectId and date stringification on the wire, regex-metacharacter escape, and streaming completeness.
* 🛠️ fix: Address P1 audit-log review findings (cursor cancel, drain race, filter naming, type dedupe, tenant scope, log enrichment)
The CSV stream handler kept draining Mongo batches after the client
disconnected because the `for await` loop only honored its abort flag
inside `onEntry`. Thread an `isCancelled` callback into
`streamAuditLogEntries` so the methods layer closes the cursor as soon
as the handler sees `close`/`aborted`; a `finally` block guarantees
release on throw. The drain promise in `writeChunk` now races against
the response's `close` event so a destroyed socket cannot strand the
handler on a `drain` that will never fire.
The HTTP filter keys `actorId` and `targetPrincipalId` always did
case-insensitive substring matches on the denormalized `actorName` /
`targetName` columns, never on ObjectIds — a client passing a real id
silently got zero rows. Renamed the wire-level keys to `actorQuery` /
`targetQuery` (matching what the matcher actually does) and kept the
old names as deprecated aliases for one release so the sibling
admin-panel PR can migrate without breaking; each legacy use logs a
deprecation warning. Renamed the corresponding fields in
`AuditLogFilters` too.
`AdminAuditLogEntryWire` duplicated `AdminAuditLogEntry` from
`types/admin.ts` field-for-field, violating the no-duplicate-types
rule. Deleted the duplicate, hoisted `AuditLogPage`,
`RecordAuditEntryInput`, and `AuditLogFilters` from
`methods/auditLog.ts` into `types/auditLog.ts`, and updated the
handler, method factory, and re-exports accordingly.
`tenantFilter` treated `''` as a valid tenant scope, producing a
`{ tenantId: '' }` query that silently returned nothing while the
schema validator rejected `''` on writes. Switched to a strict
`typeof tenantId === 'string' && tenantId.trim().length > 0` check so
reads agree with writes, with new spec coverage for empty and
whitespace-only inputs.
Audit-write failures now log the full forensic payload (action,
capability, tenantId, actorId, target metadata) inside a single meta
object so winston's standard signature surfaces it correctly; a comment
on the catch block explains why the failure mode stays silent (it must
never block a privileged operation).
Stronger filter parsing: invalid `action` values and unknown
`targetPrincipalType` now return 400 instead of silently dropping.
Extracted `MAX_LIMIT` to a constant. Replaced the
`Record<string, Date>` cast in `buildFilter` with a typed local.
Switched the stream cursor to `lean<IAuditLog[]>()` and removed the
`as IAuditLog` cast inside the loop.
* ✅ test: Cover admin audit-log handler with unit tests for auth, validation, tenant isolation, CSV output, and abort
The sibling admin handlers (grants, groups, roles, users) all have
handler specs; this one was missing. The new suite covers 401 on a
missing `req.user`, 400 on malformed ISO `from` / `to`, 400 on
limit > 500, 400 on negative offset, 400 on an unknown action or
`targetPrincipalType`, 400 on a non-ObjectId `:id`, 404 when the
methods layer returns null, that the caller's `tenantId` (not a
forged query-string `tenantId`) is the one passed to the methods
layer, that `actorQuery` / `targetQuery` round-trip, that the
deprecated `actorId` / `targetPrincipalId` aliases still map through,
that the CSV stream emits the BOM as the first chunk with CRLF line
endings and the expected header labels, that quotes, commas, and
newlines are properly escaped, that the formula-injection prefixes
(`=` `+` `-` `@` tab CR) are defanged, that an `isCancelled` callback
reaches the methods layer and flips to true on client `close`, and
that `res.end` is skipped when the client disconnected mid-stream.
* 🛡️ feat: Enforce append-only AuditLog at the schema level
Every field is now marked `immutable: true`, and pre-hooks on the
schema reject `updateOne`, `updateMany`, `findOneAndUpdate`,
`findOneAndReplace`, `replaceOne`, `deleteOne`, `deleteMany`,
`findOneAndDelete`, plus any `save()` against an existing document.
`timestamps` is reduced to `{ createdAt: true, updatedAt: false }`
since a mutable timestamp would imply mutation is allowed, and
`updatedAt` is dropped from `AuditLog` / `IAuditLog`. The methods
spec resets state between tests via the raw driver (`AuditLog.collection.deleteMany`),
which bypasses the pre-hooks; new specs assert that the model-level
update / delete / re-save paths reject with the append-only error and
that `updatedAt` is not stamped on new documents.
* ♻️ refactor: Share MAX_AUDIT_LOG_LIMIT between methods and handler
Renamed the methods-layer constant from the generic `MAX_LIMIT` to
`MAX_AUDIT_LOG_LIMIT`, exported it through `@librechat/data-schemas`,
and consumed it from the handler instead of duplicating `500` there.
Now the limit is single-sourced; bumping it once updates both the
clamp inside `listAuditLogPage` and the 400-error boundary the
handler returns to clients.
* 🛡️ feat: Gate audit-log routes on a dedicated `READ_AUDIT_LOG` capability
The audit-log routes were gated on `ACCESS_ADMIN`, which conflates "can log
into the admin panel" with "can see who granted what to whom." Anyone with
`ACCESS_ADMIN + READ_CONFIGS` (a config reviewer with no people-management
authority) could read the grant history of every user, group, and role —
information they have no need to know.
`READ_AUDIT_LOG` ('read:audit_log') is now an explicit, separately grantable
read capability with no MANAGE counterpart, matching the append-only nature
of the collection. `seedSystemGrants` iterates `Object.values(SystemCapabilities)`
so existing ADMIN-role seeds pick it up automatically on next startup.
This also makes an "auditor" persona possible: hold `ACCESS_ADMIN + READ_AUDIT_LOG`
without any MANAGE_* grants and you can review history without modifying anything.
* ♻️ refactor: Share AUDIT_ACTIONS, tighten audit dep types, document route order
Exports a runtime AUDIT_ACTIONS array from packages/data-schemas alongside the
AuditAction type so the Mongoose schema enum and the HTTP handler's whitelist
consume one source of truth instead of duplicating the literal pair.
Switches the grants handler's recordAuditEntry dep typing from a duplicated
inline object literal returning Promise<unknown> to the published
RecordAuditEntryInput type returning Promise<void>, and tightens the local
emitAudit args to AuditAction. Replaces the local ParsedFilters interface in
the audit-log handler with Omit<AuditLogFilters, 'offset' | 'limit'> to drop
the duplicate definition.
Drops the optional marker on AuditLog.createdAt. Mongoose always sets it at
insert time, so callers treating it as nullable were guarding against a state
the schema does not produce.
Adds a comment on api/server/routes/admin/audit.js noting that /export.csv
must precede /:id so a future contributor does not accidentally reorder them
into a 404 trap.
* 🛡️ feat: Resolve audit names without extra DB round-trips
For the actor name, JWT-authenticated `req.user` already carries `name`,
`username`, and `email`. `resolveUser` now derives the actor display name
from `req.user` directly and threads it through the caller context, so
every grant assign and revoke no longer triggers a separate `getUserById`
lookup.
For the target name, replaces the previous always-store-the-principalId
behavior (which buried opaque ObjectId strings in immutable audit rows
for USER and GROUP targets) with a `resolveTargetName` dep. ROLE
principals continue to use `principalId` directly because the SystemGrant
model stores role names there. USER and GROUP principals route through
the new dep, which in `api/server/routes/admin/grants.js` calls
`db.getUserById` or `db.findGroupById` respectively and falls back to
the principalId on miss or error so the audit row stays intelligible.
Drops the misleading "display name lookup happens in a later iteration"
comment.
* ✅ test: Cover audit emission, scope emitAudit to today's ROLE-only surface
Fixes a misleading test that claimed to verify "idempotent even if the grant
does not exist" while mocking deletedCount: 1 (the grant DID exist). Replaces
it with the actual no-op scenario (deletedCount: 0) and adds an assertion
that recordAuditEntry is NOT called, since the whole point of the
deletedCount > 0 gate is to avoid fictitious revocation rows.
Adds a dedicated audit emission describe block covering: grant_assigned
emission with the actor name resolved from req.user, grant_removed
emission when deletedCount is positive, and the no-emission fallback when
recordAuditEntry is not configured. The actor-name assertions exercise the
name / username / email fallback chain in resolveUser.
The previous commit also added a `resolveTargetName` dep and an
emitAudit branch for USER/GROUP targets. The grants surface is ROLE-only
today (MANAGE_CAPABILITY_BY_TYPE has only PrincipalType.ROLE), so that
code path is unreachable from the handler. Removed the dep and the
branch; the audit row uses principalId as the target name, which is the
human-readable role name for ROLE principals. A comment in emitAudit
flags where to plumb resolveTargetName back in once USER and GROUP
grants are enabled.
* 🛠️ fix: Inclusive `to` date filter and reject inverted ranges
A `?to=2025-01-15` filter previously stopped at midnight UTC of that
day, silently excluding everything that happened on January 15. The
`parseIsoDate` helper now widens a bare `YYYY-MM-DD` to 23:59:59.999Z
when called with the `end` boundary. Full ISO timestamps are honored
exactly, so callers that want minute-precision can still get it.
Also rejects inverted ranges (`from` later than `to`) with a 400 so
operators see a clear error instead of a silent empty result.
* 🛡️ feat: Cap audit-log CSV exports at 100k rows; cover stream error path
Introduces MAX_AUDIT_EXPORT_ROWS (100k) and threads a `maxRows` option
through streamAuditLogEntries. The handler now passes the cap into the
stream so a careless admin script or a hostile auditor cannot pin a
Node worker and a Mongo cursor by exporting unbounded result sets.
Beyond 100k rows, callers should slice exports by from / to date.
Adds a methods-layer spec for the cap behavior, a handler-layer spec
that asserts the option is plumbed through, and a handler-layer spec
that exercises the streamAuditLogEntries-throws-after-headers-sent path
(catch block falls through to res.end instead of attempting JSON).
Documents on buildFilter that case-insensitive substring regex filters
(actorName, targetName, capability, search) cannot use a B-tree index
and degrade to a tenant-scoped partition scan, so deployments with
hundreds of thousands of audit rows per tenant should constrain those
queries with a date window.
* 🧹 chore: Spell CSV_BOM as and drop a gratuitous optional chain
`revokeCapability` is typed `Promise<{ deletedCount: number }>` so the
`?.` on `revokeResult?.deletedCount` only obscured that the value cannot
be nullish.
`CSV_BOM` was a literal U+FEFF character invisible in most editors. Now
spelled as the Unicode escape so readers can see the constant; the test
that asserts on the first emitted chunk uses the same escape.
* 🔧 chore: Allowlist AuditLog in the tenant-isolation coverage guard
The AuditLog collection carries a tenantId field but scopes tenancy manually
inside listAuditLogPage / streamAuditLogEntries / recordAuditEntry using the
same $exists: false convention as SystemGrant. The tenant-isolation plugin
coverage spec now allows that and asserts it stays accurate.
* 🛠️ fix: Normalize blank tenantId before persisting audit entries
The `recordAuditEntry` write path was treating any non-null tenantId as a
real string, so empty or whitespace-only values reached the schema validator,
failed the non-empty-string check, and silently dropped the audit row. The
read-side `tenantFilter` already treats those values as platform-level scope,
so the write path now mirrors it: blank or whitespace-only tenantId becomes
an omitted field, which matches `{ tenantId: { $exists: false } }` queries
and clears validation. Added a regression test that records two entries with
blank and whitespace tenantId and asserts both persist with the tenantId
field absent.
* 🎨 style: collapse expect.objectContaining onto one line to satisfy prettier
* 🔒 fix: block document-level deleteOne/updateOne on AuditLog
Mongoose registers deleteOne and updateOne pre-hooks as query middleware
by default. The query-level append-only block on AuditLog therefore did
not cover Document.prototype.deleteOne() or Document.prototype.updateOne(),
leaving a path where a caller that had already loaded an audit row via
findOne could call .deleteOne() or .updateOne() on the instance and bypass
the schema contract.
Explicit { document: true, query: false } registrations close the holes,
and the spec now covers both code paths against a real in-memory Mongo.
* 🔒 fix: require ACCESS_ADMIN on audit-log routes
Every other admin router (config, grants, users, roles, groups, auth)
enforces requireJwtAuth followed by requireCapability(ACCESS_ADMIN) before
any feature-specific capability check. The audit-log router only required
READ_AUDIT_LOG, which is independent of ACCESS_ADMIN in CapabilityImplications,
so a role delegated only READ_AUDIT_LOG without ACCESS_ADMIN could read or
CSV-export the audit trail and bypass the admin boundary.
Aligned the middleware chain with the rest of the admin surface so
ACCESS_ADMIN gates entry and READ_AUDIT_LOG gates the feature within it.
* 🎨 chore: re-sort imports after dev rebase
Post-rebase sort-imports against the merge target — six audit-log files
landed with stale import ordering relative to the current scripts/sort-imports.mts
rules on dev. CI's import-order job flagged the drift; running the script
locally rewrites them in place. No semantic changes.
* 🔧 fix: explicit type annotations on audit-log model + schema exports
Dev migrated packages/data-schemas builds from rollup to tsdown with
--isolatedDeclarations enabled, which requires every exported function to
declare its return type and every exported variable to declare its type.
Two of our audit-log exports got swept up:
TS9007 models/auditLog.ts:12 createAuditLogModel return type
TS9010 schema/auditLog.ts:12 auditLogSchema variable type
Added Model<t.IAuditLog> on the factory and Schema<IAuditLog> on the
schema variable, matching the sibling SystemGrant convention. No runtime
behavior change.
* 🔧 fix: align revokeCapability type annotation with implementation
The rebase auto-merge of systemGrant.ts kept dev's outer type annotation
(`revokeCapability: ... => Promise<void>`) but our implementation returns
`Promise<{ deletedCount: number }>` (added during the bot-review loop to
let the audit emitter distinguish a real revoke from a no-op against a
nonexistent grant). The mismatch surfaced as TS2719 on the methods record
return at line 520. Updated the type annotation to match the impl.
The caller at packages/api/src/admin/grants.ts:444 reads
`revokeResult.deletedCount` to gate the audit emit, so the wider return
type is what the rest of the code already assumes.
* 🔧 fix: explicit factory return type on createAdminAuditLogHandlers
Same tsdown --isolatedDeclarations migration that hit packages/data-schemas
also applies to packages/api; the audit-log handler factory's inferred
return type tripped TS9013 against the new build pipeline. Annotated the
factory with explicit handler signatures matching the sibling
createAdminGrantsHandlers convention. Used Promise<Response | void> for
the export handler because its final res.end() path returns undefined,
unlike the other two handlers which always return a Response.
* 🛡️ feat: Generalize audit log into a tamper-evident, extensible event substrate
Reworks the SystemGrant-only audit log into a general-purpose, append-only
compliance substrate designed to absorb future event classes (agent runs,
tool/MCP calls, config + permission changes, approvals) without reshaping the
record. Nothing was shipped yet, so this replaces the grant-specific wire
shape rather than layering aliases.
Schema / record shape (packages/data-schemas):
- schemaVersion + two-level taxonomy: category + namespaced action
(grant.assigned/grant.removed), first-class outcome and severity.
- Structured actor{type,id,name} supporting non-user actors (system, agent,
service, schedule, webhook, api); generic target{type,id,name}; open
metadata map; request context{requestId,ip,userAgent,sessionId}.
Tamper-evidence (hash chain):
- Per-tenant chain keyed by chainKey with seq/prevHash/hash. Appends link to
the previous hash; a unique {chainKey,seq} index serializes concurrent
writes (dup-key retry) so the chain can never fork. createdAt is explicit so
it's covered by the hash.
- verifyAuditChain() walks a chain and detects modification, deletion, and
forged links; exposed via GET /api/admin/audit-log/verify.
Other best-practice gaps from the review:
- Keyset (cursor) pagination over seq alongside offset; stable under
concurrent appends. nextCursor in the page payload.
- Retention: purgeAuditLogEntries() privileged prefix-purge with a confirm
latch, returns a checkpoint; verify tolerates a purged prefix.
- Fail-closed option (AUDIT_LOG_FAIL_CLOSED) so a failed audit write can fail
the grant request instead of being swallowed; default stays fail-open.
- Grant handlers now capture request context and emit the new shape.
CSV export updated for the new columns (incl. seq/hash). data-schemas bumped
to 0.0.54 for the sibling admin-panel consumer. Tests rewritten: 28
methods-layer cases (chain genesis/linking, tamper detection, keyset, purge)
and the handler/grants specs updated for the new shape, fail-closed, and the
verify endpoint.
* 🛠️ fix: Address Codex review on the audit-log substrate
- F1 (fail-closed atomicity): assign/revoke now compensate (rollback grant /
restore grant) when a fail-closed audit write fails, so a 5xx never leaves an
unaudited mutation.
- F5: only emit grant.assigned for a real change — skip the audit when the role
already holds the capability (idempotent re-assert).
- F7: verifyAuditChain no longer silently trusts a non-genesis start; a purged
prefix must be authorized by a trusted checkpoint (purge now returns
{throughSeq, prevHash}), else verification fails as tampering.
- F4: block Model.bulkWrite on AuditLog (would bypass the append-only middleware).
- F3: CSV export appends an explicit TRUNCATED marker + logs when the row cap is hit.
- F6: reject out-of-range date-only filters (2025-02-31) instead of normalizing.
- F2: regenerate package-lock.json for the 0.0.54 data-schemas bump.
Tests: +1 methods (bulkWrite) +2 verify (deleted-prefix / checkpoint mismatch),
updated purge test for checkpoint flow; +4 api (re-assert skip, assign/revoke
fail-closed rollback, date reject, CSV truncation marker).
* 🛠️ fix: Address Codex round-2 on the audit-log substrate
- R2-1/R2-5 (P1/P2): base the grant.assigned audit decision on the atomic
upsert result. grantCapability now returns { grant, created } via
includeResultMetadata; the handler audits only when created. Removes the racy
pre-read, which also mis-handled inherited platform grants vs a new
tenant-scoped insert and concurrent double-assign.
- R2-2 (P2): namespace tenant chain keys (tenant:<id>) so a tenant whose id is
literally the platform sentinel can't share the platform audit chain.
- R2-4 (P2): validate literal calendar tokens for full ISO timestamps too, so
2025-02-31T00:00:00Z is rejected instead of normalizing to March 3.
Tests updated for the grantCapability { grant, created } contract (systemGrant +
grants specs) and the namespaced chain key (auditChainKey helper); +1 api date
case. data-schemas 141, api grants/audit 107 green.
R2-3 (deprecated actorId/targetPrincipalId aliases): not reinstating — the
surface is pre-release and its only consumer (admin-panel PR) migrates to the new
shape in lockstep, so there are no legacy clients to support.
R2-6 (role-deletion cascade emits no grant.removed): valid but a separate
workflow in roles.ts; tracked as a follow-up to keep this PR scoped.
* 🛠️ fix: Address Codex round-3 on the audit-log substrate
- R3-3 (P2): make a grant re-assert a true no-op — move grantedAt/grantedBy to
$setOnInsert so an existing grant is never silently mutated when the audit is
skipped (created:false now means nothing changed). grantedAt/grantedBy record
the original grant.
- R3-2 (P2): report CSV export truncation exactly. streamAuditLogEntries returns
{ count, truncated }; truncated is true only when rows existed beyond the cap,
so an exact-cap export is no longer falsely marked truncated.
- R3-5 (P2): block AuditLog.insertMany (another bulk path that skips the save
hook and could inject forged seq/prevHash/hash and poison the chain).
Tests: +insertMany rejection, +exact-cap vs truncated stream cases, +exact-cap
export-not-truncated handler case. ds 142, api 108 green.
R3-1 (deprecated query aliases) and R3-4 (role-deletion cascade audit) are
re-flags of R2-3/R2-6 — holding the prior decisions (pre-release surface; separate
roles.ts workflow tracked as a follow-up), pending maintainer direction.
* 🛡️ feat: Audit grant removals from the role-deletion cascade
Closes the forensic gap Codex flagged (R2-6/R3-4): deleting a role removed its
SystemGrants with no audit entries. `deleteGrantsForPrincipal` now returns the
removed grants, and the role-deletion handler emits a `grant.removed` audit entry
per removed grant (actor = caller, target = role, metadata.capability, request
context), matching the explicit revoke endpoint. Fail-open — the role is already
deleted, so a failed audit is logged, not propagated; sequential to keep the
per-tenant hash chain ordered.
Extracted `buildAuditContext` to admin/context.ts (shared by grants + roles).
Tests: role-deletion emits one entry per grant / none when no grants; ds 110,
api admin 202 green.
* 🛠️ fix: Address Codex round-4 on the audit-log substrate
- R4-1 (P2): don't silently drop an audit row under heavy append contention.
recordAuditEntry now retries duplicate-key seq collisions up to 12× with
jittered backoff (was 5, no backoff), so realistic bursts of parallel admin
writes resolve; the failClosed escape still applies on true exhaustion.
- R4-3 (P2): purge a contiguous seq prefix, not a date range. createdAt is
app-generated, so under multi-instance clock skew a later seq can carry an
earlier timestamp; a raw date delete could remove an interior row and break
verification. purgeAuditLogEntries now resolves the date to the first retained
seq and deletes only strictly-lower seqs, keeping the remaining chain contiguous.
Tests: +clock-skew purge case (no gap created). ds auditLog 33 green.
R4-2 (role-deletion grant audit) is a re-flag of R2-6/R3-4, already implemented
in
|
||
|
Danny Avila
|
788cc5ac07
|
🛟 fix: Auto-Recover from Stale Service Worker Assets After Deploys (#13686)
Some checks are pending
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
* 🛟 fix: Auto-Recover from Stale Service Worker Assets After Deploys - 404 missing static assets in the SPA fallback instead of serving index.html - inline recovery script unregisters stale SWs and reloads once on chunk failure - route vite:preloadError into the same recovery path for stale lazy chunks * 🛟 fix: Address Review — SW-Side Recovery, Scoped Unregister, Shared Fallback - importScripts'd sw-heal.js pings window clients on activation and reloads ones that can't pong: stale pages carry no recovery code of their own - scope SW unregistration to the deployment base for subpath installs - preventDefault vite:preloadError only when a recovery reload was initiated - extract createSpaFallback and apply the asset 404 guard to experimental.js |
||
|
Danny Avila
|
197a1dc4e2
|
🧬 feat: Add GitHub Skill Sync (#13293)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
Publish `librechat-data-provider` to NPM / pack (push) Has been cancelled
Publish `librechat-data-provider` to NPM / publish-npm (push) Has been cancelled
* feat: Add GitHub skill sync
* fix: Address GitHub skill sync CI
* fix: Harden GitHub skill sync review paths
* fix: Prevent overlapping skill sync runs
* fix: Address GitHub skill sync review findings
* fix: Satisfy Git ref lint rule
* fix: Address GitHub sync review follow-ups
* fix: Match skill frontmatter closing fence
* fix: Address GitHub sync review cycle
* fix: Address GitHub sync review follow-ups
* fix: Harden GitHub skill sync worker
* fix: Format GitHub sync rollback log
* fix: Address GitHub sync review feedback
* fix: Format skill import parse handling
* fix: Coerce scalar skill frontmatter and correct scheduler timer clear
- parse: coerce numeric/boolean name and description scalars to strings instead of dropping them to empty (restores pre-refactor behavior; preserves absent-vs-empty distinction for the when-to-use fallback)
- scheduler: clear the setTimeout handle with clearTimeout rather than clearInterval
- test: cover non-string scalar frontmatter coercion
* fix: Tolerate trailing whitespace after SKILL.md opening frontmatter fence
extractFrontmatterBlock required the opening fence to be exactly '---\n', so an opener with trailing spaces/tabs (e.g. '--- \n') silently dropped all frontmatter even though the closing-fence regex already tolerates it. Match the opener with /^---[ \t]*\n/ for symmetry. Addresses Codex P3 (parse.ts:24).
* feat: Run GitHub skill sync under a per-source tenant context
Under TENANT_ISOLATION_STRICT, the sync ran with no async tenant context, so the tenant-isolation mongoose hooks threw on every Skill/SkillFile/AclEntry operation; in non-strict mode synced skills were written tenant-less and never matched tenant-scoped reads. Add an optional per-source tenantId to the skillSync config; when set, each source sync runs inside tenantStorage.run({ tenantId }) so skills, files, and public ACL grants are created and listed within that tenant, and the skill row is stamped with the tenantId for correct dedup. Sources without tenantId keep the prior single-tenant behavior. Avoids runAsSystem. Addresses Codex P2 (sync.js:70).
Lock/status/credential bookkeeping stays outside the tenant context (those collections are intentionally global).
* test: Restore dropped tenant-context coverage for GitHub skill sync
The prior commit shipped the getTenantId import in github.spec.ts without the tenant tests that use it (lost in an interrupted edit), which failed the eslint --max-warnings=0 CI job on an unused import. Restore both github.spec.ts tenant tests (tenant-scoped run stamps tenantId and executes inside the tenant ALS context; no-tenant run stays ambient) and the two config-schemas tenant tests (accepts tenantId, rejects __SYSTEM__).
* test: Restore dropped github.spec tenant-context tests
The previous commit's github.spec.ts edit did not apply (anchor mismatch), so the getTenantId import remained unused and failed eslint --max-warnings=0. Add the two tenant tests that use it: a tenant-scoped run stamps tenantId and executes inside the tenant ALS context, and a no-tenant run stays ambient.
* feat: Scope synced skill author to tenant and harden tenant-context sync
Addresses the latest Codex review on the per-source tenant change:
- makeSourceAuthorId now folds tenantId into the synthetic author hash so the
same source mirrored into different tenants gets distinct author ids (clearer
audits, no cross-tenant author collisions). Single-tenant author ids stay
stable (suffix omitted when tenantId is absent).
- syncSourceInTenantContext uses an async callback per the tenant-context
contract so the ALS store propagates across awaited Mongoose calls.
- Tests: same-source/different-tenant yields distinct authors; mirror cleanup
is scoped to the source and deletes only its absent-upstream skills.
* fix: Repair tsc error and guard external edits in github skill sync
- Fix TS2352 in github.spec mirror-cleanup test: build the existing-skill mock via makeSkill with authorName instead of an under-typed 'as CreateSkillInput' cast (this was the failing TypeScript CI check on f00ce3c5a).
- 808: commitExistingRemoteSkillAfterFileSync re-reads to clear our own file-sync version bumps, but now compares refreshed content against the pre-sync snapshot (body/name/description/always-apply) and throws SKILL_CONFLICT on a concurrent external edit instead of overwriting it.
* docs: Note skillSync source tenantId is effectively immutable
Changing/adding/removing a source's tenantId orphans previously mirrored skills in the old tenant (a tenant-scoped sync cannot clean another tenant's data without runAsSystem, which is intentionally avoided).
* fix: Key GitHub skill upstream identity on source id and path only
Addresses Codex finding (github.ts:217): makeUpstreamId previously included owner/repo, so repointing a source to a renamed or replacement repository (same source id) changed the upstreamId, made findSkillBySourceIdentity miss the existing mirror, and then collided on the (name, author, tenantId) uniqueness constraint — leaving the source stuck failing. Identity now keys on the stable source id + root path only. The feature is unreleased, so there is no stored-id migration. Updated spec upstreamId fixtures to the new format; the existing ref-independent identity test now also covers repo moves.
* fix: Scope GitHub skill mirror deletion to the source tenant
Addresses Codex P1 (github.ts:1047/1057): an ambient source (no tenantId) runs listSkillsBySource without tenant context, which under non-strict isolation returns github-synced skills across all tenants. The mirror-deletion pass then treated other tenants' skills as absent-upstream and could delete them. Filter existingSyncedSkills to rows whose tenantId matches the source's configured tenantId (absent = its own ambient bucket) before deleting, so a sync never removes another tenant's mirrored skills. Covered by a test where an ambient run leaves a tenant-b-owned skill untouched.
* fix: Apply tenant-scoped mirror deletion implementation
The prior commit (75ccfa3fc) added the test but the source change to github.ts was lost in an interrupted edit, leaving a failing test with no implementation. This adds the actual guard: the mirror-deletion pass skips skills whose tenantId does not match the source's configured tenantId (absent = ambient bucket), so an ambient source whose listSkillsBySource returns cross-tenant rows under non-strict isolation cannot delete another tenant's mirrored skills.
* fix: Resolve global access role outside tenant context for synced skill grants
Addresses Codex P2 (github.ts:1166): default access roles (incl. skill_viewer) are seeded globally with no tenantId under runAsSystem, but a tenant-scoped sync wraps ensurePublicViewer in the source's tenant context. The PermissionService grantPermission resolved the role via a tenant-isolated AccessRole query, so the global role did not match and tenant-scoped syncs failed with 'Role skill_viewer not found'. The sync adapter now resolves the role inside runAsSystem (matching the global seed) and writes the ACL entry in the active tenant context, so the AclEntry is tenant-scoped (visible to tenant users) while the role lookup still succeeds. Covered by service tests for the resolve-vs-write split and the missing-role failure.
* fix: Strip placeholder frontmatter booleans and check skill conflict before file sync
- 1083 (github.ts:759): toCleanFrontmatter now drops a non-boolean always-apply (e.g. the 'always-apply:' / 'always-apply: # TODO' placeholder, which js-yaml yields as null). The boolean is already captured in the dedicated alwaysApply field; persisting null left ambiguous frontmatter on the synced skill.
- 1080 (github.ts:1057): for an existing mirrored skill, check for an external content edit (via getSkillById + hasExternalSkillEdit) BEFORE syncSkillFiles mutates the bundled files, so a concurrently edited skill fails fast with SKILL_CONFLICT without partial file rewrites. The post-file-sync check still guards edits that land during the file sync window.
Tests: placeholder always-apply is dropped from synced frontmatter; concurrent-edit conflict leaves files unmutated (no upsert/delete).
* fix: Harden GitHub skill sync review paths
* fix: Reuse moved GitHub skill mirrors
* fix: Scope GitHub sync identity conflicts
* test: Fix GitHub sync conflict mock typing
* fix: Support nested env-backed skill sync
* fix: Keep skill sync config base-only
* fix: Scope GitHub skill identity lookup by tenant
* fix: Harden GitHub skill sync admin gates
* fix: Guard existing skill sync permission grants
* feat: Trigger skill sync from resolved config
* fix: Scope resolved skill sync by tenant
* test: Allow manual skill sync status tenant scoping
* refactor: Extract skill sync trigger orchestrator
* test: Complete orchestrator status fixture
* chore: Bump data provider version
* fix: Restrict skill sync server credentials
* test: Complete admin skill sync status fixtures
* fix: tighten skill sync trigger safeguards
* fix: preserve alwaysApply skill sync alias
* chore: sort skill sync imports
* fix: preserve skill sync request scope
* fix: harden skill sync review edges
* refactor: move skill sync admin access to api package
* fix: add skill sync declaration return types
* fix: satisfy skill sync type checks
* fix: resolve codex skill sync review findings
* fix: harden skill sync review edges
* fix: resolve codex skill sync edge findings
* fix: satisfy API declaration build after rebase
|
||
|
Ravi Kumar L
|
865e1da857
|
⚙️ refactor: lazy-load React Query Devtools (#13639)
* perf(client): lazy-load query devtools * fix: keep query devtools deps lazy * fix: address query devtools review findings * fix: exclude query devtools from pwa precache --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Danny Avila
|
2c8d54e18c
|
🗂️ feat: Add Deployment Skill Directory (#13523)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* feat: Add deployment skill directory * chore: Address deployment skill review feedback * fix: Include deployment skill file metadata * test: Add deployment skills e2e smoke test |
||
|
Danny Avila
|
15072467b1
|
🚦 fix: Gate Chat Starts During Readiness (#13502)
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
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
* fix: guard chat starts during server readiness * style: format readiness retry condition * fix: clarify chat start retry diagnostics * fix: cancel stale chat start retries * style: use const for retry timeout |
||
|
Danny Avila
|
baa23a8e24
|
🗂️ feat: Add Private Chat Projects (#13467)
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
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
* feat: Add private chat projects
* fix: Format project files
* fix: Address project review findings
* fix: Resolve project review follow-ups
* fix: Handle project stats and cache edge cases
* style: align projects UI with sidebar patterns
* fix: resolve projects UI lint issues
* style: Align project menus and composer
* fix: Avoid project placeholder shadowing
* fix: Handle project search and stale ids
* fix: Polish project sidebar behavior
* fix: Preserve new chat stream after creation
* fix: Stabilize project sidebar sections
* fix: Smooth project sidebar organization
* fix: stabilize project chat entry
* fix: keep project workspace outside chat context
* fix: show default model on project workspace
* fix: fallback project workspace model label
* fix: preserve project scope during draft hydration
* fix: include route project in new chat submission
* fix: persist project id in agent chat saves
* fix: refine project sidebar and creation UX
* fix: export chat project method types
* fix: polish project landing context
* fix: refine project navigation affordances
* feat: rework projects UX — coexisting sidebar sections + URL-driven scope
Sidebar
- Replace the chronological/by-project mode toggle with coexisting
Projects + Chats sections (both always visible)
- Remove ProjectConversations (927 lines), the org-mode Header, and types
- Add ProjectsSection: collapsible project rows that unfurl chats inline
(full-size rows), with per-project new chat and an open/rename/delete menu
- Lift the marketplace/favorites shortcuts above the Projects section
Chat scope
- Derive a new chat's project strictly from the URL ?projectId, so the
global New Chat no longer stays stuck in a project after a project chat
Surfaces
- Chat landing: subtle, clickable project chip instead of the floating badge
- Project workspace: modest header, composer-style entry, chats list
- All-projects grid: Claude-style cards with pluralized chat counts
* chore: prune unused i18n keys; fix project chat-count pluralization
* fix: project new-chat keeps model spec; sidebar header + row polish
- newConversation: ignore a chatProjectId-only template when deciding to
apply the default model spec, so starting a chat in a project no longer
strips the conversation `spec`
- useSelectMention: the Model Selector and @ command now retain the active
project across endpoint/spec/preset switches; other new-chat paths still
clear it
- Chats header now matches the Projects header (inline chevron + a new-chat
icon button) and starts a non-project chat
- Project rows: use the new-chat icon for the per-project add button, render
at text-sm to match the chat list, and align the row actions + hover color
with conversation rows
* fix: read project scope from router params; align sidebar header icons
- useSelectMention now reads the active project from React Router's search
params instead of window.location, which can drift out of sync because
new-chat params are written to the URL via raw history.pushState; the
Model Selector and @ command now reliably keep the project on switch
- Move the Chats section header out of the virtualized list so it renders
in the same context as the Projects header and isn't shifted by the
list scrollbar
- Inset header action icons (pr-2) so Projects/Chats header icons line up
with the project-row and conversation-row trailing actions
- Extract getRouteChatProjectId into utils for the submit path
* fix: preserve chatProjectId through the new-chat template reduction
The param-endpoint guard in newConversation reduced a new chat's template to
{ endpoint } only, dropping the chatProjectId injected by the Model Selector /
@ switch — so switching models cleared the project scope. Keep chatProjectId
in the reduced template.
* style: align chat-history panel top padding; improve projects page contrast
- Add pt-2 to the chat-history panel so its top spacing matches the other
side panels (agent builder, skills, files, etc.)
- Projects grid + workspace now use the darkest surface for the page
(surface-primary) with cards, inputs, and the composer one step lighter
(surface-secondary) and tertiary on hover, so cards read as elevated
rather than darker than the background
* feat: interactive project landing chip + gallery icon for all-projects
- All-projects sidebar button uses the gallery-vertical-end icon
- The project landing chip is now interactive: click it to switch projects
via a searchable combobox (ControlCombobox), or the trailing × to drop the
project scope. Both update the draft conversation and the ?projectId search
param in place, so the typed message and selected model are preserved
* test: fix Conversations unit test for refactored sidebar; add projects e2e
- Update Conversations.test.tsx mocks for the inline Chats header
(useNewConvo, useQueryClient, conversation atom, NewChatIcon, TooltipAnchor),
drop the removed chatsHeaderControls prop, and remove the mock for the
deleted ../Header module — fixes the failing frontend Jest job
- Add e2e/specs/mock/projects.spec.ts covering project creation, the
project-scoped new-chat landing + interactive chip (switch/remove), and
listing projects on /projects
- Give the landing chip combobox a stable selectId for reliable targeting
* fix: refresh project stats after project-chat activity; stabilize e2e
- useEventHandlers: when a project chat is created/updated, invalidate the
live [projects] query (gated on chatProjectId) instead of the now-unused
projectConversations key, so the sidebar + all-projects stats refresh
after a streamed reply (addresses a Codex finding)
- projects e2e: assert the reliable project-landing behavior (chip, scoped
composer, accepted send) rather than the /c/:id transition, which the
mock LLM harness doesn't complete
* test: verify a project chat saves and is filed under its project (e2e)
- Switch to a mock endpoint before sending so the message streams without a
real API key (the default model failed with "No key found", so no chat was
saved and the page never left /c/new); this also asserts the project chip
survives the model switch
- Restore the reply + /c/:id transition assertions and add a check that the
chat is listed under the expanded project in the sidebar
- Add data-testid="project-chats-<id>" to the inline project chat list
* fix: address Codex review findings (project scope edge cases)
- useSelectMention: fall back to the conversation's chatProjectId when the
URL has no projectId, so switching model/spec inside an existing project
chat (/c/:id) keeps the project assignment
- Conversations: include chatProjectId in the MemoizedConvo comparator so a
sidebar row's project menu doesn't stay stale after a reassignment
- useDeleteProjectMutation: clear the active conversation's chatProjectId
when its project is deleted (mirrors the assignment mutation); drop the
now-dead projectConversations invalidation
- useQueryParams: carry the project into the new conversation when applying
URL settings, so /c/new?projectId=...&<settings> stays scoped
* fix: project stats pagination + archived-chat edge cases (data-schemas)
- listChatProjects: include the null lastConversationAt bucket in the desc
cursor so empty projects paginate (a $lt:<date> predicate excluded nulls,
hiding chat-less projects from "Load more")
- saveConvo: recompute project stats instead of the incremental fast path
when the saved conversation is itself archived/temporary/expired, so a
project's lastConversationAt/Id no longer points at a hidden chat
* test: cover chat-less project pagination across the dated→null boundary
* fix: validate project ownership in bulkSaveConvos
Bulk paths (import/duplicate/fork) persisted whatever chatProjectId the
payload carried; an id that does not belong to the user created an orphan
assignment hidden from both the project and the unassigned sidebar. Validate
ownership like saveConvo and strip un-owned project ids before persisting,
refreshing stats only for owned projects.
* fix(projects): preserve chatProjectId on continuation, basename-safe delete redirect, project-detail invalidation
* fix(projects): navigate project workspace chats via useNavigateToConvo to avoid stale conversation state
* fix(projects): include projectConversations cache when resolving deleted chat's project for detail invalidation
* fix(projects): refresh both projects when a save or bulk write moves a chat between them
* style(projects): use Folders icon for the sidebar Projects header
* fix(projects): require id on ProjectUser so ProjectRequest extends Express Request cleanly
* style(projects): taller project chip with hover-revealed remove button, upward combobox; sort en translations
* style(projects): show endpoint/agent icon for project workspace chat rows
|
||
Teresa Blanco
|
b45e4aeae5
|
🎭 feat: Add Credential-Free Playwright Smoke Suite with a Local Mock LLM (#13472)
* 🧪 feat: add e2e playwright tests * 🧪 feat: Add Playwright Recording Harness * test: fix mock playwright config * test: harden mock e2e environment * test: preserve mock dotenv secrets * test: harden mock isolation setup * ci: cache mock e2e builds * test: harden e2e cache and recorder checks * test: preserve data-provider exports in oauth route test * test: isolate mock auth logout state * test: allow isolated logout smoke setup * test: prepare logout smoke auth via api * test: isolate oauth route module mock --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Ravi Kumar L
|
a86e504a57
|
📡 feat: Add Authenticated Proxy Mode for Browser RUM Telemetry (#13464) | ||
|
Danny Avila
|
9cb650d1d8
|
🩺 feat: Add Explicit Readiness Endpoints (#13212) | ||
Pete Hampton
|
679672ad15
|
🪂 feat: Graceful HTTP shutdown on SIGTERM/SIGINT (#13211)
* 🪂 feat: Graceful HTTP shutdown on SIGTERM/SIGINT
* Address feedback
* don't treat ERR_SERVER_NOT_RUNNING as fatal; route telemetry shutdown through coordinator
|
||
|
Danny Avila
|
9dd062e42e
|
🧯 fix: Harden Data Retention Semantics (#13049)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* feat: support data retention for normal chats Add retentionMode config variable supporting "all" and "temporary" values. When "all" is set, data retention applies to all chats, not just temporary ones. Adds isTemporary field to conversations for proper filtering. Adapted to new TS method files in packages/data-schemas since upstream moved models out of api/models/. Based on danny-avila/LibreChat#10532 Co-Authored-By: WhammyLeaf <233105313+WhammyLeaf@users.noreply.github.com> (cherry picked from commit |
||
|
Danny Avila
|
050b7fd43a
|
📡 feat: Add Backend OpenTelemetry Tracing (#12909)
* feat: add backend OpenTelemetry tracing * fix: address telemetry type checks * fix: mark aborted telemetry requests as errors * fix: record telemetry identity after auth * fix: avoid forced telemetry signal exit * fix: harden telemetry request attribution * fix: record telemetry errors on request span * chore: order imports and reorganize middleware usage * fix: reduce telemetry startup overhead * fix: preserve live telemetry controller state * fix: redact telemetry URL attributes |
||
|
Danny Avila
|
34dd8d5f2a
|
📈 feat: Add Prometheus Metrics Endpoint + AWS Credential Providers (#13111)
* feat: add prometheus metrics endpoint * fix: format metrics route spec * chore: update dependencies in package.json and package-lock.json correctly - Bump `@smithy/core` to version 3.24.1 - Update `@aws-sdk/credential-providers` to version 3.1045.0 - Reintroduce `prom-client` dependency in package.json - Remove unnecessary dependencies from package.json * chore: import order * fix: declare s3 presigner peer dependency * fix: normalize shared link metrics path * fix: bound metrics path labels * fix: tighten metrics auth and peers * fix: collapse partial metrics paths |
||
|
Danny Avila
|
6c6c72def7
|
🚀 feat: Decouple File Attachment Persistence from Preview Rendering (#12957)
* 🗂️ feat: add `status` lifecycle to file records for two-phase previews
Schema and model foundation for decoupling the agent's final response
from CPU-heavy office-format HTML extraction.
- `MongoFile.status: 'pending' | 'ready' | 'failed'` (indexed) and
`previewError?: string` mirror the lifecycle: phase-1 emits the file
record at `pending` so the response is unblocked; phase-2 transitions
to `ready` (with text/textFormat) or `failed` (with previewError) in
the background. Absent for legacy records — clients treat that as
`ready` for back-compat.
- Mirror types added to `TFile` in data-provider so frontend cache
consumers see the new fields.
- New `sweepOrphanedPreviews(maxAgeMs)` method on the file model
recovers stale `pending` records left behind by a process restart
mid-extraction; transitions them to `failed` with
`previewError: 'orphaned'`. Cheap because `status` is indexed.
* ⚡ feat: two-phase code-execution preview flow (unblocks final response)
The agent's final response no longer waits on CPU-heavy office HTML
extraction. Phase-1 (download + storage save + DB record at
`status: 'pending'`) is awaited as before; phase-2 (extract +
`updateFile`) runs in the background with a hard 60s ceiling.
Three flows, all funneling through `processCodeOutput` and updated to
the new `{ file, finalize? }` return shape:
- `callbacks.js` (chat-completions + Open Responses streaming): emit
the phase-1 attachment immediately (carries `status: 'pending'` for
office buckets so the UI shows "preparing preview…"), then
fire-and-forget `finalize()`. If the SSE stream is still open when
phase-2 lands, push an `attachment` update event with the same
`file_id` so the client merges over the placeholder in place.
- `tools.js` direct endpoint: same split — return the phase-1
metadata immediately, run extraction in the background. Client
polls for the resolved record.
`finalize()` wraps the existing 12s per-render timeout in a 60s outer
`withTimeout`. The HTML-or-null contract from #12934 is preserved:
office types that fail extraction transition to `status: 'failed'`
with `previewError: 'parser-error' | 'timeout'` rather than falling
back to plain text (would be an XSS vector).
Promises continue running after the HTTP response closes (Node
doesn't kill them). The boot-time orphan sweep covers the only case
that loses progress — actual process restart mid-extraction.
`primeFiles` annotates the agent's `toolContext` line for prior-turn
files: `(preview not yet generated)` for pending, `(preview
unavailable: <reason>)` for failed. The model can volunteer "you can
still download it" instead of pretending the preview is fine.
`hasOfficeHtmlPath` exported from `@librechat/api` so `processCodeOutput`
can decide whether a file expects a preview at all.
* 🔍 feat: `GET /api/files/:file_id/preview` endpoint and boot orphan sweep
- New `GET /api/files/:file_id/preview` route returns
`{ status, text?, textFormat?, previewError? }`. The frontend's
`useFilePreview` React Query hook polls this while phase-2 is in
flight, then auto-stops on terminal status. ACL identical to the
download route (reuses `fileAccess` middleware). Defaults `status`
to `'ready'` for legacy records so back-compat is implicit.
`text` only included when `status === 'ready'` and non-null —
preserves the HTML-or-null security contract from #12934.
- `sweepOrphanedPreviews()` invoked on boot in both `server/index.js`
and `server/experimental.js`. Recovers any `pending` records left
behind by a process restart mid-extraction (the only case the
in-process two-phase flow can't handle on its own). Fire-and-forget
so a transient sweep failure doesn't block startup.
* 🖥️ feat: frontend two-phase preview consumer (polling + UI states)
Wires the React side to the new lifecycle so the user sees what's
happening with their file while phase-2 extraction runs in the
background and after the response stream closes.
- `useAttachmentHandler` upserts by `file_id` (was append-only) so
the phase-2 SSE update event merges over the pending placeholder
in place. Lightweight attachments without a `file_id`
(web_search / file_search citations) keep the legacy append path.
- `useFilePreview(file_id)` React Query hook with
`refetchInterval: (data) => data?.status === 'pending' ? 2500 : false`
so polling auto-stops on the first terminal response without the
caller having to flip `enabled`.
- `useAttachmentPreviewSync(attachment)` bridges polled data into
`messageAttachmentsMap`. Polling enabled iff
`status === 'pending' && isAnySubmitting` — per the design ask:
active polling while the LLM is still generating, then quiet.
Process-restart and post-stream cases are covered by polling on
the next interaction.
- `Attachment.tsx` renders a small `PreviewStatusIndicator` (spinner +
"Preparing preview…" for pending, alert icon + "Preview unavailable"
for failed) inside `FileAttachment`. Download button stays fully
functional in both states. Two new English locale keys.
- Data-provider scaffolding: `TFilePreview` type, `endpoints.filePreview`,
`dataService.getFilePreview`, `QueryKeys.filePreview`.
* 🧪 fix: stub `useAttachmentPreviewSync` in pre-existing Attachment test mocks
The new `useAttachmentPreviewSync` hook is called unconditionally inside
`FileAttachment` (added in the prior commit). Two pre-existing test
files mock `~/hooks` to provide `useLocalize` only — the un-mocked
preview hook reference resolved to undefined and crashed render with
`(0 , _hooks.useAttachmentPreviewSync) is not a function` on the
Ubuntu/Windows CI runners.
Fix is local to the test mocks: add a no-op stub that returns
`{ status: 'ready' }` so the component renders the legacy chip path.
The two-phase preview behavior itself has its own dedicated suites
(`useAttachmentHandler.spec.tsx`, `useAttachmentPreviewSync.spec.tsx`).
* 🐛 fix: route phase-2 attachment update to current-run messageId
Codex P1 review on PR #12957. `processCodeOutput` intentionally
preserves the original DB `messageId` across cross-turn filename reuse
so `getCodeGeneratedFiles` can still trace a file back to the
assistant message that originally produced it. The phase-1 SSE emit
already routes by the current run's messageId — `processCodeOutput`
runtime-overlays it via `Object.assign(file, { messageId, toolCallId })`
and the callback writes `result.file` directly.
Phase-2 was passing the raw `updateFile` return through
`attachmentFromFileMetadata`, which read `messageId` straight off the
DB record. On a turn-N run that re-emitted a filename from turn-1
(e.g. agent writes `output.csv` again), the phase-2 SSE update
routed to `turn-1-msg` instead of `turn-N-msg`. Frontend's
`useAttachmentHandler` upserts under the wrong messageAttachmentsMap
slot — turn-N's pending chip stays stuck at "preparing preview…"
while turn-1's already-resolved attachment gets re-merged.
Fix: thread `runtimeMessageId` through `attachmentFromFileMetadata`
and pass `metadata.run_id` from the phase-2 emit site. Mirrors how
phase-1 sources its messageId. Tests cover the cross-turn reuse case
plus the writableEnded / null-finalize / no-finalize paths to lock
in the broader phase-2 emit contract.
* 🛠️ refactor: address codex audit findings (wire-shape parity, DRY, defensive catch)
Comprehensive audit on PR #12957. Resolves all valid findings:
- **MAJOR #1 — Wire-shape parity**: phase-1 ships the full `fileMetadata`
record over SSE; phase-2 was using a tight `attachmentFromFileMetadata`
projection. Drop the projection and have phase-2 spread `{...updated,
messageId, toolCallId}` so both events match the long-standing
legacy phase-1 shape clients depend on.
- **MAJOR #2 — DRY**: extract `runPhase2Finalize({ finalize, fileId,
onResolved })` into `process.js` (alongside `processCodeOutput` whose
contract it pairs with). Both `callbacks.js` paths and `tools.js`
now flow through it. Single catch path eliminates divergence
surface — the fix landed in 01704d4f0 (cross-turn messageId routing)
was a symptom of this duplication risk.
- **MINOR #3 — JSDoc accuracy**: `finalizePreview`'s buffer is bounded
by `fileSizeLimit`, not the 1MB extractor cap. Updated and added a
note about peak heap from queued buffers.
- **MINOR #4 — Defensive catch**: `runPhase2Finalize`'s catch attempts
a best-effort `updateFile({ status: 'failed', previewError:
'unexpected' })` for the file_id, so a programming bug in
`finalizePreview` doesn't leave the record stuck `'pending'` until
the next boot-time orphan sweep.
- **NIT #6 — Stale PR refs**: 12952 → 12957 in 3 places.
- **NIT #7 — Schema bound**: `previewError` capped at `maxlength: 200`
to prevent a future codepath from accidentally persisting a stack
trace.
Skipped per audit verdict (non-blocking):
- #5 (memory pressure): documented in JSDoc; impl change was reviewer's
"consider", not actionable.
- #8 (double DB query per poll): low cost, indexed by_id, polling is
gated narrow.
- #9 (TAttachment cast): the union type is intentional; the casts are
safe widening, refactoring TAttachment is invasive and out of scope.
Tests: 11 new (7 `runPhase2Finalize` unit tests covering happy path,
null-finalize, throws, double-fail, no-fileId, no-onResolved; +4
wire-shape parity assertions in the existing cross-turn test). 328
backend tests pass; 528 frontend tests pass; lint and typecheck clean.
* 🛡️ refactor: address codex P1+P2 + rename to drop phase-1/2 jargon
Codex round 2 review on PR #12957 caught two race conditions and one
recovery gap, all triggered by cross-turn filename reuse (`claimCodeFile`
intentionally returns the same `file_id` for the same
`(filename, conversationId)` across turns). Plus naming cleanup the
user requested — internal "phase 1 / phase 2" vocabulary leaks across
sprints, replace it everywhere with terms describing what's actually
happening.
P1 — stale render overwrites newer revision (process.js)
Two turns reusing `output.csv` share a `file_id`. If turn-1's
background render resolves AFTER turn-2's persist step, the
unconditional `updateFile` writes turn-1's stale text/status over
turn-2's pending placeholder. Fix: stamp a fresh `previewRevision`
UUID on every emit, thread it through `finalizePreview`, and make
the commit conditional via a new optional `extraFilter` argument
on `updateFile` (`{ previewRevision: <expected> }`). The defensive
`updateFile` in `runPreviewFinalize`'s catch uses the same guard
so a programming error from an older render also can't override a
newer turn.
P1 — stale React Query cache on pending remount (queries.ts)
Same root cause from the frontend side. Cache key
`[QueryKeys.filePreview, file_id]` may hold a prior turn's `'ready'`
payload; with `refetchOnMount: false` and the polling gate on
`pending`, polling never starts for the new placeholder. Fix:
`useAttachmentHandler` invalidates that query whenever an attachment
with a `file_id` arrives. Both initial-emit and update events
trigger invalidation — uniform gate.
P2 — quick-restart orphans skipped by boot sweep (files.js)
Boot `sweepOrphanedPreviews` uses a 5-min cutoff for multi-instance
safety. A crash + restart inside the cutoff leaves `pending` records
that never get touched again. Fix: lazy sweep inside the preview
endpoint — if a polled record is `pending` and `updatedAt` is older
than 5 min, mark it `failed:orphaned` on the spot before responding.
Conditional on the same `updatedAt` we observed so a concurrent
legitimate update wins. Cheap, bounded by user activity.
Naming cleanup
- `runPhase2Finalize` → `runPreviewFinalize`
- `PHASE_TWO_TIMEOUT_MS` → `PREVIEW_FINALIZE_TIMEOUT_MS`
- All `phase-1` / `phase-2` / `two-phase` prose replaced with
"the immediate emit", "the deferred render", "the persist step",
"the deferred preview", etc. Skill-feature `phase 1/2` references
(different feature) left alone.
Tests: 10 new (4 lazy-sweep × preview endpoint, 3 cache-invalidation ×
useAttachmentHandler, 3 extraFilter × updateFile data-schemas).
Backend 332/332, frontend 531/531, data-schemas 37/37, lint clean.
* 🛠️ refactor: address comprehensive review (round 3) — stale-cache MAJOR + 3 minors
Comprehensive review on PR #12957 caught a P1 follow-on bug from the
prior `invalidateQueries` fix, plus 3 maintainability findings.
MAJOR: stale React Query cache not actually fixed by `invalidateQueries`
The previous fix called `invalidateQueries` to flush stale cached
preview data on cross-turn filename reuse. But `useFilePreview` had
`refetchOnMount: false`, which made the new observer read the
stale-marked 'ready' data without refetching. The polling
`refetchInterval` then evaluated against stale 'ready' → returned
`false` → polling never started → user stuck on stale content.
Fix (belt-and-suspenders):
a) `useAttachmentHandler` switched to `removeQueries` — drops the
cache entry entirely so the next mount has nothing to read and
must fetch.
b) `useFilePreview` no longer sets `refetchOnMount: false`, so the
React Query default (`true`) kicks in — second line of defense
if any future codepath observes stale data before the handler
has a chance to evict.
MINOR: `finalizePreview` JSDoc missing `previewRevision` param
Added with explanation of the conditional update guard.
MINOR: asymmetric stream-writable guard between SSE protocols
Chat-completions delegated the gate to `writeAttachmentUpdate`;
Open Responses inlined `!res.writableEnded && res.headersSent`.
Extracted `isStreamWritable(res, streamId)` predicate; both paths
+ `writeAttachmentUpdate` now share the single source of truth.
NIT: `(data as Partial<TFile>).file_id` cast repeated 4 times
Extracted to a `fileId` local at the top of the handler.
Tests: existing 9 invalidate-tests rewritten as remove-tests; +1 new
lock-in test asserts removeQueries is called and invalidateQueries
is NOT (regression guard against round-3 finding). 332 backend pass,
532 frontend pass, lint clean.
Skipped findings (deferred / acceptable):
- MINOR: post-submission pending state has no auto-recovery — the
`isAnySubmitting` polling gate was the user's explicit design;
LLM context surfaces failed/pending so the model can volunteer.
Worth a follow-up if real users hit it.
- NIT: double DB query per preview poll — reviewer marked acceptable;
changing `fileAccess` middleware is out of scope.
* 🛡️ test: address comprehensive review NITs (initial-emit guard + isStreamWritable coverage)
NIT — chat-completions initial emit skips writableEnded check
The Open Responses initial emit was switched to use the new
`isStreamWritable` predicate in the round-3 commit, but the
chat-completions initial emit kept the older narrower check
(`streamId || res.headersSent`). On a client disconnect mid-stream
(`writableEnded === true`) it would still hit `res.write` and
raise `ERR_STREAM_WRITE_AFTER_END` — caught by the outer IIFE
catch but logged as noise. Switch this site to `isStreamWritable`
too so both initial-emit paths share the same gate as the
deferred update emits.
NIT — `isStreamWritable` not directly unit-tested
The predicate was only covered indirectly via the deferred-preview
SSE tests (writableEnded skip, headersSent check). Export from
`callbacks.js` and add 5 parametric tests pinning down each branch
(streamId truthy, res null, !headersSent, writableEnded, happy
path) so a future condition addition can't silently regress.
* 🐛 fix: stuck "Preparing preview…" + inline the chip subtitle
Two related fixes for a stuck-spinner bug a user reported in manual
testing of PR #12957.
**Stuck spinner (the bug)**
The deferred preview render can complete a few seconds AFTER the SSE
stream closes (typical case: PPTX render finishes ~3s after the LLM
emits FINAL). When that happens, the SSE update is silently dropped
(`isStreamWritable` returns false on a closed stream) and polling is
the only recovery path.
The earlier polling gate was `status === 'pending' && isAnySubmitting`,
which mirrored the original design intent ("only query while the LLM
is still generating"). But `isAnySubmitting` flips false the moment
the model emits FINAL — milliseconds before the deferred render
commits. Polling never runs, the chip stays "Preparing preview…"
forever even though the DB has `status: 'ready'` with valid HTML.
Drop the `isAnySubmitting` part of the gate. `useFilePreview`'s
`refetchInterval` is already a function-form that returns `false` on
the first terminal response, so polling auto-stops within one tick of
resolution. The server-side render ceiling (60s) plus the lazy sweep
in the preview endpoint cap the worst case to ~24 polls per pending
attachment. Polling itself never blocks UX — the gate's purpose was
"don't waste cycles", and capping by terminal status is the correct
expression of that.
**Inline the chip subtitle (the visual)**
The previous design rendered "Preparing preview…" as a loose-feeling
spinner+text BELOW the file chip. The chip itself looked done while a
floating annotation said it wasn't.
`FileContainer` gains an optional `subtitle?: ReactNode` prop that
overrides the default file-type label. `Attachment.tsx` passes a
`PreviewStatusSubtitle` (spinner + "Preparing preview…" / alert +
"Preview unavailable") into that slot when the file's preview is
pending or failed. The chip footprint stays identical to its `'ready'`
form — just the second row swaps from "PowerPoint Presentation" to
the status indicator. No floating element, no layout shift.
Tests: regression test pinning down "polling stays enabled after the
LLM finishes" so a future revert can't reintroduce the stuck-spinner
bug. Existing FileContainer tests pass unchanged (subtitle override
is opt-in). 522 frontend tests pass; lint clean.
* 🐛 fix: deferred-preview survives reload + matches artifact card chrome
Fixes the remaining stuck-pending case after the polling gate fix: on
a reloaded conversation, message.attachments come from the DB frozen at
the immediate-persist `status: 'pending'`, but `messageAttachmentsMap`
is empty because no SSE handler ever fired for that messageId. Polling
now INSERTS a new live entry when no record matches the file_id, and
`useAttachments` merges live entries onto DB entries by file_id so the
resolved text/textFormat reach `artifactTypeForAttachment` and the
chip routes through the proper PanelArtifact card.
Also replaces the small file chip used during the pending state with
a PreviewPlaceholderCard that mirrors ToolArtifactCard chrome, so the
transition to the resolved PanelArtifact no longer reshapes the UI.
* ✨ feat: auto-open panel when deferred preview resolves pending→ready
The legacy auto-open path is gated only on `isSubmitting`, so an
office-file preview that resolves *after* the SSE stream closes would
render in place but never auto-open the panel — even though that's
exactly the moment the result becomes meaningful to the user. Adds a
per-file_id one-shot signal that `useAttachmentPreviewSync` flips on
the pending→ready edge; `ToolArtifactCard` consumes it on mount and
auto-opens regardless of submission state. The signal is *only* set on
the actual transition (history loads of pre-resolved files don't
trigger it) and is consumed once (panel close + reopen on the same
card stays user-controlled).
* 🐛 fix: drop placeholder Terminal overlay + scope auto-open to fresh resolutions
Two fixes for issues spotted in manual testing of the deferred-preview
auto-open feature:
1. PreviewPlaceholderCard was passing `file={attachment}` to FilePreview,
which triggered SourceIcon's Terminal overlay (`metadata.fileIdentifier`
is set on every code-execution file). The artifact card itself doesn't
show that overlay; the placeholder shouldn't either, so the
pending→resolved transition is visually seamless.
2. The `previewJustResolved` flag flipped on every pending→ready
transition observed by the polling hook — including stale-pending
DB records that resolve via the first poll on a *history load*.
Conversations whose immediate-persist snapshot left attachments at
`status: 'pending'` would yank the panel open every revisit.
Adds `mountedDuringStreamRef` to the hook (mirroring ToolArtifactCard)
so the flag fires only when the hook itself was mounted during an
active turn — preserving the pre-PR contract that the panel only
auto-opens for results the user is actively waiting on, never for
history.
* 🐛 fix: don't downgrade preview to failed when only the SSE emit throws
Codex P2 finding on PR #12957: the original chain placed `.catch` after
`.then(onResolved)`, so a throw inside `onResolved` (transport-side
errors — SSE write race after stream close, an emitter listener
throwing) would propagate into the finalize catch and persist
`status: 'failed'` / `previewError: 'unexpected'`. That surfaced
"preview unavailable" in the UI for a perfectly valid file, and
degraded next-turn LLM context to reflect a non-existent failure.
Wraps `onResolved` in its own try/catch so emit errors are logged but
do not affect the file's persisted status. Extraction success and
emit success are now independent: if extraction succeeds and
`finalizePreview` writes the terminal status, the polling layer / next
page load surfaces the resolved preview even if this turn's SSE emit
didn't land.
* 🛡️ fix: run boot-time orphan sweep under system tenant context
Codex P2 finding on PR #12957: `File` is tenant-isolated, so under
`TENANT_ISOLATION_STRICT=true` the boot-time `sweepOrphanedPreviews`
threw `[TenantIsolation] Query attempted without tenant context in
strict mode` and the recovery path silently failed every restart.
Stale `status: 'pending'` records would be stuck until a user happened
to poll the preview endpoint and trigger the lazy sweep — which only
covers the file the user is currently looking at, not the bulk
candidate set the boot sweep is designed to recover.
Wraps the sweep in `runAsSystem(...)` in both boot paths
(`api/server/index.js` and `api/server/experimental.js`) and pins the
contract with regression tests in `file.spec.ts` — one test asserts
the bare call throws under strict mode, the other asserts the
`runAsSystem`-wrapped call succeeds.
* 🧹 chore: trim verbose comments from previous commit
* 🧹 chore: address review findings (dead branch, lazy-sweep cutoff, stale JSDoc)
- finalizePreview: drop unreachable !isOfficeBucket branch (caller
already gates on hasOfficeHtmlPath, so this path is always office)
- preview endpoint: drop lazy-sweep cutoff from 5min to 2min — anything
past the 60s render ceiling is definitively orphaned, and per-request
sweep can be tighter than the per-instance boot sweep
- strip stale `isSubmitting` references from JSDoc in 3 spots (the
client-side gate was removed in
|
||
|
Danny Avila
|
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. |
||
|
Danny Avila
|
738003b220
|
🛡️ fix: Prevent silent crash from unhandled MCP OAuth reconnect rejections (#12812)
* 🛡️ fix: Install global `unhandledRejection` handler Node 15+ terminates the process by default when a promise rejection goes unhandled. Under MCP OAuth reconnect storms and streamable-HTTP transport resets, fire-and-forget async paths can emit transient rejections (ECONNRESET, token refresh races) that would otherwise silently kill the server — no uncaught exception log, no OOM signal. Register a listener so these paths log and the process keeps serving other requests. Refs: #12078 * 🔧 fix: Guard MCP OAuth reconnect fire-and-forget calls `OAuthReconnectionManager.tryReconnect` awaits `getServerConfig` outside its inner try/catch, so a rejection from the registry (or any throw before the guarded block) would escape the fire-and-forget `void` call sites and propagate as an unhandled rejection — the failure mode behind the silent crashes reported in #12078. Route both call sites through a `safeTryReconnect` wrapper that attaches a terminal `.catch` so unexpected rejections are surfaced via the logger instead. Refs: #12078 * 🧹 fix: Address review findings on MCP OAuth reconnect crash fix - Move `getServerConfig` inside `tryReconnect`'s try/catch so the registry rejection path is handled by the inner cleanup (the structural root cause behind the silent crash). The outer `safeTryReconnect` wrapper remains as defense-in-depth. - Extract the failed-reconnect cleanup as a private `cleanupOnFailedReconnect` method and invoke it from `safeTryReconnect`'s catch as well, so any rejection that does escape the inner try (e.g. a future regression) still resets tracker state instead of leaving the server stuck in `active` for the full `RECONNECTION_TIMEOUT_MS` window. - Update the regression test to assert tracker state is cleaned up (`isActive` cleared, `isFailed` set, `disconnectUserConnection` called) so it can detect the stale-state failure mode it was meant to guard against. - Forward non-Error rejection reasons as-is in the global handler so structured payloads like `{ code: "ECONNRESET", errno: -104 }` survive instead of being collapsed to "[object Object]" by `String()`. Refs: #12078, review of #12812 * 🚑 fix: Restore fail-fast on boot rejection in primary server entry `startServer()` was invoked bare in `api/server/index.js`. Before installing the global `unhandledRejection` handler, a startup rejection (`connectDb`, `getAppConfig`, `performStartupChecks`) terminated the process via Node's default — Kubernetes / the orchestrator restarted the pod immediately. After the handler was added, the same rejection was caught and logged, then the process kept running half-initialized (no HTTP listener) until the liveness probe eventually timed out — slow, indirect recovery instead of a fast restart. Wrap `startServer()` with the same `.catch(() => process.exit(1))` pattern already used in `experimental.js` so boot failures fail-fast. Refs: #12078, codex review of #12812 * 🚑 fix: Fail-fast on post-listen init failure in both server entries The `app.listen` callback in `index.js` and `experimental.js` is async and awaits `initializeMCPs`, `initializeOAuthReconnectManager`, and `checkMigrations`. The callback's promise is detached from `startServer().catch(...)` (the outer catch only sees errors that occurred before `app.listen` was called), so without explicit handling those init rejections used to terminate the process via Node's default and now would be swallowed by the new `unhandledRejection` handler — leaving the HTTP server listening (and passing liveness probes) while MCP / OAuth / migration state is broken. Wrap the post-listen init block in a try/catch that logs and calls `process.exit(1)` so initialization failures stay fail-fast. Refs: #12078, codex review of #12812 |
||
|
Danny Avila
|
2e706ebcb3
|
⚖️ refactor: Split Config Route into Unauthenticated and Authenticated Paths (#12490)
* refactor: split /api/config into unauthenticated and authenticated response paths
- Replace preAuthTenantMiddleware with optionalJwtAuth on the /api/config
route so the handler can detect whether the request is authenticated
- When unauthenticated: call getAppConfig({ baseOnly: true }) for zero DB
queries, return only login-relevant fields (social logins, turnstile,
privacy policy / terms of service from interface config)
- When authenticated: call getAppConfig({ role, userId, tenantId }) to
resolve per-user DB overrides (USER + ROLE + GROUP + PUBLIC principals),
return full payload including modelSpecs, balance, webSearch, etc.
- Extract buildSharedPayload() and addWebSearchConfig() helpers to avoid
duplication between the two code paths
- Fixes per-user balance overrides not appearing in the frontend because
userId was never passed to getAppConfig (follow-up to #12474)
* test: rewrite config route tests for unauthenticated vs authenticated paths
- Replace the previously-skipped supertest tests with proper mocked tests
- Cover unauthenticated path: baseOnly config call, minimal payload,
interface subset (privacyPolicy/termsOfService only), exclusion of
authenticated-only fields
- Cover authenticated path: getAppConfig called with userId, full payload
including modelSpecs/balance/webSearch, per-user balance override merging
* fix: address review findings — restore multi-tenant support, improve tests
- Chain preAuthTenantMiddleware back before optionalJwtAuth on /api/config
so unauthenticated requests in multi-tenant deployments still get
tenant-scoped config via X-Tenant-Id header (Finding #1)
- Use getAppConfig({ tenantId }) instead of getAppConfig({ baseOnly: true })
when a tenant context is present; fall back to baseOnly for single-tenant
- Fix @type annotation: unauthenticated payload is Partial<TStartupConfig>
- Refactor addWebSearchConfig into pure buildWebSearchConfig that returns a
value instead of mutating the payload argument
- Hoist isBirthday() to module level
- Remove inline narration comments
- Assert tenantId propagation in tests, including getTenantId fallback and
user.tenantId preference
- Add error-path tests for both unauthenticated and authenticated branches
- Expand afterEach env var cleanup for proper test isolation
* test: fix mock isolation and add tenant-scoped response test
- Replace jest.clearAllMocks() with jest.resetAllMocks() so
mockReturnValue implementations don't leak between tests
- Add test verifying tenant-scoped socialLogins and turnstile are
correctly mapped in the unauthenticated response
* fix: add optionalJwtAuth to /api/config in experimental.js
Without this middleware, req.user is never populated in the experimental
cluster entrypoint, so authenticated users always receive the minimal
unauthenticated config payload.
|