mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-04 05:28:30 +00:00
178 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Danny Avila
|
67b7b441b2
|
🛂 feat: Filter Model-Bound Content by Source (#14425)
* feat: introduce optional content protection seam * feat: enforce source-aware content filters * feat: complete source-aware content enforcement * test: activate skill file-text fail-close fixtures * fix: harden source-aware content filters * fix: harden model-bound content filtering * fix: preserve legacy filters and generated files * fix: inspect shared scalar metadata * test: align mocks with current dev dependencies * feat: add persisted content filter safeguards * feat: complete source-aware content filter enforcement * fix: move resume content preflight into TypeScript * fix: close content inspection edge cases * fix: harden content protection boundaries * fix: complete content protection safeguards * test: align persisted memory filter coverage * fix: reconcile content protection with current dev * fix: reconcile content protection with latest dev * fix: close content protection review gaps * fix: enforce source-aware provider boundaries * fix: preserve legacy PII preflight semantics * test: stabilize stored branch preflight fixture * fix: defer agent writes until protected model admission * perf: harden source-aware model-bound filtering * fix: canonicalize provider lineage before validation * fix: satisfy model-bound callback type checks * perf: Bound content protection filtering work * fix: Bound submission array traversal * fix: Stabilize bounded content snapshots * fix: Scope model-bound traversal overflows * fix: Preserve scoped content inspection * fix: Accumulate aggregate traversal scopes * fix: centralize content policy boundaries * test: align deferred tool policy context * test: align controller policy mocks * style: normalize content protection imports * fix: close content policy review gaps * fix: narrow active skill policy config * fix: address content protection review boundaries * fix: retain exact provenance overflow sentinel * fix: preserve literal and scoped provenance updates * fix: narrow persisted edit provenance * fix: isolate exact overflow attribution * fix: centralize stored prompt protection * fix: fail closed on incomplete transcript evidence * fix: align canonical transcript routing * refactor: centralize content policy preflights * fix: isolate upload policy error typing * style: sort policy preflight imports * refactor: centralize content policy boundaries |
||
|
Danny Avila
|
8ae94afa91
|
🪡 fix: Thread Parent Message ID Through MCP Request-Scoped Bodies (#15095)
* fix: Unify MCP request-scoped headers * fix: address request-scoped MCP review findings * test: preserve request scope on status errors * fix: treat authorized on-demand MCP servers as ready * refactor: separate MCP readiness from connection state * fix: preserve on-demand MCP readiness labels * test: satisfy OpenAI conversation ownership guard * fix: keep MCP action predicates boolean * fix: close deferred MCP request context gaps * fix: preserve on-demand MCP configuration actions * fix: fail closed on unavailable MCP parent context * test: complete MCP connecting-state mocks * fix: preserve missing MCP parent on continuations * fix: align native MCP request identities * fix: preserve edited MCP parent identity * test: use scoped Agent initializer fixture * test: expose MCP request body helper * fix: preserve MCP turn identity across resume * style: sort stream metadata imports * fix: carry normalized MCP identity to execution |
||
|
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
|
547bd8c4bf
|
🧵 feat: Persist View-Only Subagent Threads (#14957) | ||
|
Danny Avila
|
7d62be2ad3
|
🕸️ feat: Run Saved Agent Teams as Subagents (#14944)
* feat: Add graph subagent integration * style: Sort response usage test imports * fix: Preserve lazy graph runtime context * fix: Use isolated graph input helper * test: Align graph integration fixtures * fix: Preserve lazy graph runtime capabilities * fix: Bound lazy graph metadata preload * fix: Harden lazy graph resolution lifecycle * fix: Coalesce lazy graph member resolution * fix: Snapshot initialized graph members only * fix: Preserve lazy agent runtime context * fix: Preserve batched lazy context preparation * fix: Preserve graph member capability bounds * fix: reconcile graph subagents with execution profiles * style: align graph subagent types with formatter |
||
|
Danny Avila
|
7d850c308a
|
🧠 feat: Add Live Reasoning Labels (#14893)
* feat: add live reasoning labels * fix: Stabilize reasoning label checks * fix: Address reasoning label review findings * chore: Bump Agents SDK for reasoning labels * fix: Reset reused reasoning step evidence * fix: Reconcile cleared reasoning labels * fix: Fence reasoning label resets * fix: Reset reasoning ownership before gap labels * fix: Preserve THINK type through label reset * test: Expect run-global reasoning revision |
||
|
Danny Avila
|
06bf324cf0
|
🛤️ feat: Per-Agent Code Execution Routing With Stateful Session Scopes (#14848)
* feat: route code execution per agent profile * chore: sort execution profile imports * test: preserve stateful environment literal types * fix: isolate stateful code environments by user * fix: preserve per-agent code routing end to end * fix: route code priming by execution profile * fix: isolate code profile lifecycle state * fix: preserve mixed-profile code resources * fix: complete stateful skill routing |
||
Anubhav Anand
|
a2ad0aa0c8
|
🤐 feat: Allow Promptless Sends When Files Are Attached (#13717)
* ✨ feat: Allow sending file attachments without a text message When an agent asks the user to upload a document, the user could attach the file but still had to type a placeholder message ("OK", "Here is the file") before the send button enabled and the submit guard let the message through. Attachments now count as submittable content: - New isSubmittableMessage(text, fileCount) util: non-whitespace text OR at least one attached file. - ask() in useChatFunctions uses it instead of bailing on empty text, so an empty draft with attached files submits. - SendButton receives the attached file count and enables accordingly. - ChatForm only marks the text field as required when no files are attached, so react-hook-form validation no longer blocks handleSubmit. Submitting an empty draft with no attachments is still rejected at all three layers. Fixes #13646 * Address review: support replayed file-only turns + drop empty vision text - ask(): count replayed attachments (overrideFiles) in the submittable check and skip it entirely for regenerate, so a file-only message can be regenerated or saved-and-resubmitted instead of being rejected as empty. - formatVisionMessage(): omit the text content part when the message text is empty. Anthropic rejects empty text content blocks with HTTP 400, and an empty block adds nothing for other providers; image-only sends now format cleanly. Added formatMessages tests for with-text and image-only (Anthropic + other) cases. * Address review: keep attachment-only turns valid for providers, answer mode, and titles - formatMessage: substitute minimal text when a user turn carries files but no inline content, so Anthropic does not reject an empty user message for RAG or code-environment attachments. - assistants chatV1: send the same stand-in for attachment-only Threads messages, which reject an empty body. The persisted message keeps empty text. - ChatForm: attachments no longer make an empty draft submittable in answer mode, where submitText consumes the click without answering or sending. - agents request: seed title generation from attachment filenames when the turn has no text, so immediate-mode titles are not invented from an empty string. - useChatFunctions.regenerate.spec: mock the utils barrel over the real module so new exports resolve. * Cover the agents path for attachment-only turns AgentClient formats its payload with the SDK's formatMessage, not the local one, so the earlier guard missed the endpoint the feature actually targets: an attachment-only turn still reached Anthropic as an empty user message. Apply the same stand-in after the file-context and quote merges, so a turn that already gained inline content is untouched. * Carry filenames on freshly attached files The fresh-file submission mapping copied only file_id, filepath, type, and dimensions, so the attachment-only title fallback read an undefined filename and produced nothing. Include filename, and cover it with a test that submits an empty draft with one attachment. * Address review: cover assistants v2, fresh agent attachments, editor, and title fallback - agents client: the current turn has no files during buildMessages, so read the resolved attachments from message_file_map instead. The previous guard only ever fired for persisted historical turns. - assistants chatV2: the default assistants endpoint routes here, so it needs the same stand-in body chatV1 got. - assistants title: fall back to filenames, then the response, and keep the default title rather than saving an empty one. - EditMessage: retained attachments make an empty edit submittable, matching the composer, so the overrideFiles replay path is reachable from the UI. --------- Co-authored-by: Marco Beretta <81851188+berry-13@users.noreply.github.com> |
||
|
Danny Avila
|
df6e15a0de
|
🔖 feat: Bound Parent Activity Phases With an Exclusive End Index (#14768)
* 🧭 fix: Finalize Parent Activity Phases at Run Completion * 🧭 fix: Preserve Activity Phase Boundaries * 🎨 fix: Format Activity Phase Boundary Check * 🧭 fix: Ignore Late Label Artifacts at Phase Completion * 🧭 fix: Preserve Logical Activity Phase Membership * 🩹 fix: Narrow Optional Activity Phase Marker * fix activity phase tail boundaries * fix activity phase test lint * fix straddling activity phase batches * preserve activity phase boundaries at scale * fix persisted activity phase final boundary * fix resumed activity phase edge cases * fix sparse activity phase grouping * fix sparse activity phase tail scan * fix resumed activity phase text fallback * fix sparse activity phase completion scans * avoid sparse activity phase runtime scans * stabilize sparse activity phase resumes * support activity phases on current ts target * preserve sparse phase reservations * finalize activity phase boundary handling * avoid sparse phase start scans * fix activity phase final text bounds * tighten activity phase summary boundaries * format activity phase boundary checks * leave final commentary outside activity phases * recognize lane-tagged final activity text * rebase retained activity boundaries on resume * bound activity phase collection work * correct resumed phase activity count * resolve late reasoning before phase completion * preserve lane-tagged final answers * assert durable activity phase bounds in e2e * preserve empty finalized activity phases * ignore empty reasoning at phase completion * format phase completion guard * fix(api): retain overflow reasoning anchors * perf(api): index overflow reasoning anchors * perf(api): skip empty reasoning index scans * fix(api): reconcile completion boundaries efficiently |
||
James Todaro
|
e696b07619
|
🧾 fix: Honor Disabled Transactions on the Token-Count Fallback Path (#14774)
`AgentClient.recordTokenUsage` had no `transactions` parameter, so the setting never reached `createTransaction`, whose guard reads it from the object it is handed. `transactions?.enabled === false` saw `undefined` and the write went ahead. This path is reached only from `BaseClient`'s fallback branch, when the provider returns no usable stream usage, so the bulk path masked it wherever usage is reported. Where it is not, the setting had no effect at all. |
||
|
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
|
236ee6c1ab
|
🧭 fix: Re-Anchor Parent Activity Phase Bounds (#14741)
* test: cover parent activity phase finalization * test(e2e): stabilize parent phase coverage * fix(agents): reanchor parent activity phase bounds * fix(agents): preserve delayed tools in activity phases * test(agents): keep phase slice bounds typed * fix(agents): preserve sparse activity phase bounds * test(e2e): read structured phase replies |
||
|
Danny Avila
|
7347cfc195
|
🍡 feat: Batched User Questions With A Single Bounded Answer Form (#14737)
* feat: support batched user questions * test: align batched question fixtures * fix: harden batched question lifecycle * test: submit batched HITL answers in e2e * fix: address batched question review findings * fix: preserve invoke return typing |
||
|
Danny Avila
|
09cbd54f48
|
🪆 fix: Rebase Activity Phase Bounds over Sparse Content (#14729)
The aggregator writes content parts at provider-source indexes, which can skip slots and leave holes in contentParts. Array.prototype.map preserves those holes and the Map constructor iterates them as undefined, so rebaseActivityPhaseBounds threw "Iterator value undefined is not an entry object" at the end of every run with sparse content — deterministic with parent phase summaries enabled, on both the completion and resume paths. Build the identity map with an index loop that skips nullish slots. Holes must stay out of the map: one undefined key would falsely match every hole in previousParts as a retained part and corrupt the rebased bound. |
||
|
Danny Avila
|
a3cec67e08
|
🪆 feat: Add Parent Activity Phase Summaries (#14721)
* feat: add activity phase summaries * fix: preserve activity phase lifecycle semantics * fix: satisfy activity phase type checks * fix: simplify activity phase status mapping * style: format activity phase changes * fix: rebase activity phase bounds after shaping * fix: link activity phase trace ancestry * fix: reconcile activity phase bounds * style: format activity phase reconciliation test * style: align activity phase assertion * fix: retain reasoning across commentary * fix: preserve activity phase boundary state * fix: detect renderable phase children * test: type parallel phase assertion * chore: bump agents SDK for activity phases * fix: retain unphased lane reasoning * fix: preserve tool group expansion across phases * style: format phase expansion regression * fix: preserve phase interaction state efficiently * perf: skip sparse phase segment holes * perf: partition phase segments with offsets * fix: preserve phase boundaries and cursor state * test: align activity phase regressions with CI * test: keep phase context mock hoist-safe |
||
|
Danny Avila
|
7e74f8eb8c
|
🪪 fix: Strip Unresolved Header Placeholders at Final Resolution (#14595)
Unresolved {{LIBRECHAT_USER_*}} header templates leaked literally to
upstream providers when user context was missing at resolution time
(e.g. async title generation racing client disposal), letting a gateway
trust LibreChat's own template syntax as an account identity.
resolveHeaders now takes an opt-in stripUnresolved flag that blanks any
resolvable-but-unresolved LIBRECHAT_USER/BODY/OPENID placeholder, enabled
at every final resolution boundary (resolveConfigHeaders, model fetches,
Google init, summarization overrides, azureAssistants init). Staged
passes that resolve again later with more context are left untouched, as
is the async-resolved {{LIBRECHAT_GRAPH_ACCESS_TOKEN}} and unknown names.
titleConvo now resolves headers from the req captured at entry instead of
re-reading this.options.req, which disposeClient nulls concurrently.
Fixes #14580
|
||
|
Danny Avila
|
6f45a9e32e
|
🔗 fix: Normalize MCP Tool Keys at Every Producer, Resolve Raw Names via Aliases (#14553)
* 🔗 fix: Normalize MCP Tool Keys at Every Producer, Resolve Raw Names via Aliases Tool keys had two spellings that could diverge for any server whose name contains characters outside [a-zA-Z0-9_.-]: the tool cache (and registry inspector) built keys with the RAW server name, while runtime instances are named with normalizeServerName(serverName). Three code comments already asserted "tool keys embed the normalized server name" - no producer honored it. For a special-character server that meant: - definitions-only mode shipped raw def names the model echoed back, but the executor's tool map held the normalized instance name, so every call failed with "Tool not found"; - per-tool tool_options (defer_loading / allowed_callers / run_in_background / describe_intent) were persisted under raw keys that never matched the definition names the option passes resolve against, so builder settings were silently inert; - tool-key parsing against normalized candidate lists silently fell back to last-delimiter splitting, which mis-parses delimiter-bearing tool names. The reconciliation is one contract enforced in three moves: 1. PRODUCERS NORMALIZE. The tool cache (packages/api/src/mcp/tools.ts) and the registry inspector build keys with the normalized server name, matching the instance names MCP.js has always assigned. The builder's tool ids, agent.tools entries, tool_options keys, and definition names all flow from these keys, so every model-facing name now agrees. The cache STORE stays keyed by the raw config name. 2. CONFIG LOOKUPS RESOLVE ALIASES. New shared helpers in data-provider (buildServerNameAliases, normalizeMCPToolKey) map a parsed normalized name back to the raw config name that the registry, config maps, tool cache, and plugin-auth rows are keyed by. Applied in the definitions loader closure, handleTools grouping, createMCPTool's parsing fallback, getUserMCPAuthMap, and the MCP tools endpoint - matching both spellings so legacy raw keys keep resolving. 3. LEGACY DATA HEALS AT ONE BOUNDARY. initializeAgent rewrites raw-keyed agent.tools entries and tool_options keys to the normalized form (normalizeAgentToolKeys) before anything consumes them, so agents persisted under the old convention load their tools AND have all four per-tool options honored. Placeholder and server-pin tokens stay raw - they are config-identity references, not model-facing names. Servers whose names are already in the safe character set (the common case) produce byte-identical keys before and after; the fast path allocates nothing. Stale Redis-cached raw keys self-heal via the existing reconnect-on-missing path within one cache cycle. * 🧯 fix: Deterministic Alias Collisions + Raw Names in Definition Metadata Two review findings on the normalization contract: - Two configured server names that normalize to the same segment (e.g. 'Sales Force' and 'Sales:Force' -> 'Sales_Force') produce inherently ambiguous tool keys; the alias map silently resolved last-wins, so a tool selected from one server could execute against the other's config. buildServerNameAliases now resolves collisions to the FIRST configured name deterministically, and resolveMCPServerContext warns once per colliding pair per process so the operator can rename one server. A collision-resistant identifier would change every existing tool key, so detection + stable routing is the right treatment here; startup-time config validation can follow separately. - The definitions loader resolved parsed (normalized) server names to raw only inside the ToolService closure, while the definition metadata (serverName -> mcpRawServerName) kept the normalized value. Server instructions are keyed by raw config names, so a special-character server's instructions were silently omitted in definitions-only mode. loadToolDefinitions now takes rawServerNames, resolves the boundary against both spellings, and stores the RAW name in definition metadata - consistent with the instance path. * 🧯 fix: Heal Stale Caches, Skill Allowed-Tools, and Builder Selectors Three review findings on the normalization rollout, all in the transition class: - Stale cache entries (P1): the definitions-only loader treats the per-server tool map as authoritative and never reconnects on a per-key miss, so a pre-change raw-keyed Redis entry would make a special-character server's tools vanish for up to the cache TTL. getMCPServerTools now heals legacy raw-keyed entries to the normalized format at read time (keys and function names), covering every consumer with no coordinated invalidation; safe names return the map untouched. - Skill allowed-tools: a skill declaring a raw MCP key in allowed-tools bypassed the initialize-boundary heal (the union runs after it) and would neither dedupe against healed agent tools nor match the normalized tool map. The primes' allowedTools now pass through the same normalizeAgentToolKeys heal before unioning. - Builder selectors: matchesMcpServer and useVisibleTools parsed tool ids against raw server names only, so an attached special-character server rendered as an unselected orphan card. Both now accept the normalized spelling and resolve it back to the raw map key, keeping legacy raw ids working. * 🧯 fix: Fail Closed on Normalized Server-Name Collisions Escalation of the collision finding: a deterministic first-wins alias plus a warning still let the tools listing publish BOTH colliding servers, so a tool selected under the shadowed second server would silently execute against the first server's configuration (their model-facing keys are identical, so routing cannot ever distinguish them). - findShadowedServerNames identifies later-configured names whose normalized form an earlier different name claimed. - getMCPTools excludes shadowed servers from the published listing entirely (with a warn naming the collision), so their tools are never selectable - nothing ambiguous can be picked. - Server creation reserves both spellings: a generated slug may not collide with a raw config name OR the normalized form its tool keys would carry. Collision-resistant model-facing IDs remain out of scope: changing normalizeServerName's output would rewrite every existing tool key (agent documents, caches, instance names) for ALL servers to handle a misconfiguration that is now blocked from exposure instead. * ✅ fix: Dedupe Reserved Server-Name Spellings at Creation The reservation list appended normalized forms unconditionally, which duplicated every safe name (raw === normalized) and broke the route-level contract test pinning the exact list. Dedupe via a Set so safe names contribute one entry, while special-character names still reserve both spellings; adds the special-character reservation case. * 🧯 fix: Never Heal a Shadowed Server's Keys; Align Authorization Tie-Break Persisted references were the remaining collision vector: an agent or skill saved with the shadowed later server's raw key was HEALED into the shared normalized key, authorized through a last-wins map, and routed first-wins - authorized as one server, executed as another. - normalizeAgentToolKeys now refuses to rewrite keys of shadowed servers (findShadowedServerNames): rewriting would produce exactly the first server's key. Left raw, the key cannot match the normalized-keyed tool map and the tool fails visibly - broken beats misrouted. Covers agent.tools, tool_options, and skill allowed-tools through the shared heal. - filterAuthorizedTools (agents/v1.js) builds its normalized-to-raw map via the shared buildServerNameAliases instead of a last-wins Map constructor, so authorization resolves a colliding key to the SAME first server execution routes to. * 🧯 fix: Direct Identity Wins Over Aliases; Heal Client Forms and Degraded Contexts Four review findings on the normalization edges: - Alias hijack (P1): a user-DB server named exactly like an operator server's normalized form ('foo' vs YAML 'foo!') had its tools rerouted to the operator server by unconditional alias resolution. Resolution is now DIRECT-FIRST everywhere: the parsed name is tried as-is, and only when nothing resolves is it treated as a normalized spelling (definitions loader, handleTools grouping, createMCPTool fallback). buildServerNameAliases seats identity entries before derived ones so a literal name owns its slot regardless of config order, findShadowedServerNames and the collision warning derive from the same construction, and getUserMCPAuthMap fetches auth under both spellings so either owner finds its rows. - Builder double-match: a normalized name containing the delimiter ('foo mcp bar' -> 'foo_mcp_bar') also suffix-matched a server named 'bar', selecting both cards and making removal strip the wrong tool. matchesMcpServer now resolves the token ONCE against the full configured list (longest boundary, both spellings) when the caller supplies it; selection and removal share the resolution. - Builder legacy ids: an agent saved with raw-keyed ids showed its tools unchecked while the runtime heal kept them active, and selection updates never replaced the legacy entries. McpSection maps legacy raw ids to their current normalized ids when deriving and rewriting this server's selection. - Degraded context: a transient ensureConfigServers failure returned an entirely empty context, leaving normalized keys unresolvable for the request. resolveMCPServerContext now keeps the name lists (they derive from the config snapshot alone) and degrades only the lazy-init configs. * 🧯 fix: Collision Detection Sees Accessible Servers; Shadowed Refs Fail Closed End to End Round follow-ups on the collision design, all in the DB-server-visibility class: - The legacy-key heal detected collisions against operator-config names only, so healing could still produce a key that direct-first resolution routes to an invisible user-DB server. initializeAgent gains an optional getAccessibleMcpServerNames dep (wired through ToolService for controllers that mock it, directly elsewhere), consulted ONLY when a configured name needs normalization - zero cost for safe-name deployments. The heal then sees the full accessible set and skips shadowed servers' keys. - Wildcard and legacy raw tokens bypassed catalog filtering, letting a shadowed server's instances join a run under the same normalized names as the winner's. filterAuthorizedTools rejects tools of shadowed servers at authorization (its merged map sees DB + config), and handleTools skips them at execution. - The builder migrated only tool selection, not tool_options: legacy raw option keys showed disabled while the runtime honored them, and toggles could not clear them. McpSection now migrates option keys to the current normalized ids (existing normalized entries win). - A transient ensureConfigServers failure degraded to an EMPTY server context, leaving normalized keys unresolvable for the request. resolveMCPServerContext keeps the name lists (derived from the config snapshot alone) and degrades only the lazy-init configs. * 🧯 fix: Complete the Collision Audit at Every Gate; Safer Heal Semantics Round follow-ups hardening the collision audit: - Execution guards now consult the FULL accessible set: the caller's heal threads its already-fetched names through loadTools, and handleTools fetches them itself when a configured name needs normalization (never for safe-name deployments) - so a cross-tier collision (user-DB 'foo' vs operator 'foo!') fails closed at eager execution instead of joining the run under one normalized name. - Healing is SKIPPED when the collision audit cannot complete (transient lookup failure, or no dep): un-healed raw keys still resolve through the direct-first candidates, so skipping is safe while rewriting against an incomplete audit is not. - The audit lookup is gated on the agent actually carrying delimiter-bearing keys (tools, tool_options, or skill allowed-tools), so non-MCP agents never pay a registry round-trip even on specially named deployments. - normalizeAgentToolKeys gives the CURRENT (normalized) entry precedence when both spellings carry options, matching the builder's migration semantics instead of letting insertion order decide. - The builder's toCurrentToolId resolves entries boundary-exactly against every configured server (longest match, both spellings), so a raw suffix shared with a LONGER server name can no longer reassign that server's selection or options while another dialog is open. * 🧯 fix: Shared Collision Audit for Definitions Loading; Fail Closed on Audit Failure Round follow-ups closing the remaining audit gaps: - The definitions-only loader now consumes the same collision audit as eager loading: shadowed servers' entries (wildcards included) are dropped before definitions are emitted, so the default execution path can never resolve a shadowed server's normalized function name to another server. The audit names thread from initializeAgent's heal; the loader self-fetches only when a configured name needs normalization. - resolveCollisionAuditNames centralizes the audit-resolution policy (threaded set > self-fetch when needed > incomplete on failure), and BOTH loaders now fail closed under an incomplete audit: any normalization-sensitive reference (its own name needs normalizing, or it equals the normalized form of a configured special-character name) is skipped with a warning instead of being audited against operator names alone. isNormalizationSensitiveName lives in packages/api as a pure helper so test mocks use the real predicate. - normalizeAgentToolKeys collapses duplicate ids after healing (order-preserving): a document carrying both spellings converges on one key, never two instances with the same function name. * 🧯 fix: Thread the Audit Everywhere; Identity-Aware Alias Fallback Round follow-ups on audit plumbing: - The OpenAI-compatible and Responses tool loaders now forward the already-resolved accessibleMcpServerNames instead of discarding it, so the definitions loader neither repeats the registry lookup nor fails closed on a transient second lookup after the first succeeded. - The skill-only path threads its audit: when the baseline agent has no MCP keys but a primed skill's allowed-tools fetched the complete set, that set (not the operator-only list) reaches the loader, so the collision remains visible and the shadowed reference stays rejected end to end. - OAuth discovery iterates the collision-FILTERED tool list, so a request can no longer emit an OAuth prompt, wait out the connection timeout, and reconnect a server whose definitions were deliberately rejected. - The definitions loader's alias fallback is identity-aware: when the parsed name IS a known accessible server, a null tool fetch means temporarily unavailable (OAuth pending, missing user variables, disconnected) and no longer reroutes to the raw alias - previously the aliased operator server's definitions could be emitted under the unavailable DB server's names. * 🧯 fix: Legacy-Key Definition Lookup; Retain Audit for Deferred Execution - createMCPTool resolves tool definitions by BOTH spellings: the key as persisted plus the canonical normalized key built from the resolved server name. Assistants and direct tool calls persisted before the rollout bypass the agent-boundary heal and arrive with raw keys, while availableTools is now indexed canonically - previously every such call missed the index, burned a reconnect, and returned the unavailable stub permanently via the negative cache. - The initialized agent retains accessibleMcpServerNames (the COMPLETE collision audit this initialization resolved), buildAgentToolContext copies it into every per-agent tool context, and loadToolsForExecution threads it into the eager loader as bare options. Deferred/event-driven execution therefore reuses the snapshot instead of repeating the merged registry read - a transient failure there could fail-closed a tool the same turn already advertised from the successful first audit. - MCP.spec.js keeps @librechat/api pure helpers REAL (requireActual spread) so normalization paths are exercised rather than mirrored. * 🧯 fix: Parse Legacy Keys Against Both Server-Name Spellings createMCPTool's boundary candidates were normalized-only, so a legacy raw key whose server name contains the delimiter (foo_mcp_bar!) missed the suffix match and fell to the generic last-delimiter split - the canonical rebuild then produced a key that could never hit the index and the persisted call stubbed out. The candidate list now carries the RAW resolved name (and raw config names on the parse-only path) next to the normalized spellings. * 🧯 fix: Honest Audit Completeness; Shadowed-Server Form-Key Guard - resolveAllMcpConfigs tolerates ensureConfigServers failures, so the merged registry read can silently omit config-only servers while the audit still reported complete: true - a foo/foo! collision would go unseen and a persisted key could route to the wrong server. Both audit consumers now union the snapshot-derived raw config names back in (resolveCollisionAuditNames unions the caller's rawServerNames; the initializeAgent heal unions configRawServerNames), keeping the completeness label honest without an extra read: operator names come from the registry-independent config snapshot, user-DB names from the merged read that fails loudly into the existing incomplete path. - The client tool_options migration now mirrors the runtime heal's fail-closed rule for SHADOWED servers: when the dialog's server has lost its normalized slot to another catalog name, legacy raw keys stay raw instead of being rewritten onto the winning server's key, where a later save would apply the wrong server's per-tool settings. The dialog's own server joins the alias construction so a stale catalog map can't misread as a collision. * 🧯 fix: Heal Legacy Assistant MCP Tool Names on Save The assistants create/update controllers look tools up in the cached definitions by exact key, and the cache is now normalized-keyed - an assistant saved before the convention resubmits its raw-suffixed MCP name on every edit, so any save silently removed the tool. healMcpToolNames pre-heals the payload's tool list: a delimiter-bearing string that misses the cache resolves through the configured raw names (longest-suffix, boundary-exact) and rewrites to the normalized key only when that key actually exists in the cache. SHADOWED raw names stay raw and fail closed, mirroring the runtime heal; the config read happens only when a delimiter-bearing name actually misses, and read failures propagate (write path) rather than silently dropping tools. v2's update loop also stops re-reading the tool cache per iteration. * 🧯 fix: Full-Audit Shadow Set + Dedupe in the Assistant Key Heal - The assistant-save heal built its shadow set from operator config names alone, so a cross-tier collision (user-DB `foo` owning the normalized slot of operator `foo!`) looked unshadowed and the legacy key healed into the shared normalized name - which direct-first execution then binds to the DB server. The shadow set now comes from resolveCollisionAuditNames' full accessible audit, and an incomplete audit skips healing outright (every rewrite candidate is normalization-sensitive by construction, so raw-and-fail-closed is the only safe answer). - Healed string entries dedupe order-preserving: a payload carrying both spellings of the same tool collapses to one entry instead of expanding into duplicate function definitions the provider rejects. |
||
|
Danny Avila
|
e7f1838515
|
⚡ feat: Reliable Interrupt & Steer Escalation and Recovery (#14558)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* feat: surface interrupt-steer escalation on waiting messages The interrupt & steer feature shipped reachable only through the composer chord, the send-button hovercard, and the composer button; a message already waiting (queued for after the run, or steered and parked at the next tool boundary) had no path to it. Both waiting surfaces now carry one: - Queued rows get an icon-only ZapOff escalation button beside the existing Steer primary. It routes through sendQueuedNow, which now takes a preempt option on its live-run path. The tooltip teaches the composer chord, derived through resolveComposerKeyDown so a rebound or yielded chord is never advertised. - In-flight steer bubbles get an "Interrupt now" overflow entry with the same race rules as Edit: reclaim first, and only a `reclaimed` outcome resubmits (via retrySteer with preempt, swapping the chip for an interrupting one). `applied` and run-ended-mid-reclaim outcomes stop at the existing informational toasts, so the words can never land twice. Not offered on a steer already preempting. - Every during-run overflow menu gains an "Always interrupt instead" toggle for steerInterruptsByDefault, next to the existing steer/queue default toggle. MenuEntry supports disabled for the new entries. Only one interrupt can be unresolved at a time: while one preempt is pending (or the run is paused on approval, where the server 409s), every escalation control disables instead of racing the same seal. Ten new tests across both surfaces; 381 green in the affected suites. * fix: lock escalation across its reclaim window, keep the paused control visible, label as steer Codex round 1, all three findings. P2, escalation race. The single-interrupt invariant had a window between clicking "Interrupt now" and the reclaim resolving, where no preempt chip existed for the chip-derived gate to see: two bubbles escalated back-to-back could both resubmit. A shared escalating flag (Jotai, per-conversation) now covers the window and disables every escalation control on both surfaces, and a fresh recheck before resubmitting catches an interrupt armed elsewhere meanwhile (composer chord, queued row); those words re-home to the queue with an informational toast instead of breaking the invariant. P2, unreachable paused state. canSteer is defined as hasRealConvoId && !pausedOnApproval, so gating the button on canSteer removed it exactly when it was meant to render disabled; the test only passed on an impossible stub combination. The render gate is now duringRunActive && (canSteer || pausedOnApproval), and the test uses the real invariant. P2, label semantics. "Interrupt & send now" borrowed the name of the hard-abort action; this one preserves the partial answer and steers. Renamed to "Interrupt & steer now" (com_ui_interrupt_steer_now). Both behavior fixes counterfactually verified; 384 tests green across the affected suites. * fix: disable bubble escalation while the run cannot accept a steer Codex round 2, one P2. Answer mode (ask_user_question) sets duringRunActive false while pausedOnApproval stays false, since that flag only detects approval-bearing tool calls. The bubble's escalation entry stayed enabled there, so clicking it cancelled a healthy waiting steer and the preempt resubmission bounced off RUN_PAUSED, degrading the words to the queue. The entry now also disables on !duringRunActive, matching the queued-row control's gate. Counterfactually verified: reverting the gate fails the new answer-mode test. * fix: recheck live run state after the reclaim, not just at the click Codex round 3, one P2, and it is the round-1 recheck principle applied one level deeper: the entry-time disable cannot see a run that pauses (tool approval, answer mode) while the reclaim round-trip is in flight, and the .then closure held the render's stale steering controls, so the resubmit would fire into a RUN_PAUSED rejection after the reclaim had already surrendered the steer's boundary slot. The escalation continuation now reads the LIVE controls through a latest-ref: if the run can no longer accept a steer, the words re-home to the queue with an informational toast instead of resubmitting, and the resubmit itself also goes through the live controls. Counterfactually verified: reading the stale closure instead of the ref fails the new mid-reclaim pause test. * refactor: make escalation one atomic server-side arm, in place Codex round 4: four P2s, every one an interleaving of the same window — escalation as reclaim-then-repost is a compound, non-atomic operation whose continuation must revalidate the world (FIFO position lost, ref assigned too late, no run fence, competing bubble actions). Rounds 1-3 patched that window with a lock and rechecks; round 4 shows the window itself is the defect, so this removes it instead of guarding it again. Escalation is now POST /chat/steer/arm: the server flips preempt on the EXISTING queued item in one atomic store op (new IJobStore.armSteer; a decode-patch-encode LSET Lua on Redis, an in-place mutation in memory), fenced to the validated generation and refused once the queue closes. The handler mirrors the steer POST's preempt contract exactly: durable flag gated on the owner's recorded capability, volatile requestPreempt fire-and-forget because the durable flag is the truth resume/handover re-arm from. By construction this resolves all four findings: FIFO survives (the item never moves; the whole queue still drains in instruction order at the seal), there is no continuation to hold stale controls, the store op is fenced to the original run, and a competing Edit/Queue/Cancel either beats the arm (armed:false, chip untouched) or operates on the armed item, whose cancel already disarms. The client escalation entry becomes one mutation: armed:true relabels the chip in place (same steerId, same position), PREEMPT_UNSUPPORTED and lost races toast honestly, and the round 1-3 machinery — the escalating lock atom, the latest-ref, the post-reclaim rechecks and their two toast strings — is deleted rather than extended. Verified: 7 new handler tests on the real in-memory manager (including FIFO preservation and the stale-generation fence), 2 Redis integration tests against real Redis (in-place arm keeps order and every field; missing/stale/closed all refuse), client suites 396 green. * fix: decide capability inside the atomic arm, neutralize the lost-race toast Codex round 5, both findings, both edges of the new arm design rather than its mechanism. P2, capability TOCTOU. A HITL resume on a rolling deploy rewrites preemptCapable for the SAME generation, so the handler's read could go stale between validation and the flag flip, arming a steer the live owner cannot seal. armSteer now returns armed | missing | incapable, with the owner's live capability part of the same atomic predicate as the generation fence (HGET preemptCapable inside the Lua; the flat job field, not a metadata blob — the in-memory store reads the same field). The handler's pre-check is deleted rather than kept alongside; the store predicate is the single source. New handler test rewrites the capability after queueing and expects PREEMPT_UNSUPPORTED with the item left unflagged; the Redis guards test now asserts the incapable refusal against real Redis. P2, ambiguous toast. armed:false covers injected, cancelled, re-homed, and run-over alike, so telling the user the message "already reached the agent" claimed one specific outcome. The lost-race branch now uses a neutral message (com_ui_steer_arm_lost_race) and defers to the events for what actually happened. * fix: flip the escalation lock synchronously before the arm request Codex round 6, one P2. Round 4 deleted the escalating flag along with the reclaim continuation it guarded, but that left the one-interrupt gate blind during the arm request's own round trip: the chip-derived check cannot see an arm until its response relabels the chip, so on a slow connection two bubbles could both arm before either response landed. Double-arm is harmless server-side now (the run seals once and drains the whole queue in order), but every escalation control advertises "one interrupt at a time" by disabling, and the controls must tell the truth. The per-conversation escalating flag returns as a pure UX gate: set synchronously at click, before the mutation, cleared on settlement, and folded into interruptPending on both surfaces. Unlike its round 1-3 ancestor there is no continuation behind it to guard and no recheck to pair with it. Counterfactually verified: without the synchronous set, the two-bubble race test arms twice. 207 tests green across the Chat Input suites. * test(e2e): cover escalation of waiting messages through the real seal Three mock-harness tests on E2E_SLOW_REPLY, a 160-chunk stream with no tool boundary, so an in-thread steer part can ONLY come from a genuine mid-stream seal — which makes each test a behavioral proof rather than a UI check: - Queued row escalation: the ZapOff button turns a waiting queued message into a preempt-armed steer (202 echoes preempt: true) that seals and injects, where the sibling steering.spec test proves the unescalated path waits for run end instead. - Bubble in-place arm: an ordinary steer (202 with no preempt echo) waits as a bubble, POST /chat/steer/arm answers armed: true, the bubble relabels in place (same single bubble, same text, escalation no longer offered on reopen), and the armed steer seals mid-stream. - Always-interrupt toggle: flipped from a waiting row's overflow menu, plain Enter now produces a preempt: true steer that seals in the SAME run, and the menu offers the way back. An afterEach clears the localStorage preference so a mid-test failure cannot leak preempt-by-default into the rest of the serial suite. All three verified locally through the full harness (real backend, mock LLM, seeded DB): 3 passed in 27s. * feat: dedicated escalation arrow + shortcut, menu split into actions and preferences The escalation was still half-hidden: the bubble only offered it inside the overflow menu, and the tooltip taught the composer chord, which does a different thing (interrupts with typed text, not this chip). Three changes make it a first-class command: - A shared EscalateNowButton (circular arrow, ghost-bordered like the composer's interrupt control) is always visible on BOTH surfaces: beside each queued row's Steer primary and on every waiting steer bubble next to its menu. It disappears once a steer is interrupting. - A dedicated registry shortcut, escalateSteer (Cmd/Ctrl+Shift+.), editing-allowed and rebindable like every other action. Deliberately NOT an Enter chord: the composer owns every Enter chord, and the yield design rests on no default binding using Enter besides submit. Its handler clicks the newest enabled arrow control (bubbles beat queued rows), so the shortcut can never diverge from the button, and the arrow's tooltip teaches THIS command via the registry display. - The overflow menus separate one-off actions from sticky behavior changes: Edit, Cancel, Queue, then a smaller "Preferences" section holding the queueing and always-interrupt toggles, each with the standard InfoHoverCard reusing the Settings panel's descriptions. "Interrupt & steer now" leaves the menu entirely. 386 client tests green, including a menu-structure test locking the order and the absence of the escalation entry; bubble escalation tests drive the visible arrow. The e2e spec's bubble test now clicks the arrow, and a fourth test drives the dedicated shortcut end to end through a real mid-stream seal. * style: bind the escalation arrow to its message (variant A anatomy) Two same-weight circles in a row read as one control group, leaving the arrow's ownership ambiguous, and a floating arrow stops meaning anything once several messages stack. The shared control now carries variant A's anatomy: a thin divider binds a small SOLID arrow (filled, inverted) to the message region on its left, and the menu ellipsis stays a bare glyph, so the two affordances can no longer blur together — and the divider+arrow pairing repeats cleanly per chip at N messages. * chore: drop the unused within import CI lint caught * fix: advertise the escalation shortcut only while the control is live Codex on the e2e head, one P2: the tooltip appended the chord hint even while the button was disabled, advertising a shortcut that does nothing during an approval pause. The flagged control (InterruptNowButton) was since replaced by the shared EscalateNowButton, which inherited the pattern; the successor now omits the chord whenever the control is disabled, matching the rule the during-run hovercard already follows. * fix: harden steer escalation lifecycle and recovery * test(e2e): disambiguate accessible steer preferences * test: align abort persistence coverage with prerequisites * chore(i18n): remove obsolete steer race message * chore: normalize imports across steering changes * test: exercise stream integration on Redis Cluster * test: scope HITL checkpoints to generation * test: fix cluster cleanup and locale policy * fix: keep escalation visible during ask pauses * fix: fence recovery downgrade and stale predecessors * fix: require generation owner abort acknowledgement * fix: validate delayed preempt arms * test: align final escalation fixtures * fix: preserve in-memory predecessor abort handoff * fix: restore controls for recovered queued messages * test: cover recovered queue controls * fix: close final steering review gaps |
||
|
Danny Avila
|
60ca751a7f
|
🧠 fix: Preserve Deferred Tool Schemas Across HITL Resume (#14552)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🧠 fix: Preserve deferred tool schemas across HITL resume * 🧪 test: Harden deferred tool resume regression * 📦 chore: bump @librechat/agents to v3.3.10 |
||
|
Danny Avila
|
1e1de6eff9
|
🎯 fix: Exact Ask-Question Attribution via Interrupt tool_call_id (#14539)
The ask_user_question pause/answer stamps (server pause-time args stamp, resume-time answer stamp, and the client mirror) targeted the newest unanswered ask part by position. When a model emits several ask calls in one turn, the interrupt's question and the user's answer land on the wrong card. @librechat/agents > 3.3.8 surfaces the interrupting call's tool_call_id on the ask interrupt payload. All three stamps now target that id exactly when present, keeping the positional fallback for older payloads. The tool body passes config.toolCall.id through to askUserQuestion via a typed alias that is a no-op on the pinned SDK and lights up on the next dependency bump. Companion to danny-avila/agents#366, which also fixes the underlying dangling tool_use 400 (INVALID_TOOL_RESULTS) when one of the parallel ask calls streams malformed args. |
||
|
Danny Avila
|
7bb6651883
|
🛑 feat: Preemptive Steer - Backend Interrupt & Steer (#14518)
* 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3) Lets the steer route ask the generating replica to seal its live model stream at the next provider-safe boundary instead of waiting for a tool step. The run is never aborted, job status never changes, the partial answer is kept, and generation resumes in the same assistant message after the injected steer. Consumes the SDK seam in @librechat/agents (danny-avila/agents#335, #346). Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair beside abort. RedisEventTransport fans PREEMPT out on the SAME events channel and subscription (no new connection, key, or subscribe call); onPreempt returns a registration-scoped unsubscribe with the same replacement-safe state-identity guard onAbort uses. InMemory implements neither — single-process preempt lives entirely in the runtime set. Runtime state: RuntimeJobState carries the per-generation request set, createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded `cleared` tombstone so a late cross-replica arm cannot resurrect a request whose steer already drained. registerPreemptSubscription mirrors the abort registration's double fence (runtime identity + generation createdAt); releaseAbortSubscription retires BOTH listeners and the armed set, so every terminal path drops preempt state for free. Public surface: requestPreempt (arm + fenced publish, never a rejection surface, never touches job status), isPreemptRequested (O(1) level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping + fenced clear), clearPreemptRequests (empty-boundary disarm). One drain body, two boundaries: createSteerDrainHook (PostToolBatch) and createSteerPreemptBoundaryHook (PreemptBoundary) share drainAndBuildInjections, so the two injection sites cannot drift — the SDK's provider-safety argument rests on identical HumanMessage shapes. The shared body builds injections incrementally under a swallow-all catch (a mid-loop throw still injects what was applied — those parts are already persisted), clears preempt requests in finally, and disarms the generation when a boundary drains nothing. Request path: POST /chat/steer accepts preempt: true. The guard ladder is unchanged in order and in every status code. A preempt request is NEVER a rejection reason — without the capability the steer still enqueues and the 202 echoes preempt: false. Armed strictly after a successful enqueue; cancel disarms. The capability is read from the OWNING replica's recorded `preemptCapable` rather than the route replica's own SDK probe, so a rolling deploy cannot label a steer "interrupting" that the older owner will only inject at a tool step. Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a parked/claimed/replayed chip keeps its wording. Run wiring: createRun registers the PreemptBoundary hook and threads RunConfig.preemption, both gated on isSteerPreemptSupported() — a separate probe from isSteeringSupported(), so the client affordance can never arm against an SDK that only injects at tool boundaries. buildSteerWiring builds both hooks from one shared closures object, so preemption survives HITL pause/resume for free. Honest finalization: an empty preempt boundary persists and emits with unfinished: true — the same contract an abort gets — re-marked explicitly because BaseClient has already saved the row as unfinished: false by that point. Not changed: no new job status, store method, Lua, SSE event type, endpoint, or authorization surface. abortJob, completeJob, transitionStatus, closeAndDrainSteers, getResumeState, emitChunk, applySteerPart and the whole abort path are untouched. Tests: 120 packages/api steering specs (preempt lifecycle, tombstone, fences, caps, terminal release, both-boundary drain parity, level-triggered poll, request/cancel arming, owner-capability degradation) plus 5 in api for buildSteerWiring gating, and 2 Redis-gated cross-replica transport specs. * 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes All four server findings were fresh consequences of the round-1 fixes, which is the review doing exactly what it should. - Tombstone cap refused new entries instead of evicting. Every drained or cancelled steer is tombstoned, not just preempting ones, so a generation that processed 20 steers exhausted the set and the late-arm race resurfaced silently. Now evicts oldest-first (Set iteration is insertion-ordered), with the budget named PREEMPT_TOMBSTONE_MAX rather than an inline expression. - The empty-boundary disarm I added in round 1 wiped the generation's ENTIRE armed set. A second steer can enqueue and arm between the atomic drain returning empty and the disarm running — that arm is backed by a live, uninjected queue item and must survive. The drain now snapshots the armed ids BEFORE draining (getArmedPreemptIds) and clearPreemptRequests takes an explicit id list instead of clearing everything. - HITL resume finalized with a hardcoded unfinished: false. The boundary hook is re-registered on resume via buildSteerWiring, so a resumed segment can end on an empty preempt boundary exactly like a fresh one; finalizeResumedTurn now reads getPreemptStats() and the halt reason, matching the normal request path. - Ownership moves on resume, so the job's recorded preemptCapable must describe the replica that will actually generate. Refreshed before resumeCompletion; a job created on a capable replica that resumes on an older one during a rolling deploy no longer acknowledges steers as interrupting. Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first tombstone eviction, id-list disarm). 122 packages/api steering specs green. * 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis) The P1 here is the most consequential defect in the whole feature, and it was introduced by round 1's own capability fix. - `RedisJobStore.serializeJob` writes booleans generically, so `preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT field map and had no line for it. Every `getJob()` therefore dropped the flag, `job.metadata.preemptCapable` was always undefined, and `handleSteerRequest` computed `preemptArmed: false` unconditionally. Interrupt & steer would have silently degraded to ordinary tool-boundary steering in EVERY Redis deployment — i.e. the feature shipping as a no-op in production while passing every in-memory test. Now deserialized, with a round-trip assertion in the metadata spec that fails (`Received: undefined`) against the unfixed store. - The resume capability refresh moved from just-before `resumeCompletion` to immediately after `approvals.resolve` claims the run. That call already flips the job back to `running`, so the steer route accepts requests from that instant; leaving the refresh 135 lines later (across the whole client reconstruction) left a real window where a steer read the PREVIOUS owner's capability. Not the fully atomic transition Codex suggested — that reaches into the approvals Lua — but it shrinks the window from seconds to one await, which is proportionate for a label-accuracy issue. Refuted: "avoid triggering preemption inside subagents". The premise — that the run-wide poll can seal a subagent stream — does not hold against the shipped SDK. Child graphs are constructed with `subagentScope: true` (SubagentExecutor) and `preemption` is NOT propagated into child inputs, while `canClaimPreemptSeal()` requires `!subagentScope && preemption != null`. Both conditions fail independently, so a subagent can never claim a seal and the boundary cannot fire with `agentId` set. The `input.agentId != null` guard in the hook is defensive depth, not the thing standing between us and the described failure. 140 packages/api specs green. * 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners - An arm lives only in the owning replica's runtime plus a transient pub/sub message, while the steer's `preempt` flag is durable on the queue item. A HITL resume landing on a different replica therefore started with an empty armed set and a poll stuck false, so an interrupt the user had already been ACKed for silently waited for an ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts` rebuilds the armed set by peeking the durable queue (fenced on the generation) and re-arming every item flagged `preempt`; resume calls it right after claiming. Safe by construction: every item peeked is still queued, so no drained steer can be resurrected. - Capability-refresh failure now logs at error rather than warn, but deliberately does NOT fail the resume — see the reply on that thread. Tests: +2 (rebuild from queue arms only the flagged item and reports the count; a stale generation arms nothing). 124 packages/api steering specs green. * 📡 fix: Codex round 5 — acknowledge only what was actually armed - A cross-replica arm was fire-and-forget: `emitPreempt` logged its own publish failure and `requestPreempt` returned void, so the route answered `preempt: true` even when the owner never armed a poll. The steer still injected at the next tool boundary, but the chip claimed an interrupt that could not happen — and unlike HITL resume, an ordinary running generation had no durable reconciliation to recover it. `emitPreempt` now resolves to the subscriber count and rejects on failure; `requestPreempt` is async and returns whether the arm truly landed (owned locally, or delivered to at least one subscriber). The 202 reports THAT rather than what was asked for, so the chip relabels to ordinary steering exactly as it does for a capability-degraded deployment. Errors are swallowed into `false` — an unarmed interrupt is a downgrade, never a failed steer. - The owner capability is re-read immediately before enqueue rather than reused from the top of the guard ladder. `checkAgentAccess` and file resolution are awaits, so a request can span an entire HITL pause/resume that moves ownership to a replica with different capability and rewrites that very flag. Only paid for by requests that actually asked to interrupt. Tests: +3 (not-armed when the publish reaches nobody; armed when this replica owns the generation; a throwing publish downgrades instead of propagating). 127 packages/api steering specs green. * 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own Three review findings plus three CI failures the round-5 commit caused. Review: - Ownership came from `runtimeState`, which a cross-replica `getJob` populates with a FACADE runtime on any replica that merely read the job. Matching `createdAt` therefore proved only "we looked at this job", so a non-owner could arm nothing and report success. Ownership now comes from `ownedJobs`, the actual owner map. - `armPreemptIds` returns how many ids it accepted, and a local arm is only reported as armed when one was. A tombstoned id (its steer drained at an ordinary boundary mid-request) no longer answers `preempt: true` for an interrupt that cannot happen. - The cancel disarm is awaited. A dropped clear is worse than a dropped arm: the owner keeps a level-triggered request for a steer that no longer exists, seals its next chunk and truncates an unrelated answer. The boundary drain's own call stays non-blocking — there the owner is local, so the disarm is already effective and awaiting the informational publish would only delay injection. - Subscriber count is NOT read as proof of owner receipt: the count includes this replica's own facade subscription. A successful publish reports armed, a rejected one does not. Documented rather than papered over — see the acknowledgement-semantics note on the PR. CI regressions from round 5, all mine: - `registerPreemptSubscription` was AWAITED at both runtime-init sites, so job creation blocked on a second Redis channel subscription and hung when that subscribe was slow. Abort is awaited because a missed abort strands a run; a missed preempt only degrades that steer to the next tool boundary, so it now registers without gating createJob. - Two api specs mocked `@librechat/api` without the newly imported `isSteerPreemptSupported`, so the call threw before createJob; and one exact-match assertion needed the new `preemptCapable` metadata field. - My own Redis integration spec asserted arm-before-clear ordering, which two publishes carry no guarantee of — the receiving tombstone exists precisely because of that. Now asserts delivery and payload fidelity, order-independent. 158 packages/api specs, 27 api specs green. * 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A) Round 7's second finding is the incoherence I flagged on the PR: the route persisted `preempt: true` on the durable queue item while returning `preempt: false` when delivery could not be confirmed. Those two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one — so a resumed owner would honour an interrupt the client had explicitly been told degraded to ordinary steering. Rather than patch the disagreement, this settles the meaning: `preempt` in the 202 means "queued as an interrupt request", NOT "a seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so the response, the durable record, and the resume-time re-arm can never disagree. The gates that ARE knowable stay — the owner's recorded capability and a successful enqueue. Everything past that degrades to the documented fallback of injecting at the next tool boundary. A route cannot synchronously know whether another replica will seal: proving it needs a correlated request/response over pub-sub, and even that only proves the owner heard, not that it is still streaming when the arm lands. Four rounds of tightening this boolean each surfaced a narrower case; the sequence does not converge, so the invariant is now "the flag describes the durable decision" and an unconfirmed arm logs a warning instead of rewriting the answer. Also from this round: a failed disarm publish is retried once and its outcome reported. `handleSteerCancel` keeps `removed: true` — the steer really did leave the queue, and saying otherwise would make the client re-show a chip for a steer that can never arrive — and adds `disarmed: false` so the residual risk is visible rather than swallowed. Damage stays bounded regardless: the empty-boundary self-clear disarms the generation after a single seal. Tests: +1 pinning the response/durable-flag invariant. 159 packages/api specs green. * 🧹 fix: Codex round 8 — remove the unverifiable disarm signal Round 8 found the same over-promise on the disarm side that round 7 corrected on the arm side, so this applies the same answer rather than patching around it. The `disarmed: false` field added in round 7 was both unreliable and unused: a resolved publish is not proof the owner heard it (the delivery count includes this replica's own facade subscription), and it was never threaded into `CancelSteerResponse` or read by any client. A signal that claims a certainty the transport cannot provide is worse than no signal — it invites callers to trust it. Removed from the response. The retry stays, because it genuinely reduces the failure rate, and `noteSteersRemoved` still returns whether the publish succeeded FOR LOGGING, now documented explicitly as "published without error", not "the owner disarmed". Disarm is best effort with a bounded, self-healing failure: if the clear is lost the owner seals once, the empty-boundary self-clear disarms the generation, and the turn is persisted `unfinished: true` rather than silently truncated. Tightening that further needs a correlated request/response over pub-sub with a timeout — noted on the PR as the deliberate boundary of this design rather than an oversight. 130 packages/api steering specs green. * 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too The round-6 scoping fix only cleared the pre-drain snapshot when the drain came back EMPTY. On a nonempty drain the `finally` cleared just the drained ids, so a stale arm — typically a cancel whose cross-replica clear was lost — survived the boundary. It would then immediately seal the continuation meant to answer the steer that had just been injected, and land on an empty boundary as `preempt_incomplete`: the interrupt appears to work, and the answer to it is truncated. A boundary that runs has spent its seal, so everything armed at snapshot time is spent whether or not it came back from the drain. The `finally` now clears the union of the snapshot and the drained ids. Arms that land AFTER the snapshot are still spared — their queue items are live and uninjected, which is the property round 6 added. Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs `GenerationJobManager` wholesale, and the round-3/4 resume work added two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not define, so 34 specs threw. Stub extended. Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty drain spares an arm that landed mid-drain). Counterfactually verified — the stale-arm spec fails against the unfixed drain. 132 packages/api specs, 60 resume specs green. * fix: never let a failed preempt subscription reject into the void registerPreemptSubscription is called detached at both sites, so a rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal under Node's default --unhandled-rejections=throw. The comment already promised this path merely degrades steering; it now does. Swallowed and logged inside the registration rather than at each call site, so a future third caller cannot reintroduce the trap. Losing the channel costs this generation's cross-replica preempts, not the server: same-replica arming is runtime state and still works, and remote arms fall back to the next tool boundary. Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an unhandled rejection against the unfixed registration. * docs: state the real blast radius of a failed preempt subscription LibreChat's own entrypoints install a global unhandledRejection handler that logs and keeps serving, so the escaping rejection this guards was never fatal to this server — only to another consumer of @librechat/api that installs no handler. The fix stands either way; the comment just should not overstate what it prevents. * test: cover the cross-replica preempt hop with two manager instances Every other preempt test runs against a single manager, so the hop that actually carries an interrupt in production had no coverage: the steer POST lands on whichever replica the balancer picks, which is usually not the one generating. Non-owner publishes, owner arms, owner's level-triggered poll flips — none of that was exercised end to end. Two GenerationJobManagerClass instances are a faithful replica pair here. runtimeState and ownedJobs are private instance fields, there is no module-level mutable state between them, and createStreamServices duplicates a dedicated subscriber connection per call, so separate OS processes would exercise the same objects over the same Redis. Both assertions verified counterfactually against real Redis: - Deleting the preemptCapable deserialization in RedisJobStore fails this with 'Expected: true, Received: undefined' — the exact P1 that shipped past every in-memory test and would have made the feature a silent no-op on every Redis deployment. - Dropping the non-owner arm publish fails it with 'Received: false'. * test: remove the fixed sleeps and vacuity from the cross-replica preempt test Codex round 11, both findings, both on the test I added last commit. P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land before anyone was listening and the test would fail against correct code. Now it republishes until the owner's state converges, which is safe because arms and clears are idempotent set writes keyed by steerId. Side effect: the tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on delivery rather than on a timer. P3 — afterEach destroyed only the transports, leaving each manager alive in its own cleanup-interval closure, still working against a dead transport. Now tracks the managers and awaits destroy(), which disposes the job store and its timer too. Matches how the rest of this file cleans up. Fixing the sleeps exposed a third problem codex did not flag: the stale-arm test could pass vacuously, because an undelivered arm and a fenced one look identical. It now brackets the stale publish between two control arms — the first proves the owner is listening before the stale one is sent, the second proves it has had its chance to arrive. Verified counterfactually against real Redis, and stable over 5 runs: - dropping the preemptCapable deserialization fails with 'Received: undefined' - dropping the non-owner arm publish times out both tests - removing the generation fence fails the stale test with ["control-before", "steer-stale", "control-after"] — which also confirms the bracketing orders as intended rather than by luck * fix: gate interrupt on the OWNER's capability alone, not the route's Codex round 12. The comment above this gate already said 'the OWNER's recorded capability, not this replica's probe' — and then the code ANDed in isSteerPreemptSupported(), which is exactly this replica's probe. The contradiction dates to the original commit; round 6 made the gate owner-scoped and wrote that comment without removing the local conjunct. The route never seals. It enqueues and publishes an arm, neither of which touches the SDK, so during a rolling deploy a steer landing on an un-upgraded replica silently lost its interrupt even though the owner could seal. When the route IS the owner the probe is redundant anyway: the flag it would consult is the one this process wrote at createJob. The real degradation path is unchanged and still tested — an owner that recorded no capability relabels to an ordinary steer. The test that pinned the local probe asserted an impossible same-replica state (capable metadata plus an incapable local SDK, when the metadata is written from that probe); it now pins the mixed-SDK direction instead, and fails with 'Expected: true, Received: false' if the probe is put back. * fix: reconcile arms at handover, and stop holding the 202 on a publish Codex round 13, two of three findings. P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job still installs a facade runtime and subscribes, so it can accept an arm and then miss the best-effort clear that follows the drain. HITL resume promotes that facade to owner, the union keeps the orphan, and the first resumed stream seals on a steer no longer in the queue, drains nothing, and truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership only sets ownedJobs, so nothing else was clearing it. The durable queue is the sole authority at a handover: arms it does not back are now disarmed and tombstoned, so an in-flight publish cannot revive them either. Worth recording that my own independent review raised this and my verifier refuted it. Codex found it separately; two reviewers converging should have outweighed one refutation. P2 — the route awaited the arm publish before answering. The 202 reports capability, not delivery, so the await could not change the response; it only exposed the caller to Redis latency after the queue item was already durable. A client that times out and retries mints a second steer while the first stays queued, injecting the same instruction twice, whereas a lost publish merely takes the tool-boundary fallback. Detached, with both outcomes logged. All three tests verified counterfactually: union-only rearm fails the two new handover specs, and re-awaiting the publish hangs the stalled-publish spec until jest kills it. * fix: snapshot arms before reading the queue at handover Codex round 14 — a regression from my own round-13 fix, and a worse failure than the one it corrected. Round 13 read the durable queue first, then tombstoned any armed id the snapshot did not back. But approvals.resolve reopens steering before reconciliation runs, so another replica can commit a preempt steer and publish its arm while the peek is in flight. That arm is then present locally but absent from a snapshot taken before the steer existed, so a LIVE interrupt the route already acknowledged got dropped — and tombstoned, which blocks the re-arm, making it unrecoverable rather than merely late. Fixed by inverting the two reads rather than by locking or paying a second round trip. A steer is durably enqueued BEFORE its arm is published, so any id in an arms-first snapshot was already queued when it was armed, and the later peek must observe it unless it has since drained — which is exactly the orphan this reconciliation exists to drop. Arms landing after the snapshot are simply not candidates. Also re-checks runtime identity across the await, since the generation can be replaced while the queue read is in flight. New spec injects a steer + arm during the peek and verifies it survives; against the round-13 ordering it fails with Received array: []. * fix: bound the cancel disarm wait and fence enqueue to its generation Codex round 15. P2 — the cancel awaited its disarm publish unbounded. ioredis queues commands during an outage rather than rejecting, so that await could hang for the length of the outage with the steer ALREADY durably cancelled; a client that gives up then treats the cancel as failed and restores a chip for a steer that can never produce an applied event. Every successful cancel publishes, so ordinary steers were exposed too, not only preemptive ones. Now bounded at 1s, with the publish continuing behind it — its retry and logging are unchanged, it is just no longer in front of the response. This is the sibling of round 13's arm-publish finding; I fixed one path and left this one. P3 — enqueue was not fenced to the generation the capability decision was made against. The access checks, file resolution and owner re-read are all awaits, so the run can be replaced before the enqueue: the item then lands on the REPLACEMENT queue while the durable preempt flag and the arm still name the previous epoch, the arm is fenced out at the owner, and the 202 promises an interrupt that cannot happen. enqueueSteer now takes an expected generation, mirroring drain/peek, and the Redis path enforces it inside STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it. All three new specs verified counterfactually, including the Lua guard against real Redis (removing it returns 1 where -1 is required). * fix: fence the steer to its authorized generation, bound resume setup, keep preempt when Redis parks Codex round 16, all three findings. P2 — round 15 fenced the enqueue to owner.createdAt, the RE-READ job. Every guard above it (ownership, tenant, paused-state, agent ACL) ran against the job read at the top, so if the run was replaced during those awaits the fence happily accepted the steer into a generation the request was never authorized against, carrying the wrong agent's metadata. Now rejects on any mismatch between the validated job and the re-read. P2 — resume awaited its steering bookkeeping unbounded, after approvals.resolve had consumed the action and flipped the job to running, and outside the resume lifecycle's own try/finally. .catch does not fire on a promise that never settles, which is what ioredis produces during an outage, so the client times out, its retry gets a 409 for a spent action, and no cleanup runs. Bounded at 1s with the writes finishing in the background. P3 — Redis parks leftover steers inside its terminal-transition Lua, which projects item fields one by one, so preempt was silently dropped and a steer recovered from /chat/status lost its interrupting label. Added to both projections. All three verified counterfactually, two against real Redis. Worth recording that my first version of the generation-mismatch test was VACUOUS — it faked a createdAt matching no live job, so the round-15 enqueue fence rejected it for the wrong reason and the test passed with the guard removed. Rewritten to replace the run for real; it now fails with 'Expected 404, Received 202'. |
||
|
Danny Avila
|
7b6900d556
|
🏷️ feat: Activity Groups With Fast-Model Headers (#14391)
* ✨ feat: Activity Groups with Fast-Model Labels Groups each contiguous block of reasoning + tool calls into a collapsible unit headed by a fast-model label (claude.ai-style hierarchy), off the critical path: a PostToolBatch hook claims a live content slot at the batch boundary (steering index-offset pattern), renders a deterministic counts phrase instantly, and swaps in the generated label ~1s later while the next model call streams. Labels are UI-only — stripped before the SDK formatter and skipped in the legacy formatter — and reach live clients via a dedicated on_activity_label SSE event (live/replay/pending paths). Grouping preserves legacy rendering byte-for-byte when no label part is present. Generation bridges to Run.generateActivityLabel() when the SDK ships it (session-grouped Langfuse tracing); falls back to a direct call today. Env-gated: ACTIVITY_LABELS_POC=true, ACTIVITY_LABEL_MODEL. * 🧷 fix: Address Codex and Copilot Review Findings for Activity Labels - Settle in-flight label fills (bounded 3s) before finalization on both the main and resume paths, so a label resolving during the final batch still reaches the durable log and saved message. - Overlay on_activity_label chunks in RedisJobStore content reconstruction (splice path last-wins per index; replay path chronological overwrite), matching steer handling. - Wire activity labels into the HITL resume createRun so post-resume batches keep claiming slots. - Guard against out-of-order publishes: fill() awaits the claim emit before emitting the resolved label, and the client applier ignores a stale pending placeholder once a resolved label is present. - Stamp the batch's groupId onto label parts so parallel-column runs place them inside their group instead of filtering them out. - Localize the counts fallback phrase (10 keys, singular/plural) through useLocalize across chat rendering and exports. - Type the hook with Providers/ClientOptions instead of stringly types; drop the unknown cast in the spec; add a dedicated rAF retry ref for label events with effect cleanup. * 🛡️ fix: Address Independent Review — Abort, Usage, Lane Context, Redis Test - Propagate the run abort signal into label generation (both wiring call sites; runtime combines host + dispatch signals with the timeout) so a user abort cancels in-flight label calls instead of paying to timeout. - Record label-call usage like titles: the SDK bridge aggregates via chainOptions callbacks, the fallback path via a per-generation callback factory; both feed recordCollectedUsage under context 'activity-label'. - Scope block-context capture: reasoning collection stops at the previous block's label part and filters by executingAgentId, so consecutive or parallel batches can no longer bleed another block's thinking into the payload; intent text still scans past labels (persists across batches). - Forward the effective charLimit to the SDK call so host and SDK prompts agree (SDK default aligned to 600 in agents#327). - Add a Redis integration test proving last-write-wins reconstruction of on_activity_label chunks per claimed index. - Rebased onto main: only the two activity commits replay (the nine steering commits belonged to the old base branch), zero conflicts, steering suites green. * 📐 refactor: Move Activity-Label Wiring to TypeScript, Address Codex Round 2 - [P1] Slot claiming, lane stamping, emit ordering, context capture, and settle tracking now live in packages/api (createActivityLabelWiring + captureActivityBlockContext); client.js is a thin closure wrapper. - Register the activity-label hook BEFORE the steer drain so a steer draining at the same batch boundary cannot flush the tool block and orphan the label outside its group. - Resolve request-based header placeholders in resolveActivityLabelLLM (titleConvo parity) so metadata-keyed proxies work on label calls. - Trim labels centrally before filling so whitespace-only output from either generation path keeps the deterministic counts fallback. * 🧭 fix: Codex Round 3 — Capture Order, Shared Strip, Token Estimator, Hide Filter - Capture block context BEFORE pushing the label part: the scan stops at ACTIVITY_LABEL parts, so post-push capture hit the just-inserted label and silently collected no reasoning excerpts (regression test added). - Share stripActivityLabelParts from packages/api and apply it in the Responses and OpenAI-compatible controllers, closing the replay leak for entry points still running SDKs without the formatter skip. - Exclude activity_label parts from the fallback response-token estimator (UI-only parts must not inflate no-usage provider billing). - Keep label parts explicitly under hide_sequential_outputs — they summarize exactly the outputs that mode hides. * 🔁 fix: Codex Round 4 — Resume Gap, Delta Flush, Agent-Scoped Intent, Token Counter - Synthesize on_activity_label events for labels claimed or filled in the snapshot→subscribe window (the publish is fire-and-forget, so Redis-mode reconnects missed them). Feature-gated so the default path adds no content re-read; the client applier already ignores duplicates. - Flush queued deltas before applying a label part, matching the pending- action and steer appliers — without it the handler read a stale message cache and syncStepMessage pushed a pre-delta copy back. - Skip another agent's tail text when resolving intent, so parallel runs cannot seed a label prompt with a sibling agent's narration. - Exclude activity_label parts from countFormattedMessageTokens (the agent-path counter), not just the legacy BaseClient one. * 🏗️ refactor: Codex Round 5 — Extract Label Host Logic, Report Usage, Icon Strip - Move provider/model resolution, usage-metadata mapping, and the settle loop into packages/api (activityLabels/host.ts); client.js keeps only thin delegations, per the repo's TypeScript-implementation convention. - Fold label usage into the response rollup with an 'activity-label' tag (subagent precedent) so metadata.usage and the live cost gauge account for it; tagged, so it stays out of PRIMARY usage/context pairing. - Narrow tool metadata once in ToolCallGroup so THINK parts in a labeled block no longer render phantom generic icons in the stacked strip. - Import the activity-label helpers by deep path in GenerationJobManager: the package barrel now reaches provider-config/cache modules that import back into the stream layer, and the cycle broke suite loading. Declined: resetting steerOffsetState before HITL resume — resume builds a FRESH AgentClient via initializeClient (initialize.js:978), so the offset is already zero; the seed wrapper alone accounts for pre-pause parts. * 🚦 fix: Codex Round 6 — Stream Label Usage, Close Late Fills - Emit an on_token_usage chunk for label calls (sink push alone left the live session gauge blind); retained in pendingSubagentEmits so job cleanup cannot race the persist, tagged 'activity-label' as before. - Close the label scope when settle times out: the wiring gates fill() on isClosed and the client fires a label-scoped AbortController, so a straggling generation can neither mutate a saved response nor emit into a job whose runtime is gone. The controller also chains to the run signal, so a user abort still cancels label work. * 🩹 fix: Repair CI — Package Typecheck and Module Mocks Local runs covered the client tsconfig and jest, but never packages/api's own tsconfig, so nine type errors in the extracted host module shipped. - Type host.ts against the real contracts: ServerRequest, EndpointDbMethods, AppConfig from @librechat/data-schemas, IUser for createSafeUser, and a MaybeAzureConfig view for the azure instance-name probe and configuration. - Widen resolveConfigHeaders' llmConfig to Partial<RunLLMConfig>: it only reads the three provider header carriers, so auxiliary generations with a bare ClientOptions can resolve headers without assembling a run config. Type-only widening; every existing caller still satisfies it. - Add stripActivityLabelParts to the @librechat/api mock in the OpenAI and Responses controller specs — those mocks enumerate exports, so a new import read as undefined and threw before the assertions ran. - Use the real activity-label helpers in the ToolCallGroup spec's ~/utils mock; stubbing them out would hide the header logic under test. * ⚙️ feat: Configure Activity Labels via librechat.yaml, Drop Env Vars Replaces the ACTIVITY_LABELS_POC / ACTIVITY_LABEL_MODEL env gate with per-endpoint settings, following the title options convention rather than a top-level block — each endpoint picks its own cheap label model. - Add activity, activityModel, activityEndpoint, activityPrompt, activityMaxPerRun, and activityCharLimit to the endpoint schema, and to the endpoints.all pick list (enumerated, so 'all:' would otherwise drop them silently). - resolveActivityConfig reads them with title-style precedence: endpoints.all > named endpoint > custom endpoint config. - Model precedence is now activityModel > titleModel > the agent's model. activityEndpoint runs labels on another endpoint's credentials, with titleConvo's fallback-on-unknown-name behavior. - Thread activityPrompt/MaxPerRun/CharLimit through the wiring into the hook and the SDK bridge; they were hardcoded defaults. - The resume gap-repair gate keyed on the env var; it now keys on the snapshot actually containing label parts, so deployments without the feature still perform no extra content read. - Document the fields in librechat.example.yaml; add host.spec.ts covering precedence, custom-endpoint fallback, and opt-out. * 📝 refactor: Rename Enable Flag to activityLabel, Document Schema Inheritance - Rename the boolean from `activity` to `activityLabel`, matching the titleConvo/titleModel shape: a verb-object toggle whose prefix matches its modifiers (activityModel, activityPrompt, ...). `activity: true` alone read ambiguously — it could mean tracking or logging activity. - Document the two endpoint-schema inheritance paths, which behave oppositely and are ~900 lines apart: * `endpoints.all` omits from baseEndpointSchema, so new options are inherited automatically — nothing to maintain. * `azureEndpointSchema` enumerates via .pick(), so a new option is silently unavailable on Azure endpoints until listed there. The activity block now carries a pointer to the Azure caveat. * 🔍 fix: Address Codex Findings on the Config Rework - Pass the matched custom-endpoint config into the label gate. Custom endpoints live in the `endpoints.custom` ARRAY, so without it every custom endpoint resolved as disabled — including the example this PR added to librechat.example.yaml. - Give label usage a unique `runId:seq`. Label usage is billed but never appended to `collectedUsage`, so its length was static: every label event reused the last primary usage's pair and collided with itself, and the client dedupes on exactly that. - Attach `cost` to label usage when `interface.contextCost` is on; aggregateEmittedUsage treats coverage as all-or-nothing, so an event without it suppressed the whole response's cost. - Honor `activityPrompt` on the direct fallback path, not just the SDK bridge — it previously always used the built-in instruction. - Seed the per-response label cap from labels already on the response so a HITL resume cannot mint a fresh quota after every approval. - Reconcile label gaps on resume via a durable per-job `activityLabels` flag instead of probing the snapshot: the FIRST label of a run can be claimed inside the snapshot->subscribe window, which the old signal missed. The flag is read from a job record already fetched there, so runs without the feature still add no content read. - Auto-collapse labeled single-tool groups; one-call batches are common in agent runs and rendering them expanded defeats the grouping. * 🎯 fix: Correct Label Usage Seq, Cross-Endpoint Pricing, Close Scopes - Give label usage a NEGATIVE seq namespace. The previous fix was wrong: seq is a position in `collectedUsage` (push, then emit with the new length), so sink-length + array-length still lands on a real position — primary emits 1, the label computes 2, the next primary also emits 2. Labels have no position at all (billed separately, never appended), so they now occupy a namespace positional sequences cannot reach. The client key is a string used for Set membership, so the sign is inert. - Price cross-endpoint labels with the LABEL endpoint's token config: resolveActivityLabelModel now returns the resolved endpointTokenConfig, and both the streamed cost and recordCollectedUsage use it instead of the agent endpoint's rates. - Make close state per-wiring rather than per-client. A HITL resume rebuilds the wiring, and resetting a shared flag re-opened closures from the pre-pause segment whose provider call ignored the abort; settle now closes every retained scope, past generations included. * 🎯 fix: Make the Activity Header Say Something the Cards Cannot The header read "ran 1 command" next to a card already labeled "Code" — it restated the UI beneath it instead of adding to it. Two causes, both about content rather than timing: - A deterministic tool-type tally was the primary display and also fed the prompt, so the best case was a tally and the worst case was a tally dressed as prose. Removed from the metadata, the prompt, the part type, and the client. - The instruction only ever reached the fallback path. The wiring passed a prompt only when was configured, so the preferred SDK path silently used the published package default. The wiring now always supplies one and the hook forwards it on both paths. The register is rewritten around what the cards cannot show: past-tense git-commit-subject, leading with the distinctive noun, outcome over attempt, tool names and counts and arguments explicitly forbidden. The batch entries are labeled as reference material so the model stops transcribing them. Claiming a slot no longer emits. The slot still reserves its index so streamed parts never collide, but with nothing to say there is nothing to render: until a description exists the block looks exactly as it does without the feature. * 🧹 fix: Drop the Localize Hook Left Unused by the Counts Removal * ✅ test: Add Activity-Label e2e Coverage with a Recording Label Server Activity labels are the one model call a mock run does not already fake: fake-model.js swaps the GRAPH model via overrideTestModel, while run.generateActivityLabel() calls the endpoint resolved client options over HTTP. The custom endpoints already point baseURL at 127.0.0.1:8889, so serving that port exercises the real path with no production seam. fake-label-server.js answers it in both JSON and SSE form, records each prompt, and can inject blank/error responses. Recording is what lets the spec assert the CONTRACT rather than the rendering: that this repo register and the tool OUTPUTS actually reach the model. That is the bug class that produced unusable labels before, and rendered text looks identical whether or not the instruction arrived. Labels get a dedicated endpoint (Mock Provider E). A labeled block auto-collapses even at one tool call, which hides the tool cards other specs assert on -- enabling this on a shared endpoint broke steering.spec.ts. Provider D is the unlabeled control. Request-count assertions are scoped to a per-test token: a 5xx label response is retried by the provider client, and a retry can land after the next test has reset the server. * 🩹 fix: Address Review Findings on Activity-Label Indexing and Pricing Replay index (P1). Reserving the slot only in server memory left no event for it, so a cross-instance replay rebuilt content as [tool, hole, later], compacted the hole away, and the fill for the reserved index landed on the following part and overwrote it. The claim now publishes the empty, pending part so the index is real for every consumer, and fill publishes even when generation returned nothing so the client cannot stay pending. It stays invisible: an empty label still DELIMITS its batch in groupSequentialToolCalls but is not attached as the header, so grouping does not re-shuffle when the text lands and the block renders exactly as it does with the feature off. Edited-response index (P1). Edit-and-resubmit replays the kept prefix and the server indexes only new content, so run steps offset by that prefix. Labels are claimed in the same space and now take the identical shift; without it a label could land inside the prefix and overwrite it. Redis flag. deserializeJob never read activityLabels back, so every Redis reload left it undefined and resume skipped label gap reconciliation. Executing agent. RunActivityLabelOptions.agentId selects the executing agent tracing metadata AND its tool-output redaction policy; omitting it let a handoff be redacted under the default agent configuration. Label pricing. An undefined endpointTokenConfig is meaningful for a built-in label endpoint (priced from the shared table), so the nullish fallback billed those labels at a custom primary rates. Inherit only when the label runs on the agent own endpoint. HITL usage sequence. runId is the response message id and the counter was instance-local, so a resume restarted at -1 and the client runId:seq deduper discarded the post-approval label usage. Seeded past the labels already on the response. Also distinguishes "cannot serve" (undefined) from "no label" (null) in the SDK bridge, so a missing run falls back to the direct call instead of filling the slot empty. Version gating already happens at wiring time via the sdkCapable prototype probe. * 🩹 fix: Keep Unfilled Activity Labels Invisible and Unmask Endpoint Settings Follow-up review round. Publishing the reservation on every batch made two latent rendering paths reachable on every run, and both are fixed here. Empty labels no longer change grouping. The previous pass still formed a tool-group for a textless label, which wrapped even a single tool call and pulled THINK parts inside it — and since a reservation is published the moment each batch ends, that applied during every generation and permanently after a blank or failed fill. An empty label now flushes the legacy way instead: it still delimits its batch, but the block re-splits exactly as it renders with the feature off. Parallel lanes no longer show a blank line. Lanes render raw parts, so an unfilled label had nothing to draw; empty ones are dropped. Making labels act as collapsible headers inside lanes is still a separate gap. Edited responses no longer offset on resume. The sync replaces initialResponse.content with the server's aggregatedContent, which already contains the kept prefix AND everything generated since — so its length is not the prefix length, and indices reconciled from that snapshot are already absolute. Offsetting again pushed the label past its slot onto a later part. The shift now applies only to a fresh edited submission. Activity settings resolve field by field. Selecting one config object whole meant any endpoints.all block — even one carrying nothing but headers — shadowed the named or custom endpoint and silently disabled activity labels everywhere. Global still wins per field. Adds groupToolCalls coverage for the invisible-while-empty contract, which is the part most likely to regress: it is normal state on every run, not an edge case. * 🔒 fix: Scope Detached Label Writes to Their Generation Epoch Epoch scoping (P1). Label generation is detached and can outlive the generation that started it. emitChunk only proves that SOME runtime is current, not that the caller belongs to it, so an aborted generation's fill(null) -- and its usage event -- could be attributed to whichever generation replaced it, landing an index from the abandoned response on top of the new one. Because an empty label renders nothing, that overwrote content silently. emitChunk now takes an optional jobCreatedAt and drops the event when the runtime epoch differs, mirroring the existing setGraph/setContentParts convention, and both label emitters pass it. An abort now CLOSES the label scope instead of only cancelling the call: the rejected generation still runs its catch and calls fill(null), which would otherwise emit into a stream the next generation may already own. Edited-response indexing (P1). The previous pass skipped the prefix offset on resume, which was the wrong half of the problem: a sync replaces initialResponse.content with the server's aggregatedContent, which is completion-local, so after a reconnect its length is not the kept-prefix length and the offset is wrong -- but it is wrong for run steps in exactly the same way. Tool cards and the label that heads them must share one index space; a label shifting differently from its tools lands on another part. The label path now uses the identical expression as useStepHandler, with no resume special-case. Correcting the post-resume prefix length belongs in calculateContentIndex, where it fixes both at once. titleModel masking. The activity settings were made per-field last pass, but the titleModel fallback a few lines below still selected an entire config object, so a partial endpoints.all (for example one carrying only headers) hid a named endpoint's titleModel and quietly fell the label back to the main agent model. Both now read through one shared per-field helper. Resume reconciliation no longer depends solely on markActivityLabels, which is best-effort yet had come to gate correctness: a lost flag write silently dropped a label. The snapshot is consulted as a fallback. The exported host type for generateLabel now admits undefined, which is the documented "cannot serve, fall back to the direct call" signal the hook keys on -- distinct from null, meaning it ran and produced nothing. * 🧷 fix: Keep Group Identity Stable and Memoize Label Endpoint Resolution Group remount. Tool-group identity was keyed on the first part in the block. An activity label absorbs the block's leading THINK part the moment its text lands, so the key flipped from tool:<id> to fallback:<scope>:<idx> mid-run, remounting the group and discarding whatever the user had expanded. The key now scans for the first tool call, which does not move when the block re-forms. Label endpoint resolution is memoized per response. It reads provider config and can hit the database for user keys, yet nothing it depends on changes between batches of one run — and it ran twice per batch, once for generation and once for usage accounting. The promise is cached rather than the value so concurrent batches share a single in-flight resolution, and a rejection is evicted so one transient credential failure cannot disable labels for the rest of the response. * 🎯 fix: Offset Edited Resubmissions by a Prefix Length That Survives Resume The server indexes only NEW content for an edited resubmission, so the client offsets incoming indices by the prefix it retained. That prefix was read as initialResponse.content.length, which is correct only until a resume: the sync replaces that array with the server's completion-local snapshot, whose length is unrelated to the prefix. After a reconnect every offset was therefore wrong -- run steps and activity labels alike -- and could write over content the edit kept. For a label the symptom is worse than a bad position: the fill misses its own reservation, so the pending placeholder is never resolved. The prefix length is now captured when the submission is built, while initialResponse.content still IS the retained prefix, and carried on the submission as editPrefixLength. calculateContentIndex takes that length instead of deriving it from an array that a resume may have replaced, so run steps and labels share one index space by construction rather than by both happening to read the same field. Note the prefix is the FULL original content with the edited part substituted in place (useChatFunctions clones latestMessage.content and mutates one entry) -- it is not a slice, so the length cannot be inferred from editedContent.index. Group identity no longer changes when a label fills. Tool-group keys were derived from the first part in the block; an activity label absorbs the leading THINK part when its text lands, flipping the key mid-run and remounting the group, which discarded the user's expansion state. The key now scans for the first tool call, which does not move. Label endpoint resolution is memoized per response. It reads provider config and can hit the database for user keys, yet ran twice per batch -- once to generate, once for usage accounting -- while nothing it depends on changes within a run. The promise is cached so concurrent batches share one in-flight resolution, and rejections are evicted so a transient credential failure cannot disable labels for the rest of the response. The resume gap passes for steers and activity labels now share a single lazy content read instead of each issuing its own. The label pass stays gated on the run flag with a snapshot fallback: reconciling unconditionally would also close the residual first-label window, but it would bill a read to every resume of every run, including deployments with the feature off -- which the steer pass deliberately avoids. That residual requires a lost flag write, which shares fate with the content writes the labels live in. * 💵 fix: Bill Cross-Endpoint Labels at Their Own Rates recordCollectedUsage never accepted an endpointTokenConfig, so the value the activity-label caller passed was dropped and the balance transaction was written at the primary agent's rates. Only the UI cost honored the label endpoint, so a custom primary pointing activityEndpoint at another endpoint showed one price and charged another. The parameter is now accepted, and an explicit config wins outright over per-agent resolution: that map is keyed by AGENT, so it cannot describe usage that ran on a different endpoint. Group identity is stable for id-less tool calls too. The previous pass anchored the key to the first tool call ID; where a supported tool call carries no id the fallback still used the block's first part index, which shifts when a filled label absorbs the leading THINK part. The fallback now anchors to the first TOOL entry's index, so only a block containing no tool call at all keys off parts[0]. markActivityLabels is retried rather than fire-and-forget. It gates resume gap reconciliation and is a SEPARATE write from the durable label append, so a single lost write silently drops a label the content itself recorded. The earlier "shared fate with content writes" reasoning was wrong. One retry at run setup costs nothing and removes the only realistic way the gate goes stale, without billing a content read to every resume. * 🧮 fix: Stop Offsetting Once SYNC Drops the Edited Prefix The edit offset was applied unconditionally, but whether it is correct depends on which branch SYNC took. SYNC either preserves the content already loaded for the response -- which still contains the retained prefix, so the offset is required -- or replaces it with the server's aggregatedContent, which is completion-local and indexed from zero, after which any offset writes past the end of a now shorter array. That is why the two previous attempts each fixed half of it: skipping the offset on resume was right for the replace branch, applying it unconditionally was right for the preserve branch, and neither holds on its own. The offset now tracks the actual state of the rendered content. For an activity label the replace branch was worse than a bad position: the fill landed past its own reservation, so the pending placeholder was never resolved and the block kept its generic header for the rest of the run. Applied to run steps as well, not just labels. useStepHandler reads the prefix from the same submission and had the same unconditional offset, so after a mid-session resume of an edited response tool cards were misplaced too. Normalizing at the dispatch boundary keeps both in ONE index space by construction: a label that shifted differently from the tools it heads would land on another part. Note the reload path was already coherent -- useResumeOnLoad rebuilds the submission without editedContent or editPrefixLength, giving no offset against server-supplied content -- so only the mid-session SYNC path was inconsistent. * 🧾 fix: Keep Label Accounting Out of the Primary Usage Slot Label usage no longer owns getStreamUsage(). recordCollectedUsage assigned its result to this.usage unconditionally, so when the primary provider reported no usage metadata but the label provider did, BaseClient took the label's output tokens as the assistant response's authoritative count. The later primary call returns early on an empty collectedUsage and never replaced it, so the wrong value stood, the text-based fallback was skipped, and the real generation went unbilled. Secondary usage is still billed but no longer writes that slot. Cross-endpoint pricing keys off an explicit discriminator rather than the presence of a value. A built-in label endpoint prices from the shared table, so an undefined endpointTokenConfig is its MEANINGFUL value -- reading that as "no override" fell back to the primary's custom rates and restored the exact mismatch the previous pass set out to fix. The caller already knows whether the label ran elsewhere and now says so. markActivityLabels rejects on failure instead of swallowing it. The flag gates resume gap reconciliation and the caller retries it, but the internal catch resolved successfully and made that retry unreachable -- so the two changes cancelled out and a transient write failure still left the flag absent. Late label accounting is suppressed with the same gate as the late fill. A straggler that outlived the settle timeout still ran its finally block, so it charged the balance and appended to usageEmitSink after the response had passed its usage flush and metadata snapshot: a cost the user pays but is never shown. The cleared-prefix state is scoped to one generation. It was set on a resume SYNC that replaced the response and then never reset, so a later edited resubmission in the same mounted hook dispatched run steps and labels with no offset against content that still held its retained prefix. Reconnects pass isResume and keep the state; a new generation clears it. * 🔑 fix: Key Prefix State to the Stream and Honor current_model for Labels The cleared-prefix reset keyed on isResume, which skips exactly the case it was added for: a submission whose POST succeeded server-side but lost its response is retried, comes back resumed: true, and subscribes in resume mode even though it is a NEW generation. A previous generation's cleared state then survived into it, and incoming run steps and labels applied no offset against content that still held its retained prefix. The state is now keyed to the stream id, which changes with the generation and stays put across reconnects of one. activityModel now honors current_model. The options are documented as title-shaped and the titleModel fallback already excludes the sentinel, but the higher-precedence activity override passed the literal string through to getOptions and the provider, so an endpoint following that convention failed every label instead of using the agent model. * 🎯 fix: Key Prefix State to the Generation and Resolve the Run Model The cleared-prefix state was keyed to the stream id, which never changes within a conversation: request.js sets streamId = conversationId, so once a reconnect cleared the state every later edited resubmission in that conversation dispatched run steps and labels with no offset and could overwrite the prefix it retained. It is now keyed to the response message id, the only per-generation identity available here -- minted per submission and carried through a resume unchanged. That is the third identity tried for this state. isResume missed the deduplicated-retry path (a lost response returns resumed: true for a new generation); the stream id is conversation-scoped. The response id is the boundary that actually matches a generation. current_model labels now resolve the model the run is really using. initializeAgent merges the request's endpointOption override into model_parameters and the run gives it precedence, so preferring the saved agent.model could send labels to a different, potentially unavailable or more expensive model than the conversation is on. * 🆔 fix: Key Prefix State to the Submission and Keep the Origin Title Model Editing an assistant response reuses that response's messageId as editedMessageId, and useChatFunctions carries it onto initialResponse.messageId -- so re-editing the same response produced two generations with the same key and the cleared-prefix state survived between them, leaving run steps and labels with no offset against content the edit retained. Keyed now to clientRequestId, the per-submission uuid, which is minted fresh per edit attempt and forwarded unchanged on retries. That is the fourth key this state has had, and each earlier one failed at a real boundary: isResume missed the deduplicated-retry path, the stream id is the conversation id, and the response message id is reused across edits of one response. clientRequestId is the identity that actually means "this submission". The titleModel fallback is read from the ORIGINATING endpoint again, matching how titleConvo captures its config before switching credentials. Reading it after an activityEndpoint switch meant an OpenAI endpoint configured with titleModel claude-haiku and activityEndpoint anthropic fell through to the OpenAI run model and sent that name to Anthropic, failing every label. The destination endpoint supplies credentials, not the model choice. * 🧷 fix: Close the Remaining Edit, Epoch, and Scope Gaps for Labels SYNC clears the edit prefix on the new-row branch too. When a resumed edited submission cannot match an existing assistant row, that branch builds the response straight from the server's completion-local aggregatedContent, so it holds no retained prefix -- but the reset lived only in the matched branch, leaving later steps and labels adding an offset to indices that were already absolute. Label usage is keyed per GENERATION. Editing one assistant response reuses its responseMessageId while each fresh generation restarts activityLabelUsageSeq, so a second edit re-emitted the same runId:seq and the client discarded the newer usage while its balance transaction was still written. The key now carries jobCreatedAt, the run's own epoch: stable across reconnects and HITL resumes, distinct between generations. The scope is revalidated at commit time. Checking once before the await let a scope that closed mid-flight still charge the balance after finalization, while the matching fill saw the closed scope and dropped the label -- billed but never surfaced, the exact outcome the guard exists to prevent. The titleModel fallback no longer reaches the destination endpoint. With activityEndpoint set and no titleModel on the originating endpoint, it picked up the destination's, so changing only the credential target silently changed the model and its cost. Precedence is activityModel, then the originating endpoint's titleModel, then the run model; the destination supplies credentials only. * ✂️ refactor: Confine the Edit-Prefix Offset to Activity Labels useStepHandler is now byte-identical to dev again. The resume-aware prefix offset was applied there too, which was more correct in principle -- the post-resume prefix length is genuinely wrong for run steps as well -- but it changed index math that EVERY run step flows through, for every user, including everyone who never enables activityLabel. That shared correction needed five revisions in two days (isResume, the stream id, the response message id, clientRequestId, and the SYNC new-row branch), each passing the full suite and each failing at a boundary only review found. Carrying it inside an opt-in feature put every user behind logic with that track record. It belongs in its own change, with tests that construct the edit-plus-resume states none of the current suites reach. The offset now applies only where the label handler places its part, so this PR cannot alter rendering for anyone with the feature off. The known consequence is recorded in the description: with activity labels ENABLED, an edited response that reconnects mid-generation can place its label and its tool cards in different index spaces. That is a bug for opt-in users rather than a regression for everyone, and it disappears once the shared fix lands. submission.editPrefixLength stays: the label path still needs a prefix length that survives a SYNC replacing initialResponse.content. * 🧾 fix: Commit Labels Before Billing and Keep Blank Slots Invisible Round-nine review (all P2, feature-scoped): - Billing ordering (client.js:409, runtime.ts): usage accounting ran BEFORE the slot commit on both generation paths, so the settlement deadline could expire during the balance write — charged, then the fill dropped as out-of-scope: billed, never shown. `slot.fill` now resolves a commit flag, generators register their accounting via `deferUsage`, and the hook runs it only after a committed fill. - Scope gates (client.js:757): the direct-fallback `collect` omitted `scopeOpen`; both paths now gate on the OWNING wiring's scope, so a pre-pause straggler cannot bill because the resumed generation's scope is still open. - Blank-label grouping (groupToolCalls.ts:81): a blank slot forced a flush, splitting adjacent single-call batches into standalone cards where the feature-off path merges them. Blank labels now only mark the claim boundary — structurally invisible, while a later filled label still cannot claim an earlier batch. - Stale fill indices (wiring.ts:301): the skill-card unshift and the hide-sequential filter reshape contentParts before the finalization settle, so an in-flight fill emitted its claim-time index against a shifted array. Both completion paths now settle label fills before any post-run content reshaping (the finally settle stays as the error-path net; the second call sees an empty pending list). - Bounded serialization (runtime.ts:238): `JSON.stringify` fully materialized unbounded tool results to keep 200/600 chars per entry. A budget-bounded serializer stops at the limit (which also bounds cyclic values) and preserves the exact truncate-with-ellipsis output. Tests: fill/bill ordering + suppression on dropped fills (runtime.spec), blank-slot merging and claim boundaries (groupToolCalls.test), bounded serialization equivalence and giant-output truncation (runtime.spec). * 🧮 fix: Keep Deferred Label Billing Inside the Settle Window Self-review follow-up to the billing reorder: deferring usage until after the commit moved it PAST the fill's resolution, so a settle keyed on fills alone could let finalization flush the usage sink and snapshot metadata while the label's billing was still in flight — the usage row would silently miss the message rollup even on the happy path. The hook now reports its whole detached task (generate → fill → deferred usage) via a `trackTask` option, wired to the same settle tracker as the fills, so finalization waits for billing exactly as it did when accounting preceded the fill. The task never rejects. Pinned in runtime.spec: the tracked task resolves only after usage collection. * 🧰 fix: Harden Label Resolution, Output Bounds, and Cache Billing Round-ten review (all P2, feature-scoped); the sixth finding is the documented edited+reconnect index-space limitation, answered on-thread as deliberately out of scope for this PR. - Rejected-LLM memoization (runtime.ts): the hook cached a rejected `resolveLLM()` promise permanently, failing every later batch and silently defeating the host resolver's own rejected-cache eviction. The memo now evicts on rejection so the next batch retries. - `current_model` precedence (host.ts): an explicit `activityModel: current_model` resolved to `undefined` and then lost to a configured `titleModel`. The sentinel now resolves straight to the run model; the title fallback applies only when `activityModel` is absent. - Output bounds (runtime.ts): label text was persisted verbatim; a model ignoring the 4–9-word instruction (or steered by injection in untrusted tool output) could emit thousands of tokens duplicated through SSE, the chunk log, persistence, and the UI. `normalizeLabelOutput` keeps the first non-empty line, collapses whitespace, and hard-caps at 200 chars on both generation paths. - Cache-token billing (host.ts, client.js): the usage mapper dropped cache fields, vanishing Anthropic cache tokens from billing and charging OpenAI cache reads at the full input rate. The mapper now normalizes Anthropic/OpenAI/LangChain cache shapes into `input_token_details`, and the emit + cost path carries them with the label endpoint's `provider` (additive-provider adjustment). - Usage-type union (runs.ts): `TTokenUsageEvent.usage_type` now includes the emitted `activity-label` literal; the lone consumer keys on `usage_type != null`, so this is type-level completion. Tests: sentinel/title/explicit model precedence and all three cache shapes (host.spec), transient-resolution retry and output normalization with truncation (runtime.spec), the new usage literal (runs.spec). * 🪗 fix: Let Settled Labels Collapse Void Tools and Keep the Tail Cursor Round-eleven review (all P2, client-side). Two fixed; the other two findings restate documented Known limitations (edited+reconnect run-step index space; parallel-lane collapsible headers), answered on-thread. - Void-tool auto-collapse (ToolCallGroup.tsx): `allCompleted` keyed solely on output truthiness, so a tool that legitimately returns an empty string kept its labeled group expanded forever. A settled, filled label is itself a completion proof — the PostToolBatch claim only happens after every output in the batch returned — so it now satisfies `allCompleted`; pending labels keep the group live. - Trailing-reservation cursor (ContentParts.tsx): a blank label reservation at the content tail renders nothing but still counted as the last part, stripping the streaming cursor and last-item affordances from the last VISIBLE part until the next delta. `lastContentIdx` now walks back past empty label slots. Tests: labeled void-tool group auto-collapses, pending-label group stays expanded (ToolCallGroup.test). * 💳 fix: Price Label Cache Correctly, Honor endpoints.agents, Cancel Every Retry Round-twelve review: four fixed here; the remaining P1 (move the client.js bridge into packages/api) is an architecture call answered on-thread for the maintainer. - Provider on billed entries (client.js, P1): round ten added cache details to label usage entries but not `provider`, and `splitUsage` treats an unknown provider as additive — re-adding cache_read and cache_creation on top of an input count that already contains them, double-charging Anthropic/OpenAI cached label calls while the streamed cost (which carried the provider) disagreed. Every mapped entry now carries the label endpoint's provider. - endpoints.agents honored (host.ts, client.js): `initializeAgent` rewrites `agent.endpoint` to the backing provider, so activity settings under the PUBLIC `agents` endpoint — valid config, inherited by `agentsEndpointSchema` — were silently ignored. Field resolution is now `all` > public endpoint > backing provider/custom, applied to both the enable gate and the model/titleModel resolution. - E2E_LABEL_PORT reaches the YAML (playwright.config.mock.ts): an overridden port moved the fake label server and its health check but not the generated config's hard-coded 8889 baseURLs, so readiness passed while every label request targeted the wrong port. The override is now substituted into the generated copy. - Every retry frame cancelled (useResumableSSE.ts): concurrent label retry chains (reservation + fill per slot) overwrote one rAF handle, so cleanup cancelled only the newest chain; the rest ran up to 120 frames past unmount and could apply a stale label to a replacement generation reusing the same response id. Outstanding frame ids now live in a Set that cleanup drains. Tests: public-endpoint gate/precedence/all-above-public (host.spec). * 🖱️ fix: Keep the Last-Part Cursor in Parallel Lanes Too Round-thirteen review (single P2): `ParallelContentRenderer` computed `lastContentIdx` from the unfiltered array, so a trailing blank label reservation — filtered out of every lane — left NO rendered part carrying the last-part cursor and running-subagent affordances until the label filled. The sequential renderer's walk-back is extracted into a shared `lastVisibleContentIdx` helper (utils/activityLabels) used by both `ContentParts` and `ParallelContentRenderer`, so the two index spaces cannot drift again. Behavior pinned in activityLabels.spec: trailing blank skipped, consecutive blanks skipped, filled label counts, label-free content unchanged. * 🧹 chore: Alias the Retry-Frame Set for the Effect Cleanup Lint Rule * 📏 fix: Let activityCharLimit Reach Tool Inputs Round-fifteen review: `activityCharLimit` is documented as the per-entry truncation for tool input AND output, but `buildPrompt` hard-coded inputs at 200 characters — so raising the setting could never surface a distinguishing path, query, or operation that appears past the first 200 characters of a long argument. Inputs now truncate at the configured limit alongside outputs; the 200-char constant remains only for the intent line (renamed INTENT_CHAR_LIMIT to match). Config fidelity pinned in runtime.spec: a 400-char argument survives a 450 limit and truncates under a 50 limit. The round's other finding is the fifth restatement of the documented edited+reconnect index-space limitation, answered on-thread with the prior four cross-references. * 🤝 fix: No Labels for Pure Handoff Batches Round-sixteen review: a PostToolBatch containing only `transfer_to_*` calls claimed a label slot, but transfer parts are never groupable — the client flushed the handoff card standalone and the label orphaned into a stray line after it, restating what the card already says. Two-sided fix: - Hook (runtime.ts): a batch whose every entry is a transfer call claims nothing — no slot, no model call, no `maxPerRun` consumption. Mixed batches still label (the header describes the real work). - Renderer (groupToolCalls.ts): an orphan label whose `tool_call_ids` are all transfer calls is dropped instead of rendered standalone, covering content persisted before the hook-side skip. The round's two P1s are repeats answered on-thread: the packages/api extraction (maintainer-decided follow-up, recorded in the description) and the sixth restatement of the edited+reconnect index limitation. Tests: transfer-only batch claims nothing, mixed batch still claims (runtime.spec); transfer-only orphan label dropped, real-batch orphan label still renders (groupToolCalls.test). * 🎛️ fix: Sanitize Label Client Options and Bound the Batch Prompt Round-seventeen review: two fixed; the other two findings repeat the maintainer-decided packages/api extraction (follow-up) and the edited+reconnect index limitation (seventh instance), answered on-thread. - Primary-option strip (host.ts): the label client copied the resolved `llmConfig` wholesale, so an endpoint whose defaults enable extended thinking or carry model-specific output caps forwarded them to the (often cheaper) label model — unsupported options failed every label, and supported thinking spent real tokens and the settlement window on a 4–9 word header. The copy now strips `omitTitleOptions` keys and the `modelKwargs` output caps exactly like the title path, restoring the Anthropic `clientOptions` carrier by reference so proxy `defaultHeaders` still reach label requests. - Batch prompt budget (runtime.ts): per-entry truncation left the batch dimension unbounded — hundreds of parallel calls could build a prompt past the fast model's window. The entries section now has a total budget (8k chars, scaling with `activityCharLimit` so a raised limit still fits several entries); entries past it are skipped without paying their serialization cost, and the list notes how many were omitted. The first entry always renders in full. Tests: option strip with header-carrier survival (host.spec); giant batch bounded with omission marker, small batch untouched (runtime.spec). * 🛡️ fix: Keep SSRF Guards on Label Calls, Skip Mixed Handoff Batches Round-eighteen review: four fixed; the fifth repeats the maintainer-decided packages/api extraction (eighth instance), answered on-thread. - SSRF-safe carrier (host.ts, P1): the sanitize step restored the Anthropic `clientOptions` carrier only when `defaultHeaders` existed, but for user-provided base URLs `getLLMConfig` stores the guarded Undici dispatcher and `redirect: 'error'` there — dropping it reopened DNS-rebinding/redirect paths on label calls to user-controlled URLs. The carrier (client CONSTRUCTION options, not generation params) is now restored whenever present, same reference. - Primary maxTokens (host.ts): top-level `maxTokens` is not in `omitTitleOptions` and survived the strip; the title path deletes it explicitly, and a cap sized for the primary model can be rejected by the substitute. Deleted on the copy. - Bounded keys (runtime.ts): the object branch materialized every key via `Object.keys` and quoted oversized keys in full before the budget check. Enumeration is now lazy (`for..in` + own-property guard) and keys slice to the budget before quoting, like string values. - Mixed handoff batches (runtime.ts, groupToolCalls.ts): the client flushes the block at the transfer card, so a mixed batch's label orphaned exactly like a pure one. The hook now skips ANY batch containing a transfer call, and the renderer drops orphan labels covering one (legacy content). Tests: carrier survival without headers by same reference, maxTokens strip (host.spec); mixed batch claims nothing (runtime.spec); mixed orphan dropped, real-batch orphan kept (groupToolCalls.test). * 🧢 fix: Cap Label Generation, Order the Flag Persist, Detach Settled Listeners Round-nineteen review: three fixed; the fourth is the ninth instance of the edited+reconnect index limitation, answered on-thread. - Generation cap (host.ts): stripping the primary output caps left label calls with NO cap at all — `normalizeLabelOutput` bounds what persists, not what the provider generates and bills, so a model ignoring the 4–9-word instruction (or steered by injected tool output) could emit its provider-default output per batch. The sanitize step now installs a 256-token label cap (per provider family: `maxOutputTokens` for Google-style wrappers, `maxTokens` otherwise), after the filter so the omit set cannot remove it. - Flag-persist ordering (client.js): the `markActivityLabels` write was fire-and-forget, so an immediate cross-replica reconnect could read the job between the write and the first claim, see neither flag nor snapshot label, and skip gap reconciliation. Label emission now awaits the (settled-on-failure) persist chain, making "a label event exists" imply "the flag is durable" — the race window is gone; only the documented double-write-failure residual remains. - Listener detach (client.js): each HITL approval cycle's wiring adds a `once` abort listener to the shared job signal that only an actual abort removes; settled segments now detach theirs in `settleActivityLabels`, so long multi-approval runs cannot accumulate dead closures toward the listener-limit warning. Tests: the primary cap is REPLACED by the 256-token label cap (host.spec). * 🎯 fix: Route the Label Cap Per Model Family Round-twenty review: the 256-token label cap set maxTokens unconditionally, but GPT-5+ rejects max_tokens (the OpenAI builder routes its cap into modelKwargs.max_completion_tokens / max_output_tokens) and o-series models reject it with no stable kwargs alternative — every label on those models would have failed. The cap now mirrors the builder: modelKwargs for GPT-5+ (responses-API aware), no cap for o-series (title parity; the 200-char persistence bound still applies), maxOutputTokens for Google, maxTokens otherwise. Pinned in host.spec for both reasoning families. The round's other finding is the tenth instance of the documented edited+reconnect index limitation, answered on-thread. * ⏱️ fix: Persist the Label Flag at Run Start, Not on the Emit Path Round-twenty-one review: two fixed; the other three repeat the maintainer-decided packages/api extraction, the edited+reconnect index limitation, and the parallel-lane header limitation — all answered on-thread with their standing decisions. - Flag ordering, corrected (client.js): sequencing label emission behind the flag persist (previous round) delayed the claim-time reservation while the shared index offset had ALREADY shifted subsequent SDK chunks — reopening the cross-instance hole-compaction overwrite the reservation emit exists to prevent. The reservation emits immediately again; instead, run start (processStream and resume alike) awaits the settled-on-failure persist chain, so the flag is durable before any batch can claim a label. Same guarantee, zero latency on the emit path. - Tail-label cursor (ContentParts.tsx): a filled label at the content tail is consumed into the group header rather than listed in `group.parts`, so the `isLast` check missed it and nothing held the streaming cursor until the next delta. The check now includes `labelPart.idx`. * 🔌 fix: Detach Label Abort Listeners Even Without Claims A segment with labels enabled can end without a single claim (text-only, or handoff batches, which skip labels); the early return in settleActivityLabels skipped the detach added for HITL listener accumulation. The detach now runs on both paths. * ⚖️ fix: Make the Commit Flag the Sole Billing Authority Round-twenty-three review: a committed fill racing a late scope close (user abort or settle timeout during the durable emit) stayed visible — the part is mutated and persisted before the close — yet the deferred accounting's scope gates then skipped the charge: a completed provider call escaping both the label charge and the primary abort accounting. The scope gates on the deferred-usage path are removed; the hook's commit flag is now the single billing authority in BOTH directions. A dropped fill never reaches the accounting callback (billed-never-shown stays impossible), and a committed fill bills regardless of when its scope closed (shown-never-billed now impossible too). The dead `scopeOpen` payload threading is removed with it; the `recordActivityLabelUsage` parameter survives, defaulting open, for callers that own no commit signal. The round's other finding is the twelfth instance of the documented edited+reconnect index limitation, answered on-thread. * 🧮 feat: Bill Labels by Estimate When Providers Omit Usage Maintainer decision: follow the title convention rather than leaving label calls unbilled when a provider returns no usage metadata. The hook now passes a LAZY estimate thunk with the deferred accounting on the success path — the EXACT prompt the direct path sent (or the locally built equivalent for the SDK path: same entries, context, instruction, truncation contract, and continuity headers) plus the final normalized label. `recordActivityLabelUsage` invokes it only when no collected entry carries a real token count, counts both texts with the shared o200k_base tokenizer, and feeds the synthesized entry through the SAME pipeline (provider-tagged, streamed event, cost, balance transaction). Real provider usage always wins when present. The failure path passes NO estimate: a throw before a response bills only real collected metadata, never a full phantom prompt. Tests: the estimate thunk carries the exact invoked prompt and final label; the failure path defers with no estimate (runtime.spec). * 💵 fix: Estimate From the Raw Completion, Not the Normalized Label The fallback estimate counted the normalized label (first line, 200-char cap) while the provider generated and would bill the raw output up to the 256-token generation cap — under-recording verbose replies. The estimate thunk now carries the raw pre-normalization text; the persisted label is unchanged. Pinned with a multi-line reply test. * 🧾 fix: Commit Label Text Only After the Durable Emit, Estimate the Real SDK Prompt Round review on the billing work: two fixed; the third is the fourteenth instance of the edited+reconnect index limitation, answered on-thread. - Copy-first fill (wiring.ts): the fill mutated the shared content part BEFORE its durable emit, so a failed emit left the label text on `contentParts` anyway — persistence could save and display a label no client ever received and billing (keyed on the commit flag) never charged. The new state is staged on a copy; the shared part mutates only after the emit succeeds, so content, delivery, and billing move together. - Real SDK prompt for estimates (client.js): the estimate thunk carried this module's locally built prompt, but the SDK path frames entries differently — the estimated input count was for a prompt never sent. Chain-start callbacks (handleLLMStart/handleChatModelStart) now capture the prompt the SDK actually rendered, and the deferred accounting substitutes it into the estimate when capture succeeded, falling back to the local approximation otherwise. |
||
|
Danny Avila
|
d8427ffc5e
|
🛂 test: Cover Tool Approval Workflows End to End (#14427)
* test: cover tool approval workflows end to end * fix: preserve tool approval state across resume * fix: preserve agent context in mock stream responses * fix: preserve nested approvals in collapsed groups |
||
|
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
|
ad5bb477af
|
🎞️ fix: Surface Clear Error for Unprocessable Gemini YouTube Videos (#14396)
Google rejects a YouTube video it cannot ingest with a generic `400 INVALID_ARGUMENT` that names no cause, which LibreChat relayed verbatim. Attribute the failure using request context instead: when a Google/Vertex turn carried an injected YouTube video part and the provider returns that generic rejection, map it to a typed error the client localizes. Verified against the live API: a public 9h15m video is refused this way on gemini-2.5-flash, 3.5-flash, 3.5-flash-lite and 3.6-flash, including at MEDIA_RESOLUTION_LOW, while a short video with an identical payload succeeds. Duration is the dominant trigger; region and access restrictions return the same response, so the copy leads with length without overclaiming. A duration preflight was evaluated and skipped: oEmbed does not expose duration, leaving only watch-page scraping — a blocking call against undocumented markup from rate-limited datacenter IPs that would fail open and still need this mapping underneath. |
||
|
Danny Avila
|
7447fddfb2
|
🙊 refactor: Clarify Ask Question Schema Errors and Retry Guidance (#14279)
* fix(agents): clarify ask question validation errors * fix(agents): narrow question failure detection * fix(agents): persist question validation failures * fix(agents): track question validation failures |
||
Jaime Hidalgo
|
e813934731
|
🔖 fix: Preserve Ephemeral Agent Params and Identity for ask_user_question Resume (#14254)
* fix: durable ask_user_question resume for ephemeral agents * 🤖 refactor: Drop chat.js resume hunks in favor of shared packages/api helpers * 🤖 fix: Normalize resume thinking param and replay modelLabel (#14253 Bugs 1&2) * 🤖 fix: Preserve adaptive thinking display and effort across HITL resume * 🔤 style: Sort load.spec.ts imports (repo import-order) * 🤖 fix: Replay paused request body params on HITL resume (UI-form source of truth) --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Danny Avila
|
5771bf6e06
|
♨️ feat: Prewarm Stateful Code Sandboxes with Cold-Boot UX Feedback (#14239)
* ♨️ feat: Prewarm Stateful Code Sandboxes with Cold-Boot UX Feedback * 🧹 fix: Drain Prewarm Response + Reset Sandbox Atoms on Stream Cleanup * 🚿 fix: Propagate Prewarm Drain Failures + Warm Marker for Host File Tools * 🌡️ fix: Decouple Prewarm In-Flight State from Warm Refreshes + Precise Ready Gates * ☁️ refactor: Redis-Backed Sandbox Prewarm State via standardCache * 🧪 chore: Hermetic Prewarm Spec + Accurate Signal JSDoc (Copilot review) |
||
|
Danny Avila
|
9bb351ad9c
|
🧭 feat: Mid-Run Steering and Queued Messages for Agent Runs (#14220)
* 🧭 feat: Mid-Run Steering and Queued Messages for Agent Runs Steering: submit a message while a run is generating; the server queues it in the job store (cross-instance) and a run-scoped PostToolBatch hook injects it into graph state at the next tool-batch boundary, records an inline 'steer' content part on the response (replayed as a user message on later turns), and streams on_steer_applied to the client. Queuing: messages composed during a run auto-send as normal follow-up turns after clean completion (one per final event, FIFO); user aborts leave them as chips unless armed by interrupt-and-send. Requires hook injectedMessages support in @librechat/agents (danny-avila/agents#299); hard-gated via a capability probe so older SDKs 501 the steer route instead of draining and dropping messages. * 🧵 fix: Harden Steering Against Finalization Races and Route Guard Gaps Addresses local Codex review findings on the steering feature: - Close-and-drain the steer queue atomically at finalization (final event, abort) so a steer POST racing teardown is rejected instead of 202-ACKed and then silently cleared; the closed flag lives on the job hash and is reset when a replacement job reuses the stream id. - Clear inherited steer queues on createJob — a job replacement must not drain the replaced run's messages. - Keep steers queued across a HITL pause instead of draining them into ephemeral client state: resumeState re-seeds chips on reload and the resumed run injects them at its first tool boundary (steers key TTL now extends to the approval window; on_steers_pending event removed). - Queue the NO_ACTIVE_RUN steer fallback while the final SSE is still settling — a direct send would be dropped by ask()'s in-flight guard. - Reconcile the 202 ACK against on_steer_applied events that beat it over the SSE, so a chip can't be re-minted after its removal event passed. - Allow the per-send Steer override when the default action is queue. - Apply the configured message rate limiters and the PII filter to POST /chat/steer — a steer is model-bound user text. * ✅ ci: Assert Steering Capability Probe Against the Installed SDK CI installs the published @librechat/agents pin (pre-injectedMessages), where isSteeringSupported() is legitimately false — the probe test now asserts it mirrors the installed SDK's capability flag instead of hardcoding the capability-bearing build's value. Verified against both the published 3.2.61 dist and the agents#299 build. * 🛟 fix: Preserve Steer Text Across Run-End, Error, and Abort Races Codex round 2 (4 P2s): - Applied-steer-id set survives run end (capped at 100) and converted ids join it, so a 202 ACK that lands after final/abort drops its chip instead of re-minting a stranded pending one. - Failed runs no longer strand acknowledged chips: both error paths convert local pending chips to queued follow-ups (chip text is client-local), and the server closes the steer queue before emitting the error so a racing steer POST gets 404 fallback instead of a 202 whose payload dies with the job. - sendQueuedNow keys on steer availability, not the default action — send-now on a queued chip is an explicit override for queue-preferring users. - Stop path consumes pendingSteers from the abort HTTP response as a fallback for the SSE final event it may close before processing; conversion is deduped so double delivery is a no-op (shared useSteerConvert hook). * 📎 feat: Carry Attachments Through During-Run Queued Messages Steering stays text-only (SDK injection, inline STEER part, and replay are all text), so a during-run submit with media now queues the whole message as one unit instead of silently stranding the files: - QueuedMessage gains `files`; composer attachments are consumed into the queued item at queue time (steerFromComposer / queueFromComposer / interruptAndSend), fixing the latent hazard where lingering composer files glued onto whatever `ask` vacuumed up next. - Enter-steer with attachments degrades to queue with an explanatory toast; the per-send menu routes through the same composer-aware wrappers. - The drain and sendQueuedNow pass the item's files as `overrideFiles`; media items never steer (send as a normal turn when idle, re-front otherwise). ask() no longer clears composer state for caller-supplied overrideFiles — only regenerate keeps that behavior. - During-run submits hold while uploads are in flight, mirroring the send button's filesLoading gate; queued chips show a paperclip count. * 🎛️ feat: Rework During-Run Chips into Action Rows Full-width rows above the composer (reference-UI parity): each queued message shows a primary Steer/Send-now action, delete, and a "…" menu with Edit message (restores text + attachments into the composer) and a Turn on queueing/steering toggle that flips the Enter default. Steer rows share the layout with status text; failed steers keep retry / edit / queue-convert. The per-send menu gains the same default toggle. Queued file refs now retain filename + bytes so edit-restore rebuilds real composer entries (draft-recovery shape). * 🖇️ feat: Steer With Attachments (Multimodal Mid-Run Injection) Steering now carries media end-to-end instead of degrading to queue: - The steer POST accepts sanitized attachment refs (cap 10; only file_id is trusted — the drain re-fetches owner-scoped and re-derives everything else). SteerQueueItem/TPendingSteer/SteerContentPart carry `files` refs; encoded data is never persisted or queued. - New api/server/services/Files/steering.js decouples attachment building from the request path: encodeSteerContent reuses the exact per-turn pipeline (addFileContextToMessage + processAttachments' single-pass categorize/encode, SDK formatMessage assembly, prependFileContext for extracted text) with zero new encoding code. buildSteerMedia feeds the drain hook's new buildMedia seam (any failure degrades that steer to text-only — words always land); stampSteerPartMedia re-encodes past steer parts per turn with ONE batched owner-scoped fetch and stamps a transient `media` array, replaced immutably so it can never leak into a save. Replay honors resendFiles like regular message media. - The SDK's formatAgentMessages (the formatter agents actually use) gained the steer replay branch on the PR branch; the local formatMessages.js branch now mirrors the media preference. - Client: steerFromComposer consumes composer files into the POST, chips/seeding/conversions carry files everywhere (retry, queue convert, abort/error recovery), queued media items steer for real, and SteerBubble renders the steered attachments inline. * 🧵 fix: Harden Steer Recovery Races and Drain Isolation Codex round 3 (7 fixes): - A 202 ACK landing after the run ended converts straight to a queued follow-up (server queue is gone; no event will ever resolve a pending chip for a finished run). Covers stream errors with in-flight POSTs. - A Stop that lands pre-completion can arrive as a final with unfinished:true and no aborted flag — runEnd now treats it as aborted so queued messages are not auto-sent against the user's Stop. - Leftover-steer conversion merges chronologically by createdAt instead of appending, preserving the order the user composed. - Auto-drained queued messages pass explicit (possibly empty) overrideFiles/overrideQuotes/overrideManualSkills: a drain can no longer vacuum up files, quotes, or skill picks staged in the composer for the user's NEXT message (ask() treats overrideFiles != null as authoritative). - Failed-steer Retry and resume-on-load chip restoration keep the steer's attachments. - The job-replacement guard moved INSIDE the store's atomic drain/close-and-drain (Lua createdAt compare; in-memory equivalent): a stale run's hook or finalization can neither consume, close, nor steal a replacement job's steer queue, and the drain hook drops its separate check-then-drain round trip. * 🧰 refactor: Typed Steer Controller, Single-Query Media Pass, Round-4 Fixes Codex round 4 + efficiency tightening in one pass: - Moved the steer guard ladder (validation, file sanitization via a shared toSteerFileRef picker, ownership/tenant checks, status-guarded enqueue) into packages/api as handleSteerRequest; api/steer.js is now a thin wrapper. Ladder covered against the REAL in-memory job manager in request.spec.ts; the api spec pins only the wrapper contract. - Folded the steer replay stamp into the turn's ONE historical-files query: collectHistoricalFileRefs also gathers steer-part refs, the owner-scoped doc map rides client state, and stampSteerPartMedia consumes it (no second round trip) while encoding parts in parallel. - Stamped steer media now counts against the run budget (existing multimodal counter over the non-text parts, folded into indexTokenCountMap/promptTokens after the stamp). - Steer route runs the PII filter BEFORE moderateText, matching chat.js so blocked sensitive text never reaches the external moderation API. - Interrupt & send survives the abort-response-beats-SSE-final race: stopGenerating writes the run-end signal itself when the one-shot interrupt flag is armed and no signal landed (double-fire safe). - Resume reconciles chips against the server's still-queued list even when EMPTY, clearing chips for steers applied while disconnected. - The local formatter's steer flush preserves non-text assistant parts (array-content AIMessage) instead of folding to text. * 🔒 fix: Replay-Aware Capability Gate and Round-5 Race Closures - isSteeringSupported now requires BOTH halves of the SDK contract: injection (HOOK_INJECTED_MESSAGES_CAPABLE) AND replay (ContentTypes.STEER, shipped in the same SDK commit as the formatAgentMessages steer branch). An SDK that can inject but not replay 501s the steer route — no release window can create steer parts that would leak into provider-facing assistant content. - The local formatter mirrors the SDK's anchor reset: a post-steer tool_call mints a fresh AIMessage instead of attaching to the pre-steer anchor (invalid provider ordering). - Queued-chip send-now and the NO_ACTIVE_RUN fallback pass explicit (possibly empty) overrideFiles so an idle send can't vacuum composer files staged for a different draft. - Redis createJob deletes the stale steer list BEFORE the replacement hash is written as running — a steer 202-accepted against the new job can never be wiped by the reset. - Resumed-turn finalization mirrors the normal path's terminal drain: createdAt-guarded close-and-drain, leftovers ride the resumed final event as pendingSteers instead of being cleared by completeJob. - buildSteerMedia restores composer order over the $in result so multi-attachment steers reach the model in the order the user saw. * ⚛️ fix: Atomic Job Replacement and Boundary-Clean Steering Module Codex round 6 (5 fixed, 1 standing deferral): - createJob resets the steer queue and writes the job hash in ONE same-slot Lua script (JOB_CREATE_LUA): a steer POST can no longer interleave between them on cluster, so a steer accepted against one run can never be drained into another. Redis-validated. - The steering media pipeline moved to packages/api (agents/steering/media.ts) with injected getFiles and a structural client interface — /api keeps zero steering logic; specs ported to the DI seam. - handleSteerRequest checks the job BEFORE the capability gate: a steer racing completion on an unsupported SDK gets 404 (send-now) instead of a 501 queue with no run-end signal left to drain it. - useQueueDrain binds to the active conversation: navigating away between the final SSE and the drain effect leaves the signal unconsumed instead of submitting A's follow-up into B; the drain fires on return. - abortJob closes and drains the steer queue BEFORE the content snapshot, so a drain-hook apply that lands pre-drain is captured inline rather than lost between the snapshot and the terminal drain. * 🚦 fix: Parked Run-End Signals, Interrupt Priority, Settled-Run Fallbacks Codex round 7 (5 fixes): - Run-end signals for a non-active conversation are PARKED per conversation instead of squatting the shared index slot: a later run finishing on the same pane can no longer overwrite them, and the parked drain fires when the user returns. - "Interrupt & send" front-inserts carry a priority flag that outranks createdAt when abort leftovers merge back chronologically — the urgent redirect drains first, not the oldest steer. - STEER_UNSUPPORTED/RUN_PAUSED/QUEUE_FULL rejections landing after the run settled mirror the NO_ACTIVE_RUN fallback and send immediately (queueing would strand the text with no run-end signal left); on the pinned SDK this is the common Enter-near-run-end path. - A failed abort (e.g. 404 when the run completed first) still signals the interrupt drain, so the queued interrupt message can't strand and the armed flag can't leak onto a later run. - Steered-image fallback alt text is localized (com_ui_attached_image). * 📌 chore: Adopt Published @librechat/agents Types Post-Bump dev's pin bump to ^3.2.62 (the release carrying injection + steer replay) landed via merge; the steering runtime now uses the SDK's real InjectedMessage/hook-output types instead of the local structural mirrors that bridged the pre-publish window. The two-half capability probe stays as the defensive gate for mismatched deployments — and the capability spec now exercises its TRUE path against the published package in CI. * 🛅 feat: Park-and-Claim Steer Recovery + Host-View Content Reads Codex round 8 (6 fixed incl. both P1s, 1 push-back): - The long-deferred no-subscriber gap is closed: every terminal drain (final, aborted-final, error, abortJob, resumed finalize) PARKS acknowledged leftovers on the job hash (unrecoveredSteers), and the status route claims them exactly once for inactive jobs — a client that closed/reloaded past the transient final event restores its steers as queued chips within the post-terminal TTL. A replacement run clears the parked copy (a live client started it). - Same-instance content reads are steer-complete: RedisJobStore now caches the HOST content array (WeakRef) via setContentParts and prefers it over the SDK graph cache, whose view never contains host-authored steer parts; the graph fallback splice-INSERTS steer chunks at their recorded host-view indices (the graph array is unshifted, so assignment would overwrite SDK parts). - Replay token accounting now counts prepended file-context text: full stamped content minus the steer body (already counted), so large steered documents hit the budget instead of bypassing pruning. - The queue drain restores an item when ask() refuses without sending (history not yet in cache after navigating back) — text is never silently dropped. - The armed interrupt flag travels WITH a parked run-end signal, so another run on the same pane can neither consume nor clear it. - parseTextParts extracts steer text (search indexing / audio). * 🎛️ refactor: Single Send Slot + In-Thread Steer Messages - Merge the during-run send affordance into the send/stop button slot: with composer text the send button replaces Stop (Enter = default action), hover reveals Steer/Queue/Interrupt rows with shortcuts; drop the separate DuringRunActionsMenu chevron - Add during-run keyboard chords: Cmd/Ctrl+Enter = non-default action, Alt+Enter = interrupt & send (plain-Enter submitters only) - Render steers as standard user messages in the thread: SteerPart (icon + author header + user text presentation) replaces the SteerBubble, and submitted steers appear immediately at the projected injection point via the PendingSteers slot on the streaming message - Keep composer rows only for recoverable states: failed steers (retry/edit/queue) and queued follow-ups * 🩹 fix: Keep the Replacement Submission Alive Across Abort Settlement The aborted run's final SSE event fires before the abort HTTP response resolves, so an armed interrupt & send drains and starts the NEXT submission while the abort POST is still in flight. The response handler's unconditional clearAllSubmissions() then reset the new submission, aborting its stream attach before the subscribe — the follow-up ran and persisted server-side but the live placeholder finalized empty (content appeared only after reload). useAbortCleanup captures the submission before the abort round-trip and both settlement paths (success and 404-catch) clear only when the captured submission is still current; a replacement stays untouched. Plain Stop behavior is unchanged. * 🧭 test: Playwright E2E for Mid-Run Steering and Queuing - Add e2e/specs/mock/steering.spec.ts: steer mid-run (202 + immediate in-thread pending part + real MCP tool boundary + words survive run end), Cmd/Ctrl+Enter queue with auto-send after clean completion, and Alt+Enter interrupt & send with the follow-up streaming into the live view - Add the E2E_STEER_TOOL_REPLY fake-model marker: slow preamble, a real remember_fact MCP tool call (PostToolBatch boundary), then a final turn - Test 1 pins the run-end degradation contract while the SDK's top-level agentId stamping bug blocks live injection; its header documents the assertions to flip once the fixed SDK is pinned * 🧷 fix: Job-Independent Steer Recovery + Expiry and Resume-Gap Parking Codex round 10: the park-and-claim recovery had lifecycle holes. - Move parked steers off the job hash onto their own bounded-TTL store key (JOB_CREATE_LUA resets it; deleteJob leaves it alone): the default completeJob path deletes the job record immediately, and the Redis read path never deserialized the old hash field — recovery previously worked only with STREAM_KEEP_COMPLETED_JOBS on the in-memory store - Carry the owner identity inside the parked payload and authorize the claim against it, so the status route recovers steers on its jobless branch too (the common reload-after-terminal case); a non-owner claim returns nothing and re-parks the payload - Park queued steers on approval expiry: snapshot the frozen queue before the requires_action→aborted CAS (whose terminal cleanup drops the steers key) and park only when the CAS wins - Mirror the terminal drain/park block in resume.js's failure path, which previously let completeJob's backstop clear 202-accepted steers - Close the Redis snapshot→subscribe resume gap: re-peek the queue after attaching and re-surface missed on_steer_applied events from the durable content view (synthesizeAppliedSteerEvents), updating resumeState.pendingSteers to the live queue * 📌 chore: Require @librechat/agents 3.2.63 + Applied-Steer E2E Contract - Bump the @librechat/agents pin to ^3.2.63 in api/ and packages/api/: it scopes the hook agentId marker to subagent child graphs, so the steering drain hook fires at top-level tool-batch boundaries and mid-run injection is active (danny-avila/agents PR 307) - Flip e2e steering test 1 from the documented degradation contract to the applied-steer contract: the optimistic in-thread part transitions to the persisted part at the tool boundary and survives inside the response after run end, with no queued follow-up turn * 🎗️ feat: Steered Messages Join the Message-Nav Ribs Steers are user messages, so they get their own clickable rib on the navigation rail, interleaved at their in-thread position inside the response that absorbed them (one DOM query in document order). SteerPart anchors itself as #steer-<id> with a steer-render marker — both the optimistic pending entry and the persisted part — and the rib carries the user role label with a preview drawn from the steer's text body, skipping the author header. * ❎ feat: Cancel a Queued Steer Before Injection + True User-Message Alignment - Add POST /chat/steer/cancel: removes ONE still-queued steer by id via an atomic list rebuild (Redis Lua preserves order and TTL), authorized against the job owner; removed:false is advisory — the cancel lost its race to the drain or the run end, never an error - Surface an × on the in-thread pending steer (server-acknowledged entries only): optimistic removal, restored if the POST fails since the server would still inject the words - Outdent SteerPart past the response's icon column so steers sit flush with top-level message rows, reading as regular user messages * 🧯 fix: Round-11 Recovery Hardening + Provider-Free Pending Slot - Reconcile the resume steer gap by steerId SETS, not queue length — a steer added in the gap (or an equal-length drain+enqueue swap) now refreshes resumeState.pendingSteers and still synthesizes the missed on_steer_applied events - Make completeJob's terminal backstop park: direct error-path callers without the controllers' close-and-park no longer silently clear 202-accepted steers (createdAt-guarded closeAndDrain + owner park before the terminal write) - Persist the steer part BEFORE media encoding in the drain hook: an abort inside the encode window can no longer lose a file-steer (the part refs come from the enqueue-sanitized item; replay re-encodes per turn unchanged) - Move the parked-claim owner check INSIDE the atomic store claim (substring gate in the Lua / in-memory equivalent): a non-owner probe can no longer transiently delete the recovery payload; the app-side parse stays authoritative - Park queued steers in BOTH stores' own requires_action expiry cleanup, which bypassed the manager-level sweep - Sweep expired parked steers from the in-memory store's periodic cleanup; restore a queued chip when send-now's submit is refused; upsert steer ACKs so an SSE reconnect reseed cannot duplicate chips - Mount the cancel mutation per steer item so the pending slot needs no QueryClient on ordinary streaming renders (fixes the CI failure in ContentParts.integration.test) - Skipped delivery-gated parking (finding 8): transport receiver counts cannot prove browser delivery, and gating the only durable copy on them trades cosmetic chip resurrection for real text loss; the window is already bounded by claim-on-read, createJob reset, and the TTL * 🩺 fix: Annotate PARKED_STEERS_TTL_MS for isolatedDeclarations tsdown's d.ts generation requires explicit types on exported consts with computed initializers; tsc --noEmit does not run that check, so the round-11 export slipped past local verification and broke Build packages (and every downstream CI job that consumes the built dist). * 🛟 fix: Round-12 Terminal-Path Recovery + Durable Steer Events - Park queued steers before the stale-running reap deletes a crashed or hung job in BOTH stores — the one terminal path with no controller finalization; requires_action expiry parking refactored onto the same snapshot/park helpers - Enqueue instead of dropping when a steer fallback send is refused: both the NO_ACTIVE_RUN branch and the settled-run rejection branch now observe sendNow's false return - Recover on the SSE reconnect-404 terminal path: convert local pending steers to queued, claim parked steers via /chat/status, and write a non-completed run-end signal so interrupt flags release without auto-sending an unknown outcome - Fall back to a positive parked-recovery TTL when completedTtl is 0 (SET EX 0 is invalid and silently killed recovery) - Make on_steer_applied durable before publish: emitChunk gains a durable option that awaits the chunk-log append (best-effort) ahead of the transport publish; the default delta path stays fire-and-forget * 🔐 fix: Round-13 Steer Authorization + Trusted File Refs - Resolve client-supplied steer file refs against the DB owner-scoped at enqueue and queue only DB-derived shapes (same filter as the injection fetch, shared via refs.ts); any unresolved id fails loud with 400 — spoofed type/filepath metadata can no longer be persisted into assistant content or rendered in chat/share views - Enforce agent authorization on /chat/steer against the ORIGINATING run's job identity: the chat path's role gate (AGENTS:USE, with the same non-agents-endpoint skip) plus the per-agent ACL check with the capability bypass — revoked access mid-run can no longer inject; cancel stays ownership-only (nothing model-bound) - Mark steered uploads used after a successful enqueue (owner-scoped, best-effort) so the upload-window TTL cannot reap a file the persisted steer part references - Consume the parked recovery copy after live delivery: converting final/abort/error pendingSteers fires one owner-gated claim-on-read, so dismissed chips can no longer resurrect on a later reload * 🎙️ fix: Round-14 Composer-Context Fidelity + TTS and Queue-State Gaps - Keep steer text out of generic assistant text extraction: parseTextParts excludes STEER parts by default with an includeSteer opt-in for the full-record surfaces (Meili indexing, aborted-response persistence) — TTS callers no longer speak the user's own mid-run words - Mark queued uploads used at enqueue time via a minimal owner-scoped POST /files/usage (fail-closed without a user; upload limiters do not apply to a metadata touch), fired once wherever composer files enter the queued state — the upload-window TTL can no longer reap a file waiting out a long run or approval pause - Carry quote chips and manual skill picks on queued items: captured and consumed from the composer at queue/interrupt time exactly like files, threaded through the drain and send-now overrides, and restored by the queued row's Edit message - Key an early-aborted FIRST turn's run-end signal to NEW_CONVO (resolveRunEndTarget) so queued follow-ups stay visible on the restored new-chat composer instead of parking under an optimistic stream id the user never sees again * 🧿 fix: Round-15 Gap Coverage + Consolidated Sweep (Share Leak, Abort Ids, Chip Hygiene) - Run the resume steer-gap check for every still-active job: an empty snapshot no longer skips the re-peek, and synthesis now keys on the FRESH content view so an applied-in-gap steer that was never snapshotted still re-surfaces (over-emission is benign — applied-id dedupe, index-stable parts) - Thread queued context through steer degradation: sendQueuedNow passes the item's quotes/skills into submitSteer, and every fallback (requeue or settled send) restores them instead of dropping to text+files - Stop shared links from leaking steer attachment refs: the share snapshot now walks content — files-excluded shares strip steer-part files entirely; files-included shares sanitize and share-route them like top-level files (copy-on-write, non-steer content by reference) - Seed pending-steer chips unconditionally on load/return so a steer applied while away cannot linger as a stale chip beside its part - Use the abort response's resolved job id: chips/drain-signal land where the user actually is (NEW_CONVO for a new-held first turn, consistent with resolveRunEndTarget) while the parked-copy claim hits the resolved id instead of a no-op /chat/status/new - Open steered documents like normal message files (FilePreviewDialog) - Cap the applied-steer id set on the live path via a shared helper; kept surviving run end deliberately (late-ACK race depends on it) and fixed the atom comment that claimed otherwise * 💡 fix: Un-light Steer Ribs When Their Node Is Replaced Two stacked gaps kept a steer rib lit after scrolling away: the pending→applied swap replaces the DOM node under the same id, which produces no IntersectionObserver exit and — because the entry list dedupes on (id, preview) — no entries change either, so the observer kept watching a detached node; and the rail's mutation filter only reacted to .message-render nodes, so steer-node swaps and removals never triggered a refresh at all. - reconcileObservedElements re-points the observer at replaced nodes from the mutation-driven refresh regardless of entries identity, dropping stale visibility until the fresh node reports (the observer fires its initial intersection immediately, so a truly visible part re-lights within a frame) - The mutation filter now recognizes steer-render nodes alongside message rows * 🪪 fix: Round-16 Recovery Owner Fields + Context Stickiness + Share Labels - Park resumed-run leftovers with the manager facade's metadata owner fields: a bare job.userId is undefined on that shape, which made every parked payload from a resumed HITL run unclaimable - Keep a queued item's quotes/skills sticky through a successful steer ACK: the pending chip carries them (client-only), reseeds preserve them across reconnects, and every terminal conversion — local or server-list, merged by steerId — restores them onto the queued item - Convert resumeState.pendingSteers on the inactive status branch (deduped against unrecoveredSteers) so steers observed in the expired-pause-before-sweeper window convert instead of vanishing until a later reload - Label shared steer parts share-safely via the existing ShareContext: a viewer's own name no longer appears on the sharer's steered messages * ✂️ fix: Carry Steer Context Through the Failed-Chip Edit Action Retry and convert-to-queue already preserve a failed steer's carried quotes/skills; Edit message dropped them on the way back to the composer. It now restores them through the same context path. |
||
|
Danny Avila
|
53e369fba8
|
🧪 feat: stateful_code_sessions capability for warm Code API sandbox sessions (experimental) (#14150)
* ✨ feat: stateful_code_sessions capability for warm Code API sandbox sessions Wire the @librechat/agents stateful sandbox sub-config behind a new, off-by-default stateful_code_sessions agent capability. createRun sets toolExecution.sandbox.statefulSessions when code execution is active in the run AND the capability is enabled; execute_code and bash_tool factories get the param so their descriptions hedge toward persistence. Rides the existing variable-not-literal runConfig pattern, so it no-ops until @librechat/agents is bumped to the version shipping the sandbox sub-config. * ✨ feat: per-agent stateful code sessions (builder toggle + init gating) Stateful sessions now require the agent's own opt-in, not just the admin capability. New agent field stateful_code_sessions (schema + validation + types) surfaces as a toggle in Agent Builder Advanced settings, gated on the app capability and disabled without Code Interpreter. initializeAgent resolves the per-agent truth (admin capability AND builder opt-in AND code env) once: the registered bash_tool description, the execute_code factory, and createRun's toolExecution.sandbox gate all read the same resolved value. statefulSessionsAvailable threads through the same call sites as codeEnvAvailable, including handoff discovery and added convos. * 🐛 fix: propagate runtime_session_hint to sandbox executor in event-driven tool path The event-driven ON_TOOL_EXECUTE handler built config.toolCall without the resolved runtime_session_hint, so BashExecutor/CodeExecutor never sent runtime_session_hint to the Code API. Every conversation then collapsed onto the server-derived default session (no per-conversation isolation). Copy tc.runtimeSessionHint onto toolCallConfig._runtime_session_hint, mirroring the SDK direct-execution path. * 🐛 fix: address Codex review findings for stateful code sessions - OpenAI-compatible service (packages/api/src/agents/openai/service.ts) now derives and passes statefulSessionsAvailable alongside codeEnvAvailable, so the feature activates on that route (previously statefulCodeSessions resolved false there and createRun never sent toolExecution.sandbox). - Thread runtime_session_hint through the host file-authoring tools (create_file/edit_file/read_file): those host branches return before the generic tool path, so readSandboxFile/writeSandboxFile now forward the per-conversation hint instead of falling back to the Code API default session. - StatefulSessions builder toggle clears its form value when Code Interpreter is disabled, so a saved agent matches the disabled UI and re-enabling code doesn't silently reactivate stateful sessions. * 🐛 fix: normalize stateful_code_sessions on save when Code Interpreter disabled Addresses Codex review (round 2): a stale `stateful_code_sessions` opt-in could persist when Code Interpreter (`execute_code`) is disabled from the main agent builder without opening Advanced settings, silently reactivating warm sessions if code was later re-enabled. - AgentPanel: normalize in `composeAgentUpdatePayload` (the always-run save path) so `stateful_code_sessions` is forced to `false` whenever `execute_code !== true`, regardless of whether Advanced was opened. - StatefulSessions: revert the mount-scoped useEffect (round-1 approach) — it only fired while the Advanced panel was mounted, missing this path. - Add spec coverage for both branches of the normalization. |
||
|
Danny Avila
|
3945d293de
|
🗂️ feat: Per-Agent Memory Partitions (#14084)
* feat: per-agent memory partitions (memory_scope)
Adds an optional agentId partition to MemoryEntry so agents can opt into
isolated memory via a new memory_scope field ('user' | 'agent'). Partition
derives from agentId presence ({agentId: null} matches legacy docs, no
migration). Inline set_memory/delete_memory tools, the post-turn memory
agent, the request-scoped memory cache, and context injection are all
partition-aware; context is only injected into agents whose resolved
partition matches. Memory routes accept the partition param, scope
duplicate/token-limit checks per partition, and enrich entries with agent
names. Memories panel gains a partition filter and agent badges; the agent
builder gains an agent-scoped memory toggle.
* fix: address Codex review findings on memory partitions
- strip runtime ____N id suffixes in getMemoryAgentId so added-conversation
runs share the persisted agent's partition
- load each agent's own partition in multi-agent context injection instead
of skipping foreign partitions entirely
- clear memory_scope to 'user' on save when Enable Memory is unchecked
- fall back to 'all' when the selected panel partition no longer exists
- restrict GET /memories agent-name resolution to agents the requester can
VIEW
|
||
|
Danny Avila
|
988a14a405
|
🙋 feat: ask_user_question - agent-initiated questions with durable pause/resume (#14139)
* feat: ask_user_question tool — agent-initiated questions with durable pause/resume The HITL runtime merged in #13942/#14024/#14025/#14123 already ships the full ask_user_question lifecycle (payload-agnostic handleRunInterrupt, resume validation via mapAskUserAnswer, reconnect rehydration, and the client question card) — but nothing ever raised the interrupt. This adds the producer: - packages/api/agents/hitl/askUserQuestionTool.ts: LLM-callable tool whose func calls the SDK askUserQuestion() helper (LangGraph interrupt() from the tool body); zod schema with length caps mirroring AskUserQuestionRequest, plus a JSON-schema twin for the schema-only registry - Registration: agentToolDefinitions, manifest.json (Tools dialog, admin filteredTools/includedTools kill switch), basicToolInstances, handleTools constructor branch - run.ts gating: checkpointer now attaches for hitlCapable runs whose agents carry the ask tool even with the tool-approval policy disabled (the interrupt needs only durability, not humanInTheLoop/hooks); the tool is stripped fail-closed from non-HITL callers (OpenAI-compat/Responses) and subagent child configs; excluded from eager event execution (interrupts must be raised inside the Pregel task frame) - resume.js: 16k length cap on the answer wire field - e2e (real Run + FakeChatModel + LazyMongoSaver + supertest resume): tool-body interrupt pauses durably with NO approval policy, answer round-trips as the ToolMessage content, tool body re-runs once on resume, sequential questions re-pause * fix: adversarial-review findings — in-graph execution, orphan prunes, endpoint scoping, real kill switch Pre-PR multi-agent review confirmed 5 defects in the initial commit; all fixed: 1. CRITICAL — the tool never paused on the real agents endpoint: production loads tools definitions-only, flipping the SDK ToolNode to event-driven dispatch, and the host ON_TOOL_EXECUTE handler runs outside the Pregel task frame (under runOutsideTracing), where interrupt() throws and becomes an error ToolMessage. Reworked: the ask tool never rides toolDefinitions/ toolRegistry — on HITL-capable top-level agents a real instance is supplied via AgentInputs.graphTools (agents#289, requires @librechat/agents > 3.2.57), the SDK's in-graph direct-tool seam; new production-shape e2e pins the event-driven mode end to end. 2. CRITICAL — ask-only runs left orphaned interrupted checkpoints (silent context duplication on every later turn): both orphan prunes were gated on toolApproval.enabled. The pre-turn prune now also fires for ask-capable agents (exported agentRequestsAskUserQuestion), and the abort-route prune fires when the aborted job carries a pendingAction. 3. MAJOR — self-spawned subagents bypassed the strip (self config resolves from the parent's _sourceInputs): fixed SDK-side (buildChildInputs clears graphTools) and the tool is now never present on child surfaces host-side. 4. MINOR — the manifest entry leaked into the Assistants tools dialog and the legacy plugins endpoint, where tools execute with no run to pause: new agentsOnly manifest flag, scoped out of both listings. 5. MINOR — filteredTools/includedTools only hid the tool from the dialog: now enforced at run build (strip + no checkpointer), making the admin filter a real kill switch for already-saved agents. * chore: update @librechat/agents dependency to version 3.2.58 in package-lock.json and package.json files * fix: reject agents-only tools at assistant create/update (Codex round 1) The tools-dialog scoping keeps ask_user_question out of the assistants LISTING, but the v1/v2 create/update handlers resolve arbitrary posted tool strings from the shared getCachedTools map — a REST client or stale saved payload could still attach it, and the assistants runtime executes tools with no run to pause, so every call would error. New isAgentsOnlyTool(tool) (manifest-driven, handles string and function-object shapes) drops such tools with a warn at all four resolution sites (v1+v2, create+update). * fix: offset resumed-run content indices past the pre-pause seed A resumed run rebuilds the graph from the checkpoint, and the fresh graph numbers content indices from its own empty contentData — starting at 0. The resume path seeds the (also fresh) content aggregator with the pre-pause parts at exactly those indices, so the resumed model turn collided with the seed: type-matching parts silently MERGED (post-resume text appended into a pre-pause text block), and type-mismatching parts (a reasoning/think part at index 0 — any Anthropic reasoning agent) dropped EVERY delta with 'Content type mismatch', losing the entire post-resume output from the live stream and the saved message. Latent since #13942 — tool-approval resumes corrupt content the same way (probe-verified); it surfaced now because ask_user_question makes pausing a first-class flow and reasoning models make the loss total. - createContentIndexOffsetHandlers(handlers, offset): wraps ON_RUN_STEP (the single point where a content index enters the pipeline — deltas resolve through the aggregator's stepMap) and ON_AGENT_UPDATE's inline index; every other handler passes through by reference. Probe-validated: resumed output now lands as a new part after the paused tool call. - resumeCompletion wires it with offset = seedContent.length. - logToolError: a GraphInterrupt unwinding out of a tool body is the HITL pause working as designed — no longer logged as a Tool Error. * fix: unblock live streaming of the resumed segment after an answer With resume indices now ABSOLUTE (server continues after the pre-pause parts), the synthetic ask-user-question card was squatting on exactly the index the resumed segment streams into: applyAskUserQuestion appends the card at the end of the message content, so on the answering device every incoming part at that index was blocked and nothing rendered between the answer submission and the finalize replacing the message. removeAskUserQuestionPart(message, actionId) strips the pause-scoped card on successful answer submission (useResumeSubmit onSuccess) — the durable record of the Q&A is the ask_user_question tool call itself. Pure helper + specs; same-reference no-op when nothing matches. * fix: displace the synthetic question card in the streaming content writer The store-level strip on answer submit wasn't enough: the SSE step handler keeps its own in-flight copy of the streaming message, so on the answering device the synthetic ask-user-question card still occupied the ABSOLUTE index the resumed segment streams into — every delta warned 'Content type mismatch' (existing ask_user_question vs incoming text) and nothing rendered between the pending_action and finalize. Displace the card inside updateContent when any real part claims its slot — the same displacement pattern as the OAuth prompt part directly above it. Covers the streaming handler's own copy, reconnecting tabs, and other devices; once real content streams, the pause is over by definition. Spec drives a runStep + text delta into the card's index and pins: no mismatch warn, card gone, text rendered. * feat: dedicated UI + durable data for completed ask_user_question calls The completed ask call rendered as a generic tool card labeled 'Cancelled' with raw (and empty) JSON args. Two layers fixed: Data: the saved tool_call part had args:'' and no output — streamed arg chunks carry no tool name so the aggregator drops them (normal tools recover via the completion event, which never fires for a tool that interrupts mid-execution and resumes on a rebuilt run with no step id). The resume controller now stamps the paused ask part with the pendingAction's authoritative question as args and the user's answer as output (attachAskUserQuestionAnswer — pure, targets the newest unanswered ask part, so sequential questions each keep their own answer). UI: Part.tsx routes ask_user_question tool calls to AskUserQuestionCall — a compact Q&A record ('Asked a question' header, question, description, 'You answered: <label>' preferring the picked option's label, or 'No answer was given' for an abandoned pause) instead of the generic card. New i18n keys; parseAskUserQuestionArgs degrades to null on malformed model args. * fix: single question UI per pause + immediate answer display Two live-turn issues with the new durable Q&A card: 1. Duplicate question on ask: during a live pause the message carries BOTH the ask tool_call part (now rendered by AskUserQuestionCall, showing a misleading 'No answer was given' while paused) and the synthetic interactive card. The durable card now defers while the turn is live and unanswered (isSubmitting) — the interactive card owns the question UI until it's answered; an abandoned pause still shows its no-answer state once the turn settles. 2. 'No answer was given' after answering: the server stamps the answer onto the part at resume seed, but the client only received that at finalize. No stream emission needed — the client knows the answer it just submitted: resolveAskUserQuestionPart (replacing the plain strip on submit success) removes the synthetic card AND stamps output/progress onto the newest unanswered ask tool_call, seeding args from the synthetic part's question when the streamed args were lost — mirroring the server-side attachAskUserQuestionAnswer, so the Q&A record shows the answer the moment the user submits. * fix: keep the Q&A record visible while the resumed segment streams The optimistic output stamp lives in the message store, but the SSE step handler evolves its own cached copy of the streaming message (created at turn start) — the first resumed event overwrites the store with that copy, wiping the stamp, so the Q&A card blinked out during streaming and only returned at finalize. Render-layer fallback instead of fighting the handler's copy: submitted answers are recorded by ask tool_call id when resolveAskUserQuestionPart stamps the part, and AskUserQuestionCall reads the recorded answer whenever the part's own output is missing — the record survives any message-copy churn until finalize delivers the server-stamped part. * feat: present Ask User as a native builtin in the tools dialog It ships with the app and pauses the run like a first-class feature, so it belongs with the builtins (Run Code, Web Search, Memory, ...) rather than in the third-party plugin list — while its mechanics stay exactly a plugin's: - BuiltinId += 'ask_user_question' (documented exception: a native TOOL, not a capability; selection reads agent.tools, the toggle emits tool-add/remove patches instead of a capability field) - buildCatalog surfaces it as a builtin gated on the same signals as before (tools capability on + the server lists the plugin, i.e. not admin-filtered) and skips it in the plugin loop so it never double-lists - On-theme icon: lucide MessageCircleQuestion in a teal chip via the builtin icon map, matching the other native entries; the bespoke purple SVG and the manifest icon field are gone - i18n'd name/description keys like the other builtins * feat: composer popover for answering questions (mentions-style) Answering moves to the composer, matching the existing mentions/prompts popover pattern: while an ask_user_question pause is live, a popover anchors above the textarea with the question as its header, numbered option rows (hover/click, or ↑/↓ + Enter from the empty composer), and an × to dismiss. The main textarea doubles as the free-form answer — its placeholder flips to 'Something else...' and form submit routes the text to the paused run as the answer instead of starting a new turn. Dismissing (× or Escape) restores normal sends; the inline transcript surfaces stay as before (interactive card while paused, durable Q&A record after) so the question remains visible in history. - findLiveAskUserQuestion (pure, spec'd): newest unanswered synthetic part across the conversation IS the popover signal — applied on on_pending_action, stripped on answer submit, so visibility tracks the pause lifecycle with no extra state - useLiveAskUserQuestion hook shared by the popover and ChatForm; dismissals in a recoil atom so both react - popover only mounts on the primary composer (index 0), mirroring QuoteButton * feat: number-key selection + return glyph in the question popover Pressing 1-9 in the empty composer picks the matching option directly, mirroring the numbered row chips; the highlighted row shows a return-key glyph as the Enter affordance. Same empty-composer guard as the arrow keys — typing a free-form answer is never intercepted. * refactor: first-class composer answer mode (useAskAnswerMode) Replaces the bolted-on integration (inline onSubmit interception + raw capture-phase keydown listeners on the textarea ref) with a single hook that owns the whole answer mode: live-question derivation, dismissal + highlighted option (shared recoil state), option selection, free-form submit routing (submitText returns whether it consumed the submission), and keyboard handling (handleKeyDown returns whether it consumed the key, composed ahead of the textarea's normal handler — no more addEventListener). The popover is now pure rendering off the hook; ChatForm wires placeholder, onKeyDown, and onSubmit through the same instance. Deliberately scoped to the composer rather than useSubmitMessage: starters/prompt-commands keep new-turn semantics (and the existing job-replacement behavior while paused). * fix: Codex round 2 — inline answer input, approval exemption, pause-time args F1 (composer submit unreachable while paused — isSubmitting keeps Stop shown and useTextarea eats Enter): redesigned around it, borrowing Claude Code's AskUserQuestion semantics. The popover now owns free-form input via an inline 'Other' row (numbered last, 'Something else…'), with select-then-confirm rows (click/arrows/digits highlight; Submit ↵, Enter, or double-click fires; Skip dismisses). The composer returns to being a plain composer — no placeholder swap, no submit interception; Stop keeps meaning stop. F2: ask_user_question is exempt from the tool-approval prompt unless the admin explicitly lists it (allow/ask/deny all win) — approving the right to ask a question was a pure double pause; the tool is side-effect-free. F3: the question is stamped onto the paused ask tool_call's args at PAUSE time (attachAskUserQuestionArgs in handleRunInterrupt), so abandoned/expired/ stopped turns persist with the question intact and the record card can render it — previously only the answer-resume path stamped args. * fix: fold model-supplied 'Other' options into the inline free-form row The model can generate its own catch-all option ('Other (type your own)', value 'other'), duplicating the popover's built-in free-form row — two other-ish rows, one pickable as a literal answer. Two layers: - Tool description now tells the model NOT to include catch-all options (the answer UI always offers free-form input on its own) - splitOtherOption (pure, spec'd) folds a catch-all option that arrives anyway out of the choice rows and uses its label as the inline input's placeholder — conservative match (value 'other', or a label reading as a free-form invitation), no false positives on real choices * fix: single question surface + clean free-form-only popover Two live-pause confusions: (1) the inline transcript card and the composer popover both rendered — the card now defers while the popover is up for its action, returning as the fallback surface when the user dismisses the popover (and in contexts without a ChatContext, where the popover can't exist); (2) an options-less question showed a pointless numbered '1 Something else…' row — free-form-only questions now render the inline input alone, with the 'Type your answer…' placeholder (a folded model 'Other' label still wins). * feat: the composer is the free-form answer box (like the main chat input) While a question pause is live, the main chat textarea composes the free-form answer — placeholder swaps to 'Something else…' (or a folded model 'Other' label), Enter with text submits the answer through answer-mode key handling (composed BEFORE useTextarea's submitting-lock, so the lock can't swallow it), and the Stop button swaps to Send (enabled despite isSubmitting) per the select-then-confirm design. The popover slims to the question header, numbered option rows, and Skip/Submit — its inline input is gone since the composer owns free-form now. Dismissing the popover restores normal composer semantics (Stop button, normal sends). * fix: Codex round 3 + real Skip semantics - Skip now ANSWERS instead of hiding UI (danny): it resumes the run with a decline notice ('The user chose not to answer this question.') so the model moves on — a client-side dismiss left the run paused until expiry, a hung turn. × / Escape remain pure dismiss (switch to the inline card surface). - P1 (resumed approval tool indices): resumed tool_calls steps whose tool_call id matches a seeded UNRESOLVED part now rebind to that seeded slot instead of offsetting — the original part resolves in place (output attaches) and no duplicate appears; message steps keep the offset, so the text-loss fix stands. createContentIndexOffsetHandlers now takes the seed array; resolved seeded calls are not rebind targets. - P2 (stale selection across questions): selection state resets when the live actionId changes; the vestigial inline-Other state ('other' selection + text atom) is gone — the composer owns free-form. - P2 (Redis abort path loses the args stamp): the abort route re-stamps the question onto the ask tool_call in the reconstructed abort content, so a Stop-abandoned question persists with its question intact. - P2 (malformed args crash): parseAskUserQuestionArgs normalizes untrusted shapes (options: {} / non-string entries) instead of throwing in render. * feat: free-form hint in the question popover footer Left-aligned in the footer row (opposite Skip/Submit): 'Or type your answer below' — points open-ended answering at the composer, whose placeholder already reads 'Something else…'. * feat: preserve composer drafts across the answer-mode swap The answer phase gets its own draft key (ask-answer:<actionId>), passed as a draftId override into useAutoSave — the key change itself drives the existing save/restore machinery, so the conversation draft (or mid-run PENDING draft) is stashed when a question pause takes the composer and restored once the user answers, skips, or dismisses. Ask keys are exempt from the PENDING migration branch, which would otherwise move-and-delete the stashed draft. A half-typed answer survives reload/navigation while its question stays live. Answer submission (option pick, free-form, skip) resets the composer via a new non-throwing useOptionalChatFormContext, so the swap-back restores into an empty box even outside ChatView-less render contexts (Share/search). * fix: rebind resumed steps for ALL seeded tool call ids The resume controller pre-stamps the user's answer onto the seeded ask_user_question part, so the unresolved-only rebind predicate treated it as settled and shifted the tool's re-run step to a fresh offset slot, leaving a duplicate ask record in streamed/saved content. Tool call ids are provider-minted per call: a resumed step bearing a seeded id can only be the interrupted batch re-executing, so rebinding every seeded id is always correct. * feat: popover UX round 4 — clickable hint, collapse, click-submit, multiSelect - Footer hint is a button that focuses the composer; reads 'Type your answer below' (no 'Or') when the question has no options. - Collapse (chevron) hides the popover WITHOUT closing the pause: answer mode stays live (placeholder, Enter routing, draft key), the chat card renders the question with a ChevronUp affordance to re-expand. x remains dismiss. - Single-select options submit on a single click; the Submit button renders only for multi-select. - multiSelect end-to-end: tool zod schema + JSON definition twin, wire type, client parse, popover check-chips, card toggles, record-card label mapping; answer = option values joined ', '; composer Enter and the multi Submit button both fold free-form text in with the checked values. - Hardening from adversarial review: in-flight status guard on every submit path (no duplicate resumes on double-click), popover locks while submitting, collapsed mode disarms invisible digit/arrow steering, the card shares the hook's checked state while the pause is live, the card folds catch-all 'Other' options, record mapping is all-or-nothing to avoid phantom labels, composer resets only when its text was consumed or the draft machinery will restore the stash. * feat: ask_user_question in model specs and ephemeral agents A librechat.yaml modelSpec can now equip the tool the same way it equips webSearch/executeCode/fileSearch/memory: modelSpecs: list: - name: my-spec askUserQuestion: true loadEphemeralAgent pushes the tool name when the spec flag (or the ephemeralAgent request flag, wired for parity) is set; everything downstream is the existing persisted-agent machinery — createRun's hitlCapable gating, graphTools injection, checkpointer attach, subagent strip, and the admin filteredTools/includedTools kill switch all apply unchanged. * feat: tense-aware Q&A record label (Asking / Asked) Shorten the record card header per feedback: 'Asking' while the question is still unanswered (abandoned/awaiting), 'Asked' once answered — replacing the single 'Asked a question' label. * fix: Codex round 4 — added-agent ask parity + preserve answer on failed resume F1 (added.ts): mirror loadEphemeralAgent's ask_user_question branch in the added-agent loader so a model spec's askUserQuestion flag (or the ephemeral request flag) equips added top-level agents too, matching execute_code / web_search / memory. Two load.spec cases added. F3 (composer): submitAskAnswer now takes an onSuccess callback and useAskAnswerMode defers clearing the selection/composer until the resume is accepted. A failed resume (16k answer-cap 400, expired action, network error) leaves status re-answerable, so wiping the composer up front lost the user's only copy of a free-form answer; now it survives for trim/retry. (F2 — a claimed Tools-capability bypass — was verified NOT reproducible: agentRequestsAskUserQuestion matches only loaded instances/toolDefinitions/ toolRegistry, all capability-filtered; a raw tools string has no .name and never triggers the install. Replied on-thread with the probe evidence.) * fix: Codex round 5 — expired question exits answer mode so its message shows An expired question (e.g. resume returns the stale-action 409) previously left the popover open with locked controls and no explanation, because the chat card — which carries the only 'this action expired' message — was suppressed by the popover-open guard. Treat 'expired' as no longer active: the popover closes, the composer reverts to normal, and the card becomes the sole surface and renders the expired message. 'error' stays active (retryable). * feat: group ask_user_question calls as their own category A homogeneous group of ask_user_question tool calls now reads 'Asked N questions' (present tense 'Asking N questions' while the turn streams) with a question glyph and no raw-name suffix — mirroring the subagent 'Ran N agents' category treatment, instead of 'Used N tools — ask_user_question'. Mixed groups keep 'Used N tools' but humanize the suffix to 'Question' and show a question icon for the ask entries (TOOL_FRIENDLY_NAME_KEYS + ToolIcon map). A group only forms at count >= 2, so the plural is always grammatical. Three ToolCallGroup.test cases cover homogeneous label/icon/suffix, present tense while streaming, and the mixed-group fallback. * fix: Codex round 6 — composer submit lock + abort stamp before emit F7 (composer status lock): the ask submit status lived on ApprovalContext, a React context mounted only around message content (ContentParts). The PRIMARY answer surface — the composer in ChatForm — renders outside it, so useApprovalContext returned the inert FALLBACK: status was always 'idle', setStatus a no-op. The in-flight double-submit guard (round 4) and the expired-exits-answer-mode fix (round 5) therefore never engaged for the composer. Move ask submit status to a global Recoil atom (useAskSubmitStatus) read/written by the composer, the popover, and the card alike, so a fast double-click/Enter is actually blocked and expired/error surfaces on every surface. Tool-approval status stays on the context (unchanged). F5 (abort stamp before emit): the abort route re-stamped a paused ask_user_question's args AFTER GenerationJobManager.abortJob had already emitted the final SSE from the unstamped content, so a Redis/cross-replica Stop left the live client showing an empty question until reload. abortJob now takes an optional transformAbortContent applied to the persistable content BEFORE the final event is built (and returned), so the live client and the saved message agree. New abort.spec case + updated call assertions. * feat: gate ask_user_question behind its own agent capability Add a first-class AgentCapabilities.ask_user_question (in defaultAgentCapabilities, on by default) so admins can enable/disable questions independently via endpoints.agents.capabilities, exactly like execute_code / web_search — not lumped under the generic tools capability. - ToolService: both filteredTools predicates (definitions-only and instance loaders) gate ask_user_question on checkCapability(ask_user_question) before the generic tools fallthrough. When off, the tool is dropped from toolDefinitions/toolRegistry, so run.ts's agentRequestsAskUserQuestion (which keys on the loaded surface) declines to install it and attach a checkpointer — the capability is enforced end-to-end at the loader, no run.ts change needed. - Tools dialog catalog: surface the ask builtin under its own capability rather than the generic tools one, so the UI matches the backend gate. - Tests: ToolService capability on/off filtering + defaults membership; catalog builtin visibility keyed on the dedicated capability. * style: sort imports in ToolCallGroup.test (CI import-order gate) * fix: Codex round 7 — surface ask-answer errors in the open popover A failed answer submission (16k reject, network error) sets the ask status to 'error', which — unlike 'expired' — deliberately keeps the question active and retryable. But the chat card that renders the error message is suppressed while the popover is open, so a composer/popover answer failed silently. Expose an 'errored' flag from useAskAnswerMode and render a warning line (com_ui_ask_answer_error) in the popover, so the user gets feedback and retry guidance without having to collapse/dismiss. It clears automatically on retry (status flips to 'submitting'). * fix: Codex round 8 — respect IME composition before submitting answers handleComposerKeyDown runs before useTextarea's composition guard, so with a CJK/IME keyboard the Enter that commits an in-progress composition was being intercepted and submitting the partial answer (and the composition buffer can leave value empty mid-compose, mis-triggering digit/arrow steering too). Bail at the top when composing — nativeEvent.isComposing, or key==='Process' / keyCode===229 for Safari's inconsistent reporting — mirroring the existing composer guard so the character commits normally. * chore: update `@librechat/agents` to v3.2.60 * 🔧 chore: Update @opentelemetry/core to version 2.9.0 and clean up package-lock.json * feat: digit shortcuts select options when the popover has focus Previously a number key (1..N) only selected an option from the empty composer (handleComposerKeyDown on the textarea) — if focus moved into the popover (a row/Skip/Submit button clicked or tabbed to), the number keys went dead. Add handlePopoverKeyDown, wired to the popover container's onKeyDown so it catches digits bubbling from the focused control: a digit activates its option exactly like a click (single-select submits, multi toggles). No highlight/Enter dance on this path — the options are buttons whose action is the click, and intercepting Enter would fight the focused button. Gated on active && !locked so it no-ops while a submit is in flight. * chore: update @librechat/agents to version 3.2.61 and @opentelemetry packages to latest versions |
||
|
Danny Avila
|
dcdaaeac67
|
🤐 fix: Exclude Provider Secrets from HITL Pending Actions (#14136)
* 🤐 fix: Exclude Provider Secrets from HITL Pending Actions Sanitize resolved model parameters before persisting them in the pending action's resumeContext, and strip resumeContext/requestFingerprint from every client-facing copy (SSE emit, reconnect gap-fill, resume state, status route). The full record stays server-side for resume replay. * 🤐 fix: Treat Header-Carrier Keys as Sensitive Wholesale Google llmConfig places the Authorization header in customHeaders, and header names like Ocp-Apim-Subscription-Key defeat name heuristics. Match 'header' as a key fragment so every header-carrier object is dropped rather than relying on exact carrier names. * 🤐 fix: Strip Preconfigured Client Instances from Resume Params Bedrock stores a BedrockRuntimeClient on llmConfig.client when PROXY or a bearer token is configured; replaying it would also fold a mangled client object into additionalModelRequestFields on resume. |
||
|
Danny Avila
|
ed8547018c
|
⚡ perf: Persist HITL checkpoints only on pause (lazy checkpointer) (#14024)
* ⚡ feat: Persist HITL checkpoints only on pause (skip clean-exit writes) With `durability: 'exit'` (set by the SDK whenever a checkpointer is active) LangGraph persists ONE checkpoint at the exit boundary on EVERY run — paused or not. So a non-paused HITL turn writes a dead checkpoint whose only fate is to be pruned by deleteAgentCheckpoint: pure write+delete churn on the common path, given HITL only ever resumes an *interrupt* checkpoint. `InterruptOnlyMongoSaver` (a MongoDBSaver subclass) persists only interrupt checkpoints and discards clean-exit ones, so a non-paused turn writes nothing. How it tells them apart (verified empirically against @langchain/langgraph, not docs): when a run interrupts, the runner calls `putWrites` with the `INTERRUPT` ("__interrupt__") channel for the checkpoint it's about to create, and that write's `config.checkpoint_id` equals the `checkpoint.id` of the `put` that immediately follows. A clean exit calls `put` with no preceding interrupt `putWrites`. So we record the checkpoint id of any interrupt `putWrites` and persist a `put` only when its `checkpoint.id` was so marked. Keying on the globally-unique checkpoint id (not thread_id) keeps this correct even when two runs race on the same conversation (the job-replacement scenario). Correctness is preserved end-to-end: interrupt checkpoints + their pending writes persist exactly as before (resume unchanged); clean checkpoints were only ever written-then-pruned, so not writing them is observationally equivalent. The eager prune stays as the backstop. Tests (mongodb-memory-server): a bare put() is discarded; an interrupt-seeded checkpoint is persisted with its __interrupt__ pending write; and an end-to-end real-graph run writes 0 checkpoints on a clean completion and a resumable one on interrupt. NOTE: a non-paused turn's deleteAgentCheckpoint now finds nothing to delete (a 0-match no-op) — a follow-up can skip that call entirely once the lingering-abandoned-pause cleanup role is reassigned to the TTL + expiry sweeper. * ⚡ feat: Drop the redundant clean-path checkpoint prune With the lazy checkpointer (InterruptOnlyMongoSaver) a non-paused turn no longer writes a clean-exit checkpoint, so the post-completion prune in chatCompletion's finally had nothing left to delete. It was also already redundant: every fresh turn runs a pre-run prune (`deleteAgentCheckpoint` before `processStream`) that clears any checkpoint orphaned by a prior abandoned pause — verified empirically that a lingering interrupt checkpoint WOULD otherwise poison a fresh turn (LangGraph continues the abandoned state + re-interrupts), and that the pre-run prune is what prevents it. The Mongo TTL remains the backstop, and the resume path still prunes after a successful finalize. Removing the clean-path prune also deletes its job-replacement race surface (round-17 F21): an older run's late finally can no longer delete a newer paused run's checkpoint, because there is no longer a clean-path prune to race. Dropped the now-dead F21 predicate test. Net per non-paused HITL turn: from {pre-run prune + checkpoint write + post-run prune} down to {pre-run prune} — no write, no post-completion delete. * 🛡️ fix: Anchor any pending-write checkpoint; stale-only eviction (Codex) Broaden the lazy saver's keep-rule from "interrupt-only" to "persist any checkpoint that carries pending writes" (renamed InterruptOnlyMongoSaver → LazyMongoSaver). This makes it robust to delta-channel graphs without changing behavior for LibreChat's graph: - K1 (P1): a delta-channel graph can write a synthetic PARENT/anchor checkpoint (no __interrupt__ mark) that the interrupt checkpoint then points at, with the delta writes stored under the parent id. The old rule discarded that parent, breaking delta-state resume. Now any checkpoint that received putWrites is persisted, so the anchor parent and its writes survive and resume can walk the chain. - K3 (P2): for the same reason, clean delta-write rows are no longer orphaned — their checkpoint is persisted alongside them. (For LibreChat's standard Annotation/messages graph a clean run makes no putWrites at all — verified empirically — so the common path still writes nothing and the optimization is unchanged.) - K2 (P2): the 1024 FIFO cap could evict a valid in-flight id whose put() was just behind Mongo I/O, mis-classifying its interrupt checkpoint as a clean exit. Replaced with time-based eviction: only ids older than 5 min (a put always follows its putWrites within ms) are swept; a recent in-flight id is never dropped, and the map grows rather than evict a valid id if nothing is stale. New integration test: a checkpoint anchored by a NON-interrupt write is persisted. Full agents/HITL suites green (108). * style(checkpointer): fix import order to satisfy sort-imports CI * fix(checkpointer): don't persist failed-turn (error-only) checkpoints LazyMongoSaver anchored on ANY pending write, so a non-paused turn that errors (LangGraph records an __error__ write then a put) was persisted and, with the clean-path prune removed, lingered until the next fresh-turn prune or the Mongo TTL. Anchor only on resumable writes — INTERRUPT or a real (non-__-prefixed) state/delta channel — so error/bookkeeping-only checkpoints are discarded at the source. Addresses Codex P3. Codex P2 (delta-stub parent orphan) is not reachable: the SDK graph uses standard Annotation/MessagesAnnotation channels (no DeltaChannel), and under durability:'exit' putWrites precedes put with a parentless boundary checkpoint — probe-confirmed against @langchain/langgraph@1.4. Documented the durability:'exit' invariant the saver depends on. Tests: error-only put discarded; e2e throwing graph persists 0 checkpoints. * fix(checkpointer): drop bookkeeping-only write batches, not just the checkpoint The prior fix stopped the failed-turn CHECKPOINT from persisting, but putWrites still forwarded the __error__ batch to MongoDBSaver.putWrites — writing a row to agent_checkpoint_writes whose parent checkpoint is then discarded. With the post-run deleteThread removed, that orphan row lingered until the Mongo TTL or the conversation's next pre-run prune. putWrites now drops a non-resumable (bookkeeping-only) batch entirely instead of forwarding it. Probed against a real MongoDBSaver (mongodb-memory-server): a throwing graph now leaves 0 checkpoints AND 0 write rows (was 0 + 1 orphan), while interrupt->resume is unaffected — the __interrupt__ write is resumable so it is still forwarded. Addresses Codex P2 (round 3). Tests: error-only put leaves no checkpoint and no write row; e2e throwing graph leaves both collections empty; new e2e interrupt->resume completes with the approval value. * fix(checkpointer): un-anchor a checkpoint whose putWrites failed; freshen comments Self-review findings on the converged PR: 1. LangGraph dispatches put() concurrently with putWrites (probe-confirmed on 1.4.5), and put() still completes when putWrites rejects — so a transient Mongo failure during the interrupt write could persist a checkpoint whose __interrupt__ row is missing (an unresumable phantom pause). putWrites now deletes the write anchor on rejection (best-effort) and rethrows, so that put() discards the checkpoint instead. The pre-recorded anchor stays where it is — recording after the await would drop slow-I/O interrupts on the success path, which the same probe showed is reachable. 2. Renamed leftovers: two comments still said InterruptOnlyMongoSaver; the class is LazyMongoSaver. 3. Documented why the pre-run prune is deliberately unconditional per HITL turn (any cheaper gate can go stale across replicas and skip the prune exactly when an orphaned interrupt exists). Test: failed putWrites → subsequent put persists nothing (14/14 green). * fix(checkpointer): bookkeeping write batches follow their checkpoint's fate The round-3 rule dropped bookkeeping-only putWrites batches (__error__/ __resume__/__no_writes__) unconditionally — batch-scoped, when the decision must be checkpoint-scoped. Probe-confirmed (langgraph 1.4.5, durability:'exit'): a Send fan-out that pauses on one sibling records the completed siblings as pure __no_writes__ batches on the RETAINED interrupt checkpoint; dropping those markers makes resume re-execute the completed siblings (side effects measured twice). Addresses Codex M2 (P2). putWrites now PARKS a bookkeeping-only batch in memory until the checkpoint's fate is known: forwarded when the checkpoint is anchored (or was just persisted — put is dispatched concurrently), dropped when put discards it. Net: an errored turn still leaves nothing durable (0 checkpoints, 0 write rows), and a retained checkpoint stores byte-for-byte what a plain MongoDBSaver would. Codex M1 (__resume__ lost on re-pause) did not reproduce: the re-pause emits [__interrupt__,__resume__] as ONE batch (anchored, forwarded whole) and a second resume on a rebuilt graph replays both answers correctly — but the fate-scoped buffering now covers a lone __resume__ batch in any ordering too. Tests: bookkeeping preserved on a retained checkpoint in either arrival order; e2e Send-sibling pause/resume with side-effect counters (was {a:2,c:2} under the drop rule, now {a:1,c:1}); error-only turn still leaves both collections empty. 16/16 green. |
||
|
Danny Avila
|
6dbf9d5ad3
|
🪝 feat: Human-in-the-Loop Runtime - Tool Approval + Ask-User-Question (Slice B) (#13942)
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
* chore: add @langchain/langgraph-checkpoint-mongodb for HITL durable resume
* feat: HITL tool approval runtime — backend (Slice B)
- endpoints.agents.checkpointer config + durable Mongo checkpointer (seam over the app
connection; SDK MemorySaver fallback) with a TTL index + deleteThread pruning
- HITL run wiring (PreToolUse policy hook + humanInTheLoop) attached in createRun, fully
inert when toolApproval.enabled is off
- interrupt gate (pause job -> requires_action + emit on_pending_action) and a resume
route that rebuilds the run from the durable checkpoint and run.resume()s it
- atomic single-winner resolve; agent-consistency guard; expireStaleApprovals terminal
event; checkpoint pruned on every non-paused completion (thread_id == conversationId)
* feat: HITL tool approval UI — frontend (Slice B)
approve/reject/edit/respond + ask-user controls in the tool card (OAuth-button precedent),
batch-aware single submit, live + reconnect (resumeState.pendingAction) wiring, and resume
mutations posting to /agents/chat/resume.
* fix(hitl): decouple ApprovalProvider from chat context
ApprovalProvider is now pure state (safe to mount in provider-less / shared / test
renders); the context-dependent submit moved to a useResumeSubmit hook the cards call.
Part imports getAskUserQuestionPart from ~/utils/approval directly so suites that
partial-mock ~/utils render Part without throwing.
* fix(hitl): address Codex review — backend
- P1: enforce per-tool allowed_decisions on resume (reject a crafted decision the
policy disallows) via findDisallowedDecisions
- prune the durable checkpoint on user-abort of a paused run, and before a fresh
HITL turn, so a new turn cannot rehydrate an expired/aborted interrupt (thread_id
is the stable conversationId)
- persist + use isTemporary and the original parentMessageId on resume (temporary
chats stay temporary; initializeAgent scopes thread files off the right parent)
- generate a deferred first-turn title BEFORE completeJob so its event reaches the
client and the final event carries the real title
- moderateText: skip when there is no text (tool-approval resume) and moderate the
ask-user answer, instead of denying on an empty input
* fix(hitl): address Codex review — frontend
- render ToolApproval for ANY paused agent tool card (bash/code/file/etc.), not just
the generic ToolCall, by wrapping the tool-card branch in Part (moved the rendering
out of ToolCall)
- findPendingActionMessageIndex only matches an assistant message, never the user
message (the underscore-strip could target the user bubble before the assistant
placeholder exists)
* fix(hitl): address Codex re-review
- title eligibility checks the user message’s parent (first turn), not the response’s
parent — the previous check could never be true and skipped title generation
- use client.buildResponseMetadata() for the resumed message so contextUsage /
thoughtSignatures survive (the abort-only helper dropped them)
- moderate decisions[].responseText (the respond action’s user text)
- give /chat/abort req.config (configMiddleware) so the HITL checkpoint prune on abort
actually runs
- read resume state BEFORE setContentParts so the in-memory store does not lose the
pre-pause seed content
- count resumes against LIMIT_CONCURRENT_MESSAGES (increment/decrement) so paused-then-
resumed turns cannot bypass the limit
- require actionId on resume so a body without it cannot resolve the current action
* fix(hitl): address Codex re-review (round 3) — resume fidelity
Bring the lean resume path to parity with sendMessage for things it bypassed:
- carry userMCPAuthMap into the rebuilt run so approved MCP tools keep the user's creds
- seed initialSessions (buildInitialToolSessions) so approved code/file/skill tools have
the pre-pause uploaded-file context (esp. cross-replica / after restart)
- await client.artifactPromises and persist them as response attachments (else tool
artifacts created after the pause vanish on reload / for late subscribers)
- merge metadata: cumulative usage (+ summary marker) from the job, contextUsage /
thoughtSignatures from the client — fixes the round-2 regression that underreported
post-resume cost
* fix(hitl): address Codex re-review (round 4) — resume hardening
- resume: require an EXACT paused agent_id match (reject omitted/ephemeral
agent_id, not just a different one) and reject an endpoint mismatch, so a
request can't rebuild the claimed checkpoint on a different graph
- moderateText: also moderate a tool-approval decision's reject `reason` and
stringified `editedArguments`, not just `responseText`
- request: re-mark the paused response `unfinished:true` after BaseClient saves
it as completed, so an expired / never-resumed approval doesn't leave a
"finished" response in history; the resume path overwrites it on success
* test(hitl): route-level integration test for the resume controller
Adds api/server/controllers/agents/__tests__/resume.spec.js, a supertest
integration test that drives the real ResumeAgentController over the full
pause -> approve -> resume -> finalize lifecycle with the SDK run, durable
checkpointer, Mongo, and concurrency cache mocked. The pure decision/liveness
helpers run for real via requireActual, so the guard ladder is exercised end to
end rather than stubbed.
25 cases covering:
- the authorization / staleness / agent-and-endpoint / actionId guard ladder
- tool_approval validation (undecided tool call, policy-disallowed decision)
- ask_user_question answer requirement
- the concurrency gate (429) and the atomic single-winner claim (409)
- the happy path: ACK, run reconstruction, decision->SDK mapping, finalize
(save the now-finished response, emit done, complete job, prune checkpoint)
- first-turn title generation before stream completion
- re-pause (no double finalize), abort-during-resume (no double finalize),
and the resume-failure terminal path (emitError + completeJob + prune)
* test(hitl): strengthen resume coverage + add approval util tests
Acts on a self-audit of the new resume integration test.
resume.spec.js (25 -> 32 cases):
- replace the tautological emitDone assertion (it only checked the hardcoded
`final: true`) with a structural check of the finalEvent payload —
responseMessage content/id/unfinished, requestMessage identity, title
- cover the previously-unwalked finalize branches: tool-artifact attachments
(null-filtered), the aggregatedContent fallback when live content is empty,
and client response-metadata attachment
- add guard cases: unsupported pending-action type (400) and the
pre-multi-tenancy null-tenantId pass-through (must not 403)
- add error-path cases: first-turn title generation throwing must still
finalize, and a completeJob failure during a resume error must force a
terminal job state via the last-resort updateJob
client/src/utils/approval.spec.ts (new, 15 cases):
- applyPendingAction tool_approval: join by tool_call_id not position,
skip completed calls, default allowed_decisions to [], referential
stability when nothing changes
- applyPendingAction ask_user_question: append, idempotent replace on replay,
non-array content coercion
- getAskUserQuestionPart type guard; findPendingActionMessageIndex
assistant-only resolution (never resolves to the user bubble)
* fix(hitl): address Codex re-review (round 5)
Five findings verified against the code before fixing:
- resume: require an EXACT endpoint match (like agent_id) — a resume that OMITS
endpoint must not fall through, since the shared chat middleware treats a
missing/non-agents endpoint as the ephemeral agent and could rebuild the
claimed checkpoint on a different graph
- resume: filter malformed content parts before saving the finished response,
matching the normal AgentClient path (a resumed turn could otherwise persist
an empty/invalid tool_call part that breaks reload/rendering)
- resume: accumulate tool artifacts across pause segments — persist them on
re-pause and MERGE (not overwrite) at finalize, so artifacts produced before
a second approval pause aren't dropped by the next rebuilt client
- approval (client): findPendingActionMessageIndex returns -1 when a provided
responseMessageId isn't found, so the caller retries instead of attaching the
prompt/approval to a prior assistant reply; fall back to the last assistant
only when no responseMessageId is given
- RedisJobStore: make appendChunk extend-only (XADD + EXPIRE-if-shorter via a
single eval) so the on_pending_action chunk emitted after a pause can't reset
the chunk-stream TTL back to the running window and evict pre-pause content
before the approval is resolved
Tests: +endpoint-omitted/unsupported-type/malformed-filter/attachment-merge/
re-pause-persist cases in resume.spec.js (36); ask-retry -1 semantics in
approval.spec.ts (16); extend-only TTL assertion in the RedisJobStore Redis
integration spec.
* test(hitl): mongodb-memory-server integration test for the checkpointer seam
The checkpointer unit spec covers config/selection with no DB connection; this
exercises the durable Mongo seam against a real (in-memory) MongoDB — the part
correctness actually depends on:
- getAgentCheckpointer builds a real MongoDBSaver when Mongo is connected and
setup() creates the TTL index (expireAfterSeconds) on the checkpoint collection
- memory type returns undefined (SDK MemorySaver fallback) even when connected
- saver is memoized per resolved config
- deleteAgentCheckpoint prunes a thread's persisted checkpoint (the cross-turn
isolation guarantee: turn N+1 on the same conversationId can't rehydrate it)
- pruning is thread-scoped — deleting one conversation leaves others intact
- undefined threadId is a no-op
* fix(hitl): address Codex re-review (round 6)
Four findings verified against the code before fixing:
- messageFilterPii: scan the resume payload's user-authored text (ask-user
`answer`, and a tool-approval decision's `respond` text, `reject` reason, and
edited tool arguments) — the shared /resume route ran through the PII filter
but it only inspected req.body.text, so a blocked token rode the resume
payload back into the model/tool (mirrors the earlier moderateText fix)
- resume: re-prime skill files invoked in the pre-pause segment before rebuilding
the run, so an approved code/file-backed tool keeps the injected skill-file
session refs instead of running without them (mirrors the normal path's
primeInvokedSkills; the pre-pause content stands in for the message payload)
- hitl: pin the graph identity. Persist a fingerprint of the graph-determining
request fields (endpoint, agent_id, model, spec, ephemeralAgent — normalized)
on the pending action at pause, and reject a resume whose recomputed
fingerprint differs. This closes the ephemeral-agent gap, where agent_id is
undefined so the id guard can't tell two ephemeral configs apart
- resume: reject incomplete edit/respond decisions (findIncompleteDecisions) —
an `edit` without an object editedArguments or a `respond` without non-empty
responseText is 400'd before mapping, rather than defaulting to {} / '' and
resuming with behavior the user never approved
Tests: incomplete-decision + fingerprint match/mismatch cases in resume.spec.js
(41); findIncompleteDecisions + computeAgentRequestFingerprint unit tests; and
resume-field PII cases in messageFilterPii.spec.ts.
* fix(hitl): address Codex re-review (round 7)
Four findings verified against the code before fixing:
- RedisJobStore: clear `agent_id` on createJob (add it to staleHitlFields). The
job hash is keyed by conversationId and reused across turns; updateMetadata
only writes agent_id when truthy, so a conversation that switched from a saved
agent to an ephemeral/no-agent turn kept the old id and the resume guard
rejected the valid pause as a different agent. (real correctness bug)
- fingerprint: include `promptPrefix` in computeAgentRequestFingerprint, and
re-send it on resume (ResumeAgentFields + buildResumeFields). Ephemeral agents
derive their system instructions from promptPrefix, so a resume changing it
previously passed the pin and rebuilt different instructions. (completes the
round-6 fingerprint)
- resume: the re-pause branch now persists the segment's accumulated CONTENT
(filtered), not just artifacts, so an approval that expires/reaps without a
final resume no longer loses everything streamed during the resumed segment.
- request: carry `manualSkills`/`alwaysAppliedSkills` on the persisted user
message so a resumed turn's reconstructed requestMessage keeps its skill pills
instead of dropping them until a full reload.
Deferred (narrow, no safe contained fix yet — see PR thread replies):
- resume rebuild without `addedConvo` for a multi-conversation/added-agent pane
- cross-replica re-prime of manually-selected (not model-invoked) skill files
Tests: stale-agent createJob clearing (Redis integration), promptPrefix
fingerprint match/mismatch (resume.spec.js + policy.spec.ts), re-pause content
persistence (resume.spec.js).
* fix(hitl): address Codex re-review (round 8)
Five findings verified against the code before fixing; the headline is a durable-
resume correctness fix (the fingerprint had surfaced it as a 403):
- resume durability (the important one): persist the graph-determining request
fields (endpoint, agent_id, model, spec, promptPrefix, ephemeralAgent) on the
pending action as `resumeContext`, and REPLAY them onto the resume request via
a router-level middleware that runs before buildEndpointOption. The client
can't reconstruct the ephemeral-agent config after a reload/cross-session, so
the round-6/7 fingerprint would 403 a valid durable resume — and even without
it the rebuilt agent would lose its tools. Replaying server-side rebuilds the
SAME graph regardless of client state (and a crafted resume can't swap it; the
fingerprint still matches because the body is restored first).
- RedisJobStore: also clear `isTemporary` on createJob (same class as agent_id):
a prior temporary turn's flag would otherwise survive a reused conversation
hash and a later non-temporary resume would save its response as temporary.
- resume: persist `contextMeta` (context-window calibration) onto the saved
response like BaseClient does, so the next turn can seed its pruner.
- request: carry manualSkills/alwaysAppliedSkills into the onStart metadata
update (not just the preliminary one it overwrites), so a resumed turn's
requestMessage keeps its skill pills.
Deferred (narrow — see thread reply):
- saved-agent edited WHILE a run is paused: agent_id matches but the definition
changed; needs an agent version/config hash, which is a larger change for a
narrow window.
Tests: resumeContext pick/apply + round-trip (policy.spec.ts), contextMeta +
manualSkills-on-requestMessage (resume.spec.js), isTemporary clearing (Redis
integration).
* style(hitl): prettier line-wrap in policy.spec.ts (R8 lint fix)
* fix(hitl): address Codex re-review (round 9)
Five findings, all fixed (addedConvo — deferred in rounds 7/8 — is now trivial
thanks to the round-8 replay):
- replay addedConvo: add it to RESUME_CONTEXT_KEYS so the resume middleware
restores the parallel/secondary-agent config from the paused request; the
client can't reconstruct it, and it determines the rebuilt graph.
- skill pills (the real fix this time): the round-8 onStart metadata write was
overwritten by trackUserMessage (the authoritative userMessage writer). Carry
manualSkills/alwaysAppliedSkills in the emitted `created` message and persist
them in trackUserMessage; widen UserMessageMeta + SerializableJobData.userMessage.
- execute-code files on resume: seed the paused user message's own files onto
req.body.files before initializeClient — they're excluded from the
parent-walk code-session rebuild, so an approved code/read-file tool would
otherwise resume without them.
- in-memory pending-action UI: route ApprovalEvents.ON_PENDING_ACTION in the
resume replay/pending-event loops to applyPendingActionToMessages (mirror the
live handler), so a pause that lands in the snapshot window still renders its
approval controls instead of sitting paused with no UI.
- abort isTemporary: the /chat/abort partial-save now sources isTemporary from
the job metadata, not req.body (the stop button posts only conversationId), so
aborting a paused temporary chat no longer persists an orphaned partial.
Tests: addedConvo in pickResumeContext (policy.spec.ts), file-restore on resume
(resume.spec.js), abort-from-job-isTemporary (abort.spec.js).
* fix(hitl): address Codex re-review (round 10) — resume/expiry races
Three concurrency/coherence findings, verified against the code before fixing:
- expiry-sweep CAS scope: both stale-approval sweeps (GenerationJobManager
expireStaleApprovals and the RedisJobStore requires_action cleanup) called
expire()/transitionStatus WITHOUT the observed pendingAction.actionId, so the
CAS only checked status===requires_action. Between the read and the CAS a user
could resolve the observed action and the run re-pause on a FRESH action; the
stale sweep would then abort that valid new pause. Now both pass the observed
actionId as expectActionId, so the CAS only fires for the action read as stale
(a re-paused action has a different id → no-op).
- resume graph cache: resumeCompletion cached the rebuilt graph (created with
messages:[]) via setGraph; RedisJobStore.getContentParts prefers a cached
graph over reconstructing from the chunk log, so a same-replica reload/status
poll mid-resume returned aggregatedContent missing the pre-pause content. Skip
setGraph on resume so introspection falls back to the complete chunk
reconstruction (setContentParts still seeds the in-memory store).
- pending-action UI: applyPendingActionToMessages scheduled a SINGLE
animation-frame retry then dropped the pending action; Recoil/React updates can
take several frames under load, leaving a valid requires_action run with no
approval controls. Retry across frames (bounded at 120) until the target
message commits.
Test: expire() with a mismatched expectedActionId no-ops while the matching id
expires (pendingAction.spec.ts).
* chore(deps): update @librechat/agents to version 3.2.53 and @langchain/langgraph to version 1.4.7 in package-lock.json and related package.json files
* refactor(hitl): add resolveToolApprovalPolicy seam for layered policy
Extract the single point where tool-approval policy is resolved for a turn
(`resolveToolApprovalPolicy`) and route the run call site through it instead
of reading `endpoints.agents.toolApproval` inline.
Behaviour-preserving: only the `endpoint` layer is wired today, so the result
is identical to reading the app policy directly. The `agent` and `skills`
layers are reserved seams with documented precedence (endpoint owns the
`enabled` kill switch; agent overrides mode/allow/deny/ask/reason; skills may
only tighten), so future per-agent and per-skill policy plumbing lands in one
function rather than at the `createRun` site. Adds focused unit tests.
* fix(hitl): address Codex re-review (round 11) — resume hardening
F1 (P2, security) — applyResumeContext now DELETES any RESUME_CONTEXT_KEY
absent from the persisted context, so the resume body carries exactly the
graph-determining fields the pause had. Previously only defined keys were
overwritten, leaving a client-supplied `addedConvo` (which the request
fingerprint does not cover) in place — a crafted resume could rebuild a
single-agent checkpoint as a different multi-agent graph/tool set.
F3 (P2) — the resume route ACKs (res.json) before initializeClient, so a
post-ACK getMCPRequestContext(req, res) saw the response as finished and
returned undefined, leaving the resumed run without its run-scoped MCP
connection store (approved MCP / OAuth-overlay tools then ran without their
request-scoped connections). Pre-seed the store with a null res +
cleanupOnResponse:false before the ACK and tear it down in the finally,
mirroring the normal stream path (request.js). userMCPAuthMap was already
preserved separately, so credentials were not lost — only the connection store.
Declined: the ApprovalContext NEW_CONVO guard (P2) is a false positive — the
`created` SSE event updates the conversation atom before any pause renders, so
the id is concrete by click time (details in the PR thread).
Tests: policy.spec (absent-key delete) + resume.spec (MCP context pre-seed/cleanup order).
* fix(hitl): address Codex re-review (round 12) — resume fidelity + multi-tool UI
F4 (P2) — temporal prompt vars: resume rebuilt the agent without restoring
req.conversationCreatedAt or req.body.timezone, so {{current_datetime}}-style
vars compiled a different system prompt than the paused graph (resume wall-clock,
unzoned). Add 'timezone' to RESUME_CONTEXT_KEYS (persisted at pause, replayed by
the resume middleware) and restore conversationCreatedAt from the convo before
initializeClient — mirroring the normal path's resolveConversationCreatedAt.
F5 (P2) — multi-tool approval: applyPendingActionToMessages stopped retrying once
ANY tool-call part was tagged, so siblings that rendered on later frames never got
approval controls and the resume route 400'd the partial batch. Add
countTaggedApprovalParts and keep the bounded RAF retry going until every
action_request is tagged (ask_user_question unchanged — one synthetic part).
F6 (P3) — Edit accepted `null`/`[]` (valid JSON, non-object), enabling Submit for
a value the resume route rejects via findIncompleteDecisions. Mirror the server's
plain-object check in the client (store + editIsValid) so Submit only enables for
an accepted value.
Tests: policy.spec (timezone round-trip), resume.spec (conversationCreatedAt
restore), approval.spec (countTaggedApprovalParts).
* fix(hitl): address Codex re-review (round 13) — recurse into subagent approvals
F9 (P2) — a tool paused INSIDE a subagent has its tool_call_id in the parent
subagent tool_call's nested `subagent_content`, not as a top-level message part.
applyToolApproval and countTaggedApprovalParts only scanned top-level content, so
the approval never attached and the round-12 retry loop counted 0 tagged parts and
spun to its frame cap with no controls. Both now recurse into `subagent_content`
(immutably, so React refs update): the nested call gets tagged and is counted, so
the retry terminates. Added approval.spec cases for the nested tag + count.
Note: surfacing the interactive approve/reject controls inside the subagent view is
a deliberate follow-up — ToolApproval -> useResumeSubmit -> useChatContext crashes
when rendered in the portaled subagent dialog (outside the chat/approval providers),
so that needs the controls scoped to the in-provider inline render (or the dialog
wrapped with the providers). This commit fixes the data/traversal layer only.
F7 (discovered-tool history on resume) and F8 (redis chunk TTL pause race) were
verified false positives — see the PR threads.
* fix(hitl): address Codex re-review (round 14) — resume fidelity + expiry relay
F13 (P2) — manualSkills are graph-determining (skill allowed-tools union into the
tool set before tools load) but weren't replayed, so a reload lost the skill tools
and a crafted resume could inject a different skill past the fingerprint. Add
'manualSkills' to RESUME_CONTEXT_KEYS (same replay-only pattern as timezone/
addedConvo; the delete-absent half blocks injection). Not alwaysAppliedSkills —
that's resolved server-side from the DB, not req.body.
F12 (P2) — the resume final SSE built requestMessage from job.metadata.userMessage
(persisted without files), so attachments vanished from the user bubble on resume.
Spread the already-restored req.body.files onto it, matching the normal path.
F11 (P2) — multi-replica approval expiry: RedisJobStore.cleanupRequiresActionIndex
on another replica can win the requires_action->aborted CAS (it sets the hash error
but has no event transport), and the local sweep then skips because the job is no
longer requires_action, so a client subscribed here never gets the terminal error
until the reap path. expireStaleApprovals now relays APPROVAL_EXPIRED_ERROR for a
locally-subscribed job already aborted FOR approval expiry (error-string gated,
idempotent via the errorEvent flag). emitError already publishes cross-replica.
Tests: policy.spec (manualSkills round-trip + inject-drop), resume.spec (final
requestMessage carries restored files).
* fix(hitl): render approval controls for subagent-nested tool pauses (F10)
Round-13 made applyToolApproval/countTaggedApprovalParts recurse into
subagent_content (data), but SubagentDialogPart rendered nested TOOL_CALL parts
with <ToolCall> only and never mounted <ToolApproval>, so a tool paused inside a
subagent showed no controls and the run was unresolvable.
Render <ToolApproval> in SubagentDialogPart's TOOL_CALL branch when the nested
tool_call carries an approval and isn't yet resolved, mirroring the top-level
Part.tsx render. The subagent dialog portals (OGDialog → ReactDOM.createPortal),
but React context flows through the React tree, not the DOM tree, so ToolApproval
resolves ApprovalProvider/ChatContext and the controls work + submit.
Also harden useResumeSubmit: read ChatContext via useContext (non-throwing)
instead of the throwing useChatContext wrapper, so the cards never crash when
rendered outside a ChatContext.Provider (e.g. a search/citation render that passes
chat context as a prop) — they degrade to inert (buildResumeFields returns null).
* style(hitl): re-sort run.ts imports after dev rebase
* fix(hitl): address Codex re-review (round 15) — resume content fidelity
F14 (P2) — hide_sequential_outputs was applied in chatCompletion before
saving/emitting content but not on resume, so a sequential-agent chain that
pauses for HITL and resumes persisted/emitted intermediate outputs the setting
is meant to hide. Extracted the filter into applyHideSequentialOutputsFilter()
and call it from both chatCompletion and resumeCompletion (after handleRunInterrupt,
covering the finalize + re-pause reads of client.contentParts).
F16 (P2) — on a reloaded HITL pause, the DB already holds the paused user row +
partial assistant row; useResumeOnLoad fed those as submission.messages, then
finalHandler/createdHandler appended the same pair via requestMessage/responseMessage,
duplicating the turn (buildTree doesn't dedupe children by messageId). buildSubmission-
FromResumeState now strips the paused user/response rows (by messageId, incl. the
padded/unpadded response id) from submission.messages — they're re-supplied by the
placeholders + final event. Frontend-only; live (non-reload) pause path untouched.
Deferred: F15 (collapsed-card subagent approval registration/visibility) — see thread.
Tests: client.test (filter keeps last + tool_call parts / no-op when off),
useResumeOnLoad.spec (paused pair stripped from submission.messages).
* fix(hitl): address Codex re-review (round 16) — chunk TTL, slot, job replacement
F17 (P2) — chunk-stream TTL on pause-before-chunk. CHUNK_APPEND_LUA derived its
ceiling only from the chunk key's current TTL, so when the chunks key didn't exist
at pause (fire-and-forget append in flight, or an ask-user pause before any chunk),
the on_pending_action append created the stream with only the 20m running TTL while
the approval window is 24h — content evicted before resume. The Lua now also reads
the job key (KEYS[2]); when status == requires_action it takes max(running, TTL(jobKey))
(the approval window transitionStatus set), else the running TTL. Extend-only preserved;
gated on paused status so normal runs never inflate. Both keys share {streamId} (cluster-safe).
F19 (P2) — with LIMIT_CONCURRENT_MESSAGES, the approval prompt was emitted before the
original request released its slot, so a fast Approve got /resume 429'd. handleRunInterrupt
now releases the slot (idempotent via pendingRequestReleased) right after the pause, before
the prompt; the request.js pause branch and resume.js finally only release if it didn't
(no double-release).
F20 (P2) — finalizeResumedTurn never checked the job wasn't replaced before emitDone/
completeJob/saveMessage, so a stale resume could clobber a newer turn that reused the
conversationId. Added the createdAt guard the normal request path uses (skip finalization
when the live job's createdAt != the paused job's).
Deferred: F18 (subagent_content not reconstructed on Redis resume) — joins the subagent
cluster (F15). See thread.
Tests: RedisJobStore integration (pause-before-chunk gets approval TTL; running stays short),
resume.spec (skip finalization on replacement; no double slot release on re-pause).
* 🛡️ fix: Guard HITL terminal side-effects against job replacement
Jobs are keyed by streamId == conversationId, so a new request REPLACES the
running one on the same conversation. The replaced generation's tail must not
clobber the live generation's state. Each path now re-reads the live job and
compares createdAt against the generation's captured identity before acting.
- Thread the generation's createdAt onto the client (request.js + resume.js)
as client.jobCreatedAt — the identity every guard compares against.
- handleRunInterrupt: skip approvals.pause when this run is no longer the live
job, so a stale interrupt can't flip the NEWER job to requires_action.
- chatCompletion finally: skip the checkpoint prune when replaced, so an older
run's late finally can't delete the newer run's resume checkpoint.
- resume catch-path: gate emitError/completeJob/prune behind a stillLive check
(fail-open if the read throws), mirroring finalizeResumedTurn's success guard.
- Persist the turn's uploaded files on job.metadata.userMessage (authoritative
trackUserMessage writer) and prefer them on resume over the user DB row, whose
save can still be racing a fast /resume.
Tests: 13 guard-predicate cases in jobReplacement.spec.js.
* 🔁 fix: Harden HITL resume — ownership re-check, file seeding, deferred-tool replay
Three follow-ups to the round-17 job-replacement guards (Codex review 4594099963):
- G1 (resume.js): the success-path ownership guard runs at the START of
finalizeResumedTurn, but saveMessage + first-turn title generation await long
enough for a new request to replace the job on the same conversationId. Re-read
the live job immediately before emitDone/completeJob/prune so the terminal writes
can't tear down the REPLACEMENT job — mirrors the catch-path guard.
- G2 (request.js): onStart's metadata/chunk writes that persist the turn's files
are fire-and-forget, so a fast approval could read job.metadata.userMessage before
files landed. Seed files into getPreliminaryUserMessage instead — that write is
AWAITED before the run starts, so files are durable before any interrupt can emit.
- G3 (run.ts + client.js + resume.js + IJobStore.ts): the resumed graph is rebuilt
with messages: [], so createRun's tool_search-discovery scan finds nothing. A
deferred tool discovered earlier in the turn (and targeted by the paused call) was
therefore absent from the rebuilt schema-only toolMap — resume would throw "unknown
tool" (no loadRuntimeTools fallback is wired). Capture discovered tool names at
pause via extractDiscoveredToolsFromHistory(run.getRunMessages()), persist them on
job.metadata.discoveredTools, and replay them into createRun's new discoveredToolNames
input (merged with message-extracted names, gated on hasAnyDeferredTools — inert
otherwise). A new createRun test proves the deferred tool is promoted with the replay
and absent without it (reproducing the bug).
Tests: real createRun deferred-replay suite (run-summarization.test.ts) + G1/G2/G3
guard predicates (jobReplacement.spec.js). Full suite green.
* 🔒 fix: Close HITL resume metadata + file-substitution + pause-race gaps
Four findings on the round-18 commit (Codex review 4594430222):
- H1 (P1, regression in round-18 G3): the discoveredTools captured at pause never
reached resume — three metadata allowlists dropped it: GenerationJobManager
.updateMetadata, RedisJobStore.deserializeJob, and buildJobFacade (plus the
GenerationJobMetadata type). Added discoveredTools to all four, so the deferred-tool
replay actually works end-to-end (in-memory store already kept it via Object.assign).
- H2 (P2, security): /resume honored a client-supplied `files` array, letting a crafted
client resume an approved code/read-file tool against a DIFFERENT file set than the one
approved (files aren't in the resume fingerprint/context). Resume now ALWAYS sources
files from the paused job (metadata → DB row), clearing any client-supplied set.
- H3 (P2, ephemeral fidelity): non-default model parameters (temperature, max tokens,
custom endpoint params) were lost on resume — ephemeral agents derive them from the
request body, which the resume payload omits. Capture the resolved model_parameters in
resumeContext at pause and replay them onto the body on resume (excluding `model`, which
is replayed via the fingerprinted RESUME_CONTEXT_KEYS path). Saved agents already source
these from the DB.
- H4 (P2, Redis race): a pause landing between the resume snapshot and the Pub/Sub
subscription reached neither resumeState.pendingAction nor (Redis) pendingEvents, and
approval events aren't persisted to replayEvents — the client attached to a paused job
with no approval UI. subscribeWithResume now re-reads the live job AFTER subscribing and
surfaces the pending action if the snapshot missed it (live read, no staleness).
Tests: discoveredTools metadata round-trip + subscribeWithResume re-read (pendingAction
.spec.ts); client-file substitution rejection (resume.spec.js); model-parameter replay
predicate (jobReplacement.spec.js).
* 🧹 fix: Clear stale discovered tools, release slot on claim error, extend run-step TTL
Three follow-ups on the round-19 commit (Codex review 4594783691):
- I1 (P2): the round-19 discoveredTools field wasn't cleared on Redis streamId reuse.
HSET only overwrites listed fields and handleRunInterrupt only writes discoveredTools
when THIS turn discovers a deferred tool — so a replacement turn that pauses without its
own discovery inherited the prior run's tool names and force-loaded undiscovered deferred
tools on resume. Added discoveredTools to createJob's staleHitlFields HDEL list (the
in-memory store already builds a fresh object, so it was Redis-only).
- I2 (P2): with LIMIT_CONCURRENT_MESSAGES, approvals.resolve runs after the slot increment
but before the run's try/finally, so a store/Redis error there leaked the slot until the
counter TTL expired (spurious 429s on retry of the still-paused approval). Wrapped the
claim in try/catch that decrements the slot and returns 500.
- I3 (P3): saveRunSteps did SET ... EX running unconditionally, resetting the run-steps key
to the 20-min running TTL even while the job is paused for the longer approval window —
a reload after that window lost the tool timeline. Now uses a paused-window TTL script
mirroring the chunk-stream no-shrink behavior (extends to the approval window when the
job hash is requires_action).
Also fixes a latent strict-tsc cast error in the round-19 pendingAction test.
Tests: claim-throws-releases-slot (resume.spec.js); discoveredTools cleared on reuse +
saveRunSteps preserves the paused TTL (RedisJobStore integration, USE_REDIS).
* 🛡️ fix: Guard fast-resume save race, gate HITL to resumable routes, expire on stale submit
Three findings on the round-20 commit (Codex review 4595045652):
- J2 (P1): a fast /resume can claim + finalize the COMPLETED response while the original
request's pause branch is still awaiting `response.databasePromise`; the later
unfinished-save then overwrites the completed content. Re-check the job is still paused on
THIS generation's action (a claim leaves requires_action; a replacement bumps createdAt)
before marking the row unfinished; fail open on a read error.
- J3 (P1): the tool-approval wiring (humanInTheLoop + PreToolUse hook + checkpointer) was
applied to EVERY createRun caller when toolApproval.enabled, but the OpenAI-compatible and
Responses controllers never inspect run.getInterrupt() or persist a pending action — an
approval-gated tool would pause there with no approval surface or resume endpoint and the
route would emit a normal final response / [DONE] with the tool call dangling. Gate the
wiring on a new createRun `hitlCapable` flag, set only by AgentClient (chat + resume).
- J4 (P2): a stale-action 409 on submit returned without driving expiry, leaving the job
requires_action with a dead action until the periodic sweeper ran — any attached SSE client
got no terminal event and the stream appeared to hang. Extracted GenerationJobManager
.expireApproval(streamId, actionId) (expire CAS + terminal SSE, shared with the sweeper) and
call it from the resume route when the observed action is stale.
J1 (nested subagent approval controls not mounting while the details dialog is closed) is a
valid frontend issue in the deferred subagent-HITL path — tracked separately (replied on the
thread) since the fix touches the shared dialog primitive and needs UI verification.
Tests: HITL-gate both directions (run-summarization.test.ts); expire-on-stale-submit
(resume.spec.js); fast-resume unfinished-save guard predicate (jobReplacement.spec.js).
* 💄 style: Wrap captureAgents signature to satisfy prettier (CI lint)
|
||
|
Danny Avila
|
397ddc5366
|
🧠 feat: Add Memory as an Agent Capability with Inline Tools and Ephemeral Badge (#13869)
* 🧠 feat: Memory Agent Capability with Inline Tools and Ephemeral Badge
Add `AgentCapabilities.memory`, which expands into the inline set_memory/delete_memory tool pair (mirroring the execute_code expansion via registerMemoryTools) when a run-level memoryAvailable gate holds: capability enabled, memory configured, MEMORIES.USE permission, and personalization not opted out. Surfaces the memory artifact as an attachment in the agents tool-end callback.
Adds the ephemeral path (TEphemeralAgent.memory, load/added agent tool injection), a fully-gated memory badge plus tools-dropdown entry, the agent-builder Memory toggle with form round-trip, and a mock e2e test asserting the badge reaches the request payload. Additive to and independent of the existing post-turn memory extraction agent.
* 🩹 fix: Address Codex review on memory capability (gating, validKeys, usage guard)
- Strip the memory capability from the served agents capabilities when memory is not configured/enabled, so the badge, tools dropdown, agent-builder toggle, and backend capability gate stay consistent instead of exposing an inert toggle on default installs (where MEMORIES.USE defaults true).
- Surface configured memory.validKeys in the inline tool definitions so the model is told the allowed keys up front, matching the runtime createMemoryTool schema.
- Append a strict explicit-request usage guard to the agent instructions when inline memory tools are registered, preserving the memory-agent's privacy behavior.
- Add AppService tests covering memory-capability stripping.
* ✅ test: Update AppService capability snapshots for memory strip
AppService now strips the memory capability from the served agents defaults when no memory block is configured; update the spec's expected capability lists to defaultAgentCapabilitiesWithoutMemory for the no-memory-config cases.
* 🛡️ fix: Address Codex re-review on memory capability (round 2)
- Strip the memory capability from the FINAL served agents config, not just defaults; loadEndpoints reparses any endpoints.agents block, so memory was still exposed in that common shape (packages/data-schemas/src/app/service.ts) + regression test.
- Re-check the full memory gate (config, opt-out, MEMORIES.USE) inside handleTools before constructing set_memory/delete_memory, so an unsolicited tool call from a model/custom endpoint can't bypass the runtime gates (api/app/clients/tools/util/handleTools.js).
- Restore the persisted memory toggle for model-spec conversations via applyModelSpecEphemeralAgent (client/src/utils/endpoints.ts).
- Clear LAST_MEMORY_TOGGLE_ on logout and clear-all-chats so a stale memory preference can't leak across users on a shared browser (client/src/utils/localStorage.ts).
* 🧠 fix: Address Codex re-review on memory capability (round 3)
- Serialize set_memory writes and advance a running token total inside createMemoryTool, so parallel batched calls in one event-driven turn can't each pass the limit check against a stale total and collectively exceed memory.tokenLimit (packages/api/src/agents/memory.ts) + tests.
- Inject the keyed memory context (withKeys) instead of withoutKeys when the running agent has the inline memory capability, so delete_memory has a visible key to target (api/server/controllers/agents/client.js).
* 🔐 fix: Address Codex re-review on memory capability (round 4)
- Detect inline memory by tool NAME (set_memory/delete_memory) across an initialized agent's tools + toolDefinitions, since the 'memory' marker is expanded at init and the prior string check never matched; inject the keyed memory context for any primary OR sub-agent that carries the inline memory tools (api/server/controllers/agents/client.js).
- Enforce memory WRITE permissions in the inline tool gate: set_memory requires CREATE+UPDATE and delete_memory requires UPDATE (matching the REST memory routes), so a USE-only role can't mutate/delete memories via agent tool calls (api/app/clients/tools/util/handleTools.js).
* 🔒 fix: Address Codex re-review on memory capability (round 5)
- Gate inline memory registration (memoryAvailable) on the memory WRITE permissions (USE+CREATE+UPDATE), so a read-only-memory role no longer has set_memory/delete_memory shown to the model only for the runtime loader to refuse them (api/server/services/Endpoints/agents/initialize.js).
- Enforce the per-agent memory opt-in at execution: handleTools now refuses to construct set_memory/delete_memory unless the agent actually declared them (toolDefinitions/tools), blocking hallucinated/undeclared memory tool calls from mutating memory.
- Fail closed when getFormattedMemories errors with a configured tokenLimit, instead of writing as if storage were empty and bypassing the cap (api/app/clients/tools/util/handleTools.js).
* 🩹 fix: Address Codex re-review on memory capability (round 6)
- Fix a P1 regression from the prior round: the execution-context agent keeps the raw 'memory' capability marker (not the expanded set_memory/delete_memory names), so the opt-in check now matches the marker. This restores memory writes/deletes AND avoids hijacking an MCP tool that merely shares the set_memory/delete_memory name (api/app/clients/tools/util/handleTools.js).
- Count repeated set_memory writes to the same key as replacements, not additions, against tokenLimit — set_memory upserts, so a same-key rewrite swaps its prior token contribution instead of double-counting (packages/api/src/agents/memory.ts) + test.
- Gate the memory badge, tools dropdown, and agent-builder toggle on the full memory write permissions (USE+CREATE+UPDATE) via a shared useHasMemoryAccess hook, so a read-only-memory role no longer sees an enabled Memory control the backend would refuse to wire up.
* 🧷 fix: Address Codex re-review on memory capability (round 7)
- Recognize inline memory across both execution-context agent shapes: initializeAgent now sets a LibreChat-only memoryToolsRegistered flag on the InitializedAgent, and the opt-in/detection checks accept that flag OR the raw 'memory' marker. Fixes memory failing for processAddedConvo agents (which store the initialized config, marker already expanded) while staying MCP-name-collision-safe (api/app/clients/tools/util/handleTools.js, packages/api/src/agents/initialize.ts, api/server/controllers/agents/client.js).
- Scope keyed memory context to memory-enabled agents only: useMemory now returns both keyed and unkeyed contexts, and buildMessages injects the keyed one (memory keys + token metadata) only to agents that can call delete_memory, while the primary/post-turn path keeps the unkeyed values — so a primary without memory tools no longer sees memory keys it doesn't need.
* 🔏 fix: Address Codex re-review on memory capability (round 8)
- Enforce memory size limits on inline writes: createMemoryTool now rejects keys over 1000 chars and values over memory.charLimit, matching the REST memory routes, so an inline-memory agent can't persist blobs the memory UI/API would reject (packages/api/src/agents/memory.ts, api/app/clients/tools/util/handleTools.js) + test.
- Recheck the agents 'memory' endpoint capability at execution time, so a stale/hallucinated set_memory/delete_memory call can't mutate memory after an admin removes the capability while the agent document still carries the marker (api/app/clients/tools/util/handleTools.js).
* ♻️ refactor: Move inline-memory backend logic into packages/api + share memory load
Workspace boundary: the inline-memory gating/detection logic that had crept into /api now lives in packages/api/src/agents/memory.ts (TS), with /api kept as thin wrappers.
- Add agentHasInlineMemoryTools, isMemoryToolAllowed, and buildInlineMemoryTool to packages/api; handleTools.js now calls buildInlineMemoryTool instead of constructing/gating the tools inline, and client.js imports agentHasInlineMemoryTools instead of redefining it.
- Optimize repeated memory loads: getRequestMemories memoizes getFormattedMemories per request (WeakMap keyed by req), so the run's memory-context load and every memory-enabled agent's set_memory token-usage load share a single DB fetch instead of one per agent.
* 🧠 fix: Invalidate request memory cache after inline writes
Inline set_memory/delete_memory now invalidate the request-scoped
getFormattedMemories cache on a successful write, so a later tool round
in the same response is seeded with the post-write usage total instead
of the stale pre-write one (multi-round writes no longer collectively
exceed tokenLimit, and a set after a delete is not over-counted). The
within-round sharing across multiple memory-enabled agents is preserved.
* 🧠 fix: Persist memory capability on saved agents; honor registration flag
- Add Tools.memory to the v1 systemTools allowlist so filterAuthorizedTools
no longer silently drops the memory marker when an agent with the Memory
capability is created/updated/duplicated through the builder (previously
the capability only worked for ephemeral chats, not persisted agents).
- agentHasInlineMemoryTools now honors an explicit memoryToolsRegistered
boolean before falling back to the raw `memory` marker, so an initialized
config whose registration was denied (memoryAvailable false) is not given
keyed memory context just because the marker survives in tools.
* 🧩 fix: Bring memory tool to parity with other ephemeral tools
- Add `memory` to the model-spec schema/type and honor `modelSpec.memory`
in both ephemeral paths (load.ts, added.ts) and the frontend spec
application, so admins can pre-enable Memory from a model spec exactly
like webSearch/fileSearch/executeCode.
- Add LAST_MEMORY_TOGGLE_ to the timestamped-storage cleanup list so stale
per-conversation memory toggles are purged on startup like the others.
- Hide the agent-builder Memory toggle for users who disabled memory in
personalization (memories === false), mirroring the chat badge's opt-out
gate, so the setting isn't shown as inert/misleading.
* ✅ test: Cover memory in applyModelSpecEphemeralAgent spec defaults
Update the exact-object assertions to include the new `memory` field and
add positive coverage that `modelSpec.memory` maps to the ephemeral
agent's `memory` flag. Fixes the shard 2/4 failure from
|
||
|
Danny Avila
|
1662adc581
|
📺 feat: Google URL Context Param with Native YouTube Video Understanding (#13924)
* ✨ feat: Add Google url_context Param with Native YouTube Video Understanding Mirror the web_search grounding wiring for a new Google/Gemini `url_context` model param (resolves to the native `urlContext` tool). When enabled, YouTube URLs in the latest user message are injected as Gemini video parts (fileData), since the URL Context tool does not support YouTube. * 🎞️ fix: Provider-aware YouTube injection limits for url_context Address Codex review on the YouTube video-understanding path: - Cap injected YouTube parts per request by provider/model (Vertex: 1; Gemini Developer API: 10 on 2.5+, 1 on earlier models) so multi-link messages cannot exceed the provider limit and get rejected. - Set a video/mp4 mimeType on Vertex YouTube fileData (matching Vertex samples); the Developer API still omits it. * 🧩 fix: Round-trip url_context for Google-compatible custom endpoints Add url_context to openAIBaseSchema so the per-chat value persists for custom endpoints configured with customParams.defaultParamsEndpoint: 'google', matching how web_search is already picked there. * 🚦 fix: Gate url_context tool to Gemini 2.5+ models Per Google's URL Context supported-models list (2.5+/3.x only), skip the native urlContext tool on earlier models (debug-log + no-op) instead of sending it and triggering a provider 400. This also gates the coupled YouTube video-understanding injection to 2.5+, since it keys off the resolved urlContext tool. * ✂️ fix: Strip YouTube URLs from urlContext text; keep url_context out of OpenAI schema - Remove url_context from the shared openAIBaseSchema (revert): it is Google-only and would otherwise leak as an unsupported param to OpenAI/Azure/OpenRouter requests. On Google-compatible custom endpoints url_context is enabled via admin addParams/defaultParams, same as web_search. - When injecting YouTube video parts, strip the matched YouTube URLs from the prompt text so the urlContext tool (which reads URLs from text and cannot fetch YouTube) does not consume its URL budget on them. Non-YouTube URLs are left intact. * 🎯 fix: Refine url_context model gating and YouTube injection edges Address Codex round 4: - Exclude non-text modality variants (image/live/tts) from URL Context support, mirroring the Google tool-combination modality exclusion. - Use the resolved run model (model_parameters.model) for YouTube injection limits instead of the saved base model. - Strip only the YouTube links actually routed to video (id-aware); keep over-limit links in the text so the model can still reason about them. - Keep timestamped YouTube links (?t=/&start=) in the text so the moment cue survives. - Recognize youtube-nocookie.com/embed links. * 🎚️ fix: Exclude audio Gemini variants + preserve pre-id YouTube timestamps Address Codex round 5: - Add `audio` to the url_context modality exclusion so audio-only Gemini variants (e.g. gemini-2.5-flash-preview-native-audio-dialog) skip the tool instead of 400ing. - Detect YouTube timestamps anywhere in the matched URL (incl. before `v=`, e.g. watch?t=90&v=<id>), so timestamped links are kept in the prompt text as intended. |
||
|
Danny Avila
|
d9a76fca90
|
🧠 feat: Configurable Reasoning Replay for Custom Endpoints (#13921)
* 🧠 feat: Configurable Reasoning Replay for Custom Endpoints Adds customParams.includeReasoningContent so OpenAI-compatible custom endpoints (e.g. Xiaomi MiMo, Kimi) can replay reasoning_content on tool-call turns natively, without impersonating the moonshot provider. * 🔁 feat: Replay reasoning_content across turns for opted-in custom endpoints Extends the DeepSeek reasoning-content format spoof to honor customParams.includeReasoningContent, so custom OpenAI-compatible endpoints (Xiaomi MiMo, Kimi) reconstruct reasoning_content from persisted history on later turns, matching DeepSeek thinking-mode parity. Adds shouldReplayReasoningContent predicate (tested) and surfaces the flag on the initialized agent. * 🪢 refactor: Split within-run vs cross-turn reasoning replay flags moonshot only replays reasoning_content within a run's tool calls, not across turns. Decouples the two: includeReasoningContent = within-run replay (exact moonshot parity), new includeReasoningHistory = cross-turn reconstruction from persisted history (implies includeReasoningContent, since reconstruction is a no-op without the within-run replay flag). * 🩹 fix: Apply reasoning replay across all param-format branches Move the within-run includeReasoningContent application out of the OpenAI-only branch in getOpenAIConfig to after the branch dispatch, so custom endpoints using anthropic/google defaultParamsEndpoint gateway modes also honor includeReasoningContent/includeReasoningHistory. Addresses Codex finding. * chore: Update @librechat/agents to v3.2.46 * 🧽 refactor: De-spoof reasoning replay via explicit preserveReasoningContent Now that @librechat/agents 3.2.46 exposes an explicit preserveReasoningContent option on formatAgentMessages, pass it directly instead of impersonating provider: deepseek. Behavior is unchanged (shouldReplayReasoningContent still gates DeepSeek + the custom includeReasoningHistory flag); also corrects the comment to reference includeReasoningHistory. * 🌳 fix: Walk subagents in the reasoning-history replay gate The gate only checked the primary agent and top-level handoff/parallel configs, so an opted-in custom endpoint used solely as a nested subagent had its persisted reasoning dropped on later turns. New exported anyAgentReplaysReasoningContent walks subagentAgentConfigs (cycle-safe, mirrors anyAgentHasCodeEnv); client.js uses it. Addresses Codex finding. |
||
|
Danny Avila
|
bc6b032421
|
🛑 refactor: Demote User Abort Logs (#13904)
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: Demote user abort logging * fix: Handle abort causes * fix: Demote user-aborted agent completion to debug log The error users still saw originated in AgentClient's completion catch, which logged every caught error (including user aborts) at error level before checking the abort signal. Branch on abortController.signal.aborted so user-initiated aborts log at debug while real failures stay error-classified. Also give the handleAbortError it.each cases distinct titles. |
||
|
Danny Avila
|
5eb1c2c107
|
🖇️ feat: Reference Selected Chat Text with Multi-Quote Popup (#13868)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🖇️ feat: Reference Selected Chat Text with Multi-Quote Popup Add a ChatGPT/Codex-style quote feature: selecting text in any message shows an 'Add to chat' popup that accumulates removable quote chips above the composer. On submit, the excerpts are merged into the user message text as Markdown blockquotes (counted in the user message token count, not a system message) and persisted on the message so they render on the user bubble and survive reload. - packages/api: add getReferencedQuotes + mergeQuotedText helpers (blockquote merge, length/count caps) with unit tests - BaseClient.sendMessage: temporarily merge req.body.quotes into userMessage.text before buildMessages, restore clean text, persist quotes array - data-schemas + data-provider: add optional quotes field to message schema/type - client: pendingQuotesByConvoId atom, QuoteButton selection popup, PendingQuoteChips composer row, MessageQuotes persistent display - useChatFunctions: drain pending quotes onto the message, carry forward on regenerate - add localization keys and component/integration tests * 🧪 test: Add Playwright e2e for chat quote feature Add e2e/specs/mock/quotes.spec.ts covering select -> 'Add to chat' popup -> chip -> send -> persistent reference block -> reload, plus multi-select accumulation and chip removal. Selection is driven programmatically (real DOM Range + dispatched mouseup) to summon the popup deterministically. Add data-testid hooks (add-to-chat-button, pending-quote-chips, message-quotes) to the quote components for stable selectors. * 🛡️ fix: Address Codex review on quote feature - Run PII filter + OpenAI moderation over req.body.quotes (P1): quoted excerpts are merged into the model-facing user message, so they must clear the same filters; a crafted quotes payload could otherwise bypass them. Adds tests. - Carry quotes through edit/save-and-submit replays (overrideQuotes in EditMessage), mirroring overrideManualSkills, so edited turns keep context. - Hide the quote UI for Assistants endpoints (which bypass BaseClient merge), so users can't queue quotes the assistant never receives. - Clear pending quote/skill queues by resolved conversationId in useClearStates, not the UI index, so queued-but-unsent selections don't linger in Recoil. - Cap queued quotes client-side at 10 to match the backend QUOTE_MAX_COUNT, so the composer never shows more quotes than are actually sent. * 🧵 fix: Durably re-merge quotes + Codex round 2 Address Codex's re-review of the quote feature: - Durable history re-merge (per maintainer decision): quotes are no longer merged at request time and stripped; instead each user message's persisted message.quotes is merged into its formatted content in AgentClient.buildMessages (new prependQuotes helper) for current AND historical turns. The model receives the referenced context on every prompt and the token count stays consistent with what was persisted; stored text stays clean for display. - Attach normalized quotes to the user message in handleStartMethods (before getReqData/onStart) so the optimistic bubble, resumable abort metadata, and saved row all carry them (fixes the abort-metadata gap). - Skip the quote drain entirely for Assistants endpoints in useChatFunctions, leaving the pending atom intact (UI is already hidden there). - Normalize req.body.quotes via getReferencedQuotes before moderation/PII so only the trimmed/truncated/capped excerpts the model will receive are checked. - Tests: prependQuotes unit tests; BaseClient quote tests assert early attachment + clean text; e2e now verifies the model receives the merged blockquote on the current turn and re-merged from history on a later turn (new E2E_ASSERT_QUOTE mock marker). * 🔗 fix: Quote share/memo/abort/PII gaps (Codex round 3) - Shared links: include quotes in the anonymized projection + SharedMessage type (+test) so the /share view renders the same reference blocks as the owner, mirroring manualSkills/alwaysAppliedSkills. - MessageRender memo: compare quotes length so a server/resume copy whose only change is the quote list re-renders (the block no longer goes stale/missing). - Resumable job metadata: include quotes in the userMessage written to GenerationJobManager so a reload/reconnect mid-stream reconstructs the chips. - PII + moderation: also scan the merged blockquote+text exactly as the model receives it, so a secret split across a quote and the typed body (each clean alone) is caught (+cross-boundary test). - e2e: make quote-add robust against the auto-scroll-dismisses-selection race via a retried select+click helper. * 🛑 fix: Keep quotes on aborted turn's request message (Codex round 4) abortMiddleware reconstructs finalEvent.requestMessage from jobData.userMessage but only copied ids + text; include quotes so a stopped quoted turn keeps its MessageQuotes in the UI and a regenerate-before-reload still sends the referenced context. Completes the resumable-metadata fix from the prior round. * 🧮 fix: Quote recount + preliminary abort metadata (Codex round 5) - Force a canonical token recount for messages carrying quotes in AgentClient.buildMessages, so a plain text-only Save edit (which recomputes tokenCount from text alone) can't leave a stale, quote-excluding count that undercounts context on later turns — recount from the quote-merged copy self-heals it. - Seed normalized quotes into the preliminary userMessage metadata (getPreliminaryUserMessage), so an abort during init/tool-loading (before onStart) still reconstructs the stopped turn's MessageQuotes. * ✅ fix: Add getReferencedQuotes to controller test mocks (CI) request.js's getPreliminaryUserMessage now calls getReferencedQuotes; the agents controller specs mock @librechat/api wholesale, so the mock must export it or the call throws and cascades. Added a faithful mock (normalize/cap, null when empty) to request.resumeMetadata.spec.js and jobReplacement.spec.js. * 📐 fix: Quotes in context projection + resumable metadata (Codex round 6) - Context-usage projection (resolveContextProjection): select message.quotes, prepend them into the projected user text, and recount quoted messages so the context gauge counts the same prompt the model receives (a text-only Save edit no longer makes the gauge undercount / over-report remaining budget). - Resumable job metadata: trackUserMessage (created-event rewrite) and abortJob (final requestMessage) now carry quotes; SerializableJobData.userMessage and CreatedEvent.message gained an optional quotes field. With the cross-replica created-event spread, stopping/reconnecting a quoted turn after the created event keeps its MessageQuotes. * 💬 feat: Collapse multi-select quotes into one chip with hover popup Composer feedback: the quote chip area now shows a single chip — the excerpt text for one selection, or a collapsed "{n} selections" pill for multiple, with a hover popup (HoverCard) listing every excerpt and a per-item remove. The chip is taller (py-1.5/text-sm) to read less skinny. Adds com_ui_quote_selections and com_ui_remove_all_quotes; updates unit + e2e tests (e2e drives the count via a data-quote-count hook and exercises the hover popup). * ♿ fix: Make multi-selection quote popup keyboard accessible The collapsed "{n} selections" pill used a HoverCard, which Radix only opens on pointer hover — its interactive content was unreachable by keyboard. Replaced it with a Popover: the trigger is a real button that opens on click / Enter / Space (focus moves into the list, each excerpt's × is tab-navigable, Escape closes and restores focus), with hover-open preserved for mouse via controlled open state + a close grace period. Hover-initiated opens skip auto-focus so they don't pull focus off the composer. Adds an e2e asserting keyboard open/close. * 📐 fix: Clamp the Add-to-chat button within the viewport (Codex round 7) The floating selection button positioned via translate(-50%,-100%) (bottom-center anchor) but clamped top/left as if they were its top-left, so a selection near the viewport top or sides could render the button partly/fully offscreen. Now it measures the button (ref + useLayoutEffect) and computes an on-screen top-left — clamping by the full width within side margins and flipping below the selection when there's no room above — with no transform, and stays hidden until measured so it never flashes at an unclamped spot. * ↩️ fix: Restore pending quotes on early-abort draft (Codex round 8) When a turn is stopped before the created event (e.g. during tool/MCP init), the final handler restores requestMessage.text to the draft, but the pending-quote atom was already drained on submit — so a retry sent no quotes. The abort requestMessage now carries quotes (preliminary metadata + abort fixes), so the three early-abort/no-response draft-restore paths in useEventHandlers now also re-queue pendingQuotesByConvoId from requestMessage.quotes. * ♿ fix: Use Ariakit Popover for quote selections (keyboard focus) The multi-selection popup used a hand-rolled Radix Popover with Popover.Anchor + a manual button, so Radix had no trigger to return focus to — Escape dumped focus to the page top. Refactored to Ariakit (the codebase's popover primitive, per DropdownPopup/Fork): the `PopoverDisclosure` is the real trigger, so Escape closes and returns focus to the composer instead of the top of the page. Keyboard opens (Enter/Space) autofocus into the list and tab through each excerpt's remove; hover opens for mouse with autofocus suppressed so it never pulls focus off the composer. e2e asserts the keyboard open/navigate/Escape flow keeps focus on a real control (never BODY). |
||
Ravi Kumar L
|
27b0782201
|
📛 feat: Tag Langfuse Traces With Tenant ID (#13808)
* feat: tag Langfuse traces with tenant id * fix: propagate tenant id to agent Langfuse config |
||
|
Danny Avila
|
44c253d48a
|
🪙 fix: Correct Context Usage Gauge After Summarization (#13744)
* 🪙 fix: Persist Context Snapshot + Summary Marker After Summarization The post-summarization context is correctly compacted by the SDK, but the breakdown wasn't reliably reaching the client, leaving the gauge on the whole-history estimate (stuck at 100% forever once a conversation compacts). Two server changes in buildResponseMetadata: - Snapshot guard: persist the breakdown when a PRIMARY usage event follows the latest snapshot (tracked via contextUsageSink.latestUsageIndex, recorded in the on_context_usage handler) instead of a brittle snapshot-vs-primary count. A summarization detour adds an extra snapshot whose only following usage is tagged 'summarization', which the count guard could miscount and drop. - Summary marker: whenever a turn compacts (summaryTokens > 0), persist a lightweight metadata.summaryUsedTokens (the pre-invoke compacted context size) UNCONDITIONALLY — so even when the full snapshot can't be saved (interrupted final call) or never reaches the client, the per-message estimate has a signal to cap the discarded history. Tests: client.contextMetadata.spec (guard + marker, incl. marker-survives-drop) and a real-pipeline summarization integration test. * 🪙 fix: Cap the Context Estimate at the Summary Marker When the gauge falls back to the per-message estimate (no usable snapshot on the branch), sumBranch summed the ENTIRE branch history — after a summarization that discarded most of it, this over-counts and pins the gauge at 100% in perpetuity. sumBranch now stops at the deepest summarized response (metadata.summaryUsedTokens) and records it as summaryBaseline; the walk counts only post-summary messages, and useTokenUsage adds the baseline. So the estimate reflects the compacted context (summary + recent turns), not the discarded history. USD/default behavior unchanged when no marker is present. Test: sumBranch caps a huge pre-summary history at the compacted baseline. * 🪙 fix: Address Codex Review on the Summarization Marker - Branch cost/usage is no longer truncated at the summary marker — sumBranch caps only the CONTEXT-window count there and keeps accumulating provider usage/cost to the root (cumulative spend isn't discarded by compaction). - findBranchSnapshotAnchor stops at a summarized response with no snapshot of its own, so it can't recover a stale PRE-summary snapshot and show discarded history; the summary-baseline estimate is used instead. - Abort path: buildAbortedResponseMetadata now persists the summaryUsedTokens marker (pre-invoke, no completedOutputTokens ambiguity, so safe on abort) so a STOPPED summarized turn isn't re-summed on reload. - Marker baseline fallback now includes summaryTokens (a separate breakdown field) so it doesn't under-report the compacted size. DRY'd into a shared computeSummaryUsedTokens used by the completion and abort paths. - Estimate popover surfaces the summary baseline as a row so the displayed rows reconcile with the header total. Tests: sumBranch cost-not-truncated + anchor-stops-at-marker (client); computeSummaryUsedTokens fallback + abort marker (packages/api). * 🪙 fix: Attribute Persisted Context Usage to the Snapshot Run Match the post-snapshot primary usage to the latest snapshot's runId before persisting metadata.contextUsage. Parallel/direct runs interleave snapshots and usage (A snapshot → B snapshot → A usage → B no-usage); the prior index-only guard persisted B's snapshot with A's output. finalCallOutputTokens now filters completedOutputTokens to the snapshot's run. Untagged events (older lib/resume) match any run for back-compat. * 🪙 fix: Harden Summary Marker Against Tool-Loops, Stale Anchors, and Emit Races Codex round on the summarization marker: - Avoid double-counting earlier tool-loop outputs in the summary marker: those outputs sit in BOTH the latest snapshot's pre-invoke baseline AND the response message's tokenCount the client estimate adds on top. computeSummaryUsedTokens now subtracts the run's prior primary outputs (priorRunOutputTokens) — the live path bounds them by the snapshot's usage index, the abort path by all primaries (an interrupted final call emits none). Single-call turns subtract 0. - Stop treating pre-summary anchors as active: sumBranch no longer sets containsAnchor once the context is capped at a summary marker, so a stale pre-summary snapshot can't override the summary-baseline estimate. - Capture latestUsageIndex BEFORE awaiting emitEvent: a yield (resumable SSE / Redis) during parallel runs could let this call's own usage advance the index past the event that proves the snapshot completed, dropping a valid breakdown. * 🪙 fix: Subtract Summarization Output from the Summary Marker recordCollectedUsage folds the summarization call's completion into the response message's tokenCount, while the generated summary is also in the snapshot baseline as summaryTokens. The client estimate (summaryBaseline + responseTokenCount) thus counted the summary twice — inflating the gauge after compaction even on a single-call turn whenever the full snapshot is unavailable. priorRunOutputTokens now also counts summarization-tagged output (still excluding subagent/sequential, which recordCollectedUsage keeps out of the reported total), so the marker subtracts it. Updated unit + guard tests. * 🪙 fix: Refine Marker Subtraction for Summarization RunId and Abort Boundary Two Codex follow-ups on the marker-subtraction logic: - Subtract summarization output regardless of runId: the summarize detour is its own model-end call that may carry a distinct runId, but its output still lands in this response's tokenCount AND the snapshot baseline (summaryTokens). It is now counted unconditionally (still within the response's own usageEmitSink), while primaries keep the parallel-run runId filter. - Don't subtract primaries on the abort path: the job stores no snapshot/usage boundary, so a primary that completed AFTER the latest snapshot is NOT in the baseline; subtracting it would cancel real output and under-report. priorRun- OutputTokens gains an includePrimary flag (false for abort) — abort subtracts only the always-pre-snapshot summarization output. * 🪙 fix: Run-Scope Summary Subtraction and Stop Subtracting on Abort Two Codex follow-ups, resolved by reverting the round-4 detour: - Run-scope the summarization subtraction: the summarize detour inherits the graph run id (traceConfig spreads config.metadata.run_id), so its usage shares the answer snapshot's runId — it is NOT a distinct run. priorRunOutputTokens now filters summarization by runId like primaries, so a parallel sibling run's summary (different runId, in the sibling's baseline) is no longer subtracted from this branch's marker. Drops the includePrimary flag added last round. - Stop subtracting on the abort path: abort tokenCount is countTokens(text) (abortMiddleware) or absent (agents route) — it does not fold in summarization or earlier-call output the way recordCollectedUsage does, so the marker must keep the full baseline. buildAbortedResponseMetadata now subtracts nothing. |
||
|
Danny Avila
|
2350ebb24a
|
📨 feat: Custom Headers on Built-in Provider Endpoints (#13742)
* 📨 feat: Custom Headers on Built-in Provider Endpoints Add a `headers` config option to the built-in `openAI`, `anthropic`, and `google` endpoints (incl. Anthropic/Google Vertex), mirroring the custom endpoint header mechanism. Values support the same placeholder resolution (env vars, `{{LIBRECHAT_USER_*}}`, `{{LIBRECHAT_BODY_CONVERSATIONID}}`) and are resolved at request time so dynamic values like conversationId resolve against the live request — without losing provider-native request shaping. Closes #13082. Covers #13713: forwarding conversationId to a reverse proxy is now `X-Conversation-Id: '{{LIBRECHAT_BODY_CONVERSATIONID}}'` — an unknown header is ignored by the native Anthropic API, so no 400 and no metadata gating needed. - Schema: `headers` on `baseEndpointSchema` (openAI/google/anthropic/all). - New `mergeHeaders`/`resolveConfigHeaders` utils centralize the per-provider header locations (`configuration.defaultHeaders`, Anthropic `clientOptions.defaultHeaders`, Google `customHeaders`); provider-managed headers (auth, `anthropic-beta`) always win on collision. - Each initializer threads configured headers (endpoint over `all`) into the right place; request-time resolution runs across all locations in the main and title flows. * 🩹 fix: Cast endpoints.all to TEndpoint for headers DeepPartial widening Adding `headers` (a Record) to `baseEndpointSchema` makes `DeepPartial<TCustomConfig>` widen its value type to `string | undefined`, which is not assignable to the concrete `TEndpoint['headers']: Record<string, string>` at the `loadedEndpoints.all` assignment. Cast at the assignment site, mirroring the existing `anthropicConfig as TAnthropicEndpoint` cast in the same function. * 🛡️ fix: Harden built-in endpoint custom headers (Codex review) Address Codex P2 findings on the custom-headers feature: - Anthropic title requests: `omitTitleOptions` strips the `clientOptions` carrier, which dropped its `defaultHeaders`. Preserve just the header carrier so gateway/reverse-proxy metadata still reaches title generation. - mergeHeaders: match header names case-insensitively so an override (e.g. a provider-managed `Authorization`/`anthropic-beta`) replaces/uniones a case-variant from the base instead of emitting two names a client may collapse. - OpenAI: withhold admin-configured headers when the user supplies the base URL (`user_provided`), since values may carry `${SECRET}`/token placeholders that must not reach a user-controlled endpoint — mirrors the custom-endpoint guard. - Azure: honor global `endpoints.all` headers (same OpenAI carrier) while keeping Azure-managed `api-key`/version headers authoritative. Adds tests for each. * 🔐 fix: Resolve-once + provider-managed header safety (Codex review round 2) Address Codex P2 findings: - Azure: keep global `endpoints.all` headers unresolved at init and let request-time `resolveConfigHeaders` resolve them once, avoiding a second-order env expansion of already-substituted user values. - Google: `resolveConfigHeaders` no longer template-resolves the provider-managed `Authorization` header (built from a possibly user-provided key), so a user key like `${ENV}` can't leak server environment values. - Model fetches: thread configured headers (endpoint over `all`) + user object through `getOpenAIModels`/`getAnthropicModels` → `fetchModels`, so a gateway-fronted built-in provider receives the header on `/models` too. Fixed `fetchModels` to merge custom headers for Anthropic instead of overwriting them (managed `x-api-key`/version still win). Adds/updates tests for each. * 🧯 fix: Header provenance, memory/title coverage, idempotency (Codex round 3) Address Codex P2 findings, including two regressions from the prior round: - Google auth (findings 6 & 8): move native Google header resolution to init (`initializeGoogle`), resolving admin templates BEFORE the key-derived auth header is built. resolveConfigHeaders no longer touches Google `customHeaders`, so admin `Authorization` templates resolve again (fixes the round-2 regression) while the SDK auth header (possibly a user-provided key) is never env-expanded. - Memory runs: memory extraction now calls `resolveConfigHeaders`, so native Anthropic (and OpenAI) headers resolve for memory requests too. - Vertex titles: restore the ORIGINAL `clientOptions` object reference (not a copy) when preserving headers across `omitTitleOptions`, so the Vertex `createClient` closure and the resolved headers stay on the same object. - Reuse: `resolveConfigHeaders` is now idempotent (resolve-once per header map), preventing a second pass from env-expanding values already substituted with user/body data when an agent object flows through buildAgentInput twice. Adds/updates tests for each. |
||
|
Danny Avila
|
4ee68d5240
|
💸 feat: Per-Agent Endpoint Token Config in Multi-Endpoint Billing (#13738)
* 💸 feat: Per-Agent Endpoint Token Config in Multi-Endpoint Billing
Price each collected/emitted usage item with the producing agent's resolved
endpoint token config, instead of the primary agent's for the whole graph.
Previously AgentClient.recordCollectedUsage and the subagent usage emitter used
a single this.options.endpointTokenConfig (the primary's) for every usage item.
A connected agent or subagent on a different custom endpoint that shares a model
id with an entry in the primary's tokenConfig was therefore mis-priced (a model
absent from it already fell back to the built-in rate map — no regression).
- Tag each usage with its producing agent: ModelEndHandler stamps
usage.agentId = agentContext.agentId; createSubagentUsageSink stamps the
child's subagentAgentId (UsageMetadata gains an optional agentId).
- buildAgentToolContext retains endpointTokenConfig so initialize.js can build
an agentId -> endpointTokenConfig map from agentToolContexts (the one map that
holds every agent, including pure subagents pruned from agentConfigs).
- AgentClient.resolveAgentEndpointTokenConfig(usage) looks up that map by
agentId, falling back to the primary config; used by both the billing path
(new optional resolveEndpointTokenConfig on recordCollectedUsage) and the
subagent cost emitter.
- recordCollectedUsage's resolver is optional and falls back to the batch
endpointTokenConfig, so the shared responses.js/openai.js call sites are
unchanged.
- Tests: two-endpoint graph with a colliding model id prices per-agent; resolver
nullish falls back to batch; subagent sink tags the child agent id.
* fix: Align emit-path cost with per-agent billing; honor known-agent built-in pricing
Addresses Codex review on the per-agent endpoint token config:
- Emit path (callbacks.js) now prices each on_token_usage event with the
producing agent's config (resolved via usageCost.resolveEndpointTokenConfig),
so streamed/persisted metadata.usage.cost matches the per-agent balance
transaction. The agentId tag is resolved server-side and stripped from the
emitted/persisted payload.
- Resolver (resolveAgentTokenConfig) now treats a known agent's config as
authoritative, including undefined → built-in pricing, so a known non-custom
agent in a custom-primary graph is no longer charged the primary's rates.
Only untagged/unknown usage falls back to the primary config.
- endpointTokenConfigByAgentId records every known agent (value may be
undefined) so the resolver distinguishes known-no-rates from unknown.
|
||
|
Danny Avila
|
b03b2a0a29
|
💾 feat: Persist Context Breakdown & Branch/Total Usage Cost (#13734)
* 💾 feat: Persist Context Breakdown & Branch/Total Usage Cost Persist the granular context breakdown and per-response usage/cost on the response message metadata, and re-derive branch + total usage/cost from a per-message index so the popover survives reloads and is branch-aware live. - Add aggregateEmittedUsage + buildPersistedContextUsage helpers in packages/api; capture the latest visible snapshot and every emitted on_token_usage payload via contextUsageSink/usageEmitSink. - Attach metadata.contextUsage (Part A) and metadata.usage (Part B) on the agents response message in sendCompletion. - Carry per-message usage on the token index; add sumTotalUsage/setEntryUsage and branch-scoped usage on sumBranch. - Repurpose the session accumulator into a single in-flight pending holder; flush it into the index at finalize; hydrate breakdowns on load. - Render branch cost with a conditional all-branches total in the breakdown. * 🧹 chore: Remove orphaned com_ui_session_cost i18n key * 🩹 fix: Address Codex review — normalize usage server-side, fix reload deltas - Persist per-event-normalized display units in metadata.usage (TResponseUsage) so reloaded mixed-provider turns match the live session; client reads them directly instead of re-normalizing with a single stamped provider (P2). - Persist completedOutputTokens (final call output) on metadata.contextUsage so a reloaded multi-call turn adds the post-snapshot delta, not the full tokenCount the snapshot already counts (P2). - buildIndex preserves a prior entry's immutable usage when a rebuilt cache message lacks metadata.usage, so a mid-session rebuild (regenerate) keeps a sibling branch's flushed cost (fixes the e2e regenerate failure). - Track costKnown so turns saved with contextCost off don't render $0.00 when cost display is later enabled (P3). - Use an epsilon for the all-branches cost comparison to avoid a spurious total row from float summation order (P3). - Update unit/integration/e2e tests for the new shapes; regenerate e2e asserts the all-branches total after reload (deterministic via persisted metadata). * 🩹 fix: Address Codex round 2 — pending leak, cost coverage, reload delta - Clear the in-flight pending usage on terminal abort/error (resetLive), so a stopped generation's tokens no longer merge into the next response (P2). - costKnown now means COMPLETE coverage (ANDed): a branch mixing cost-bearing and cost-less turns is flagged incomplete and the cost row is hidden rather than rendering an under-reported total (P2). - Drop the tokenCount fallback for completedOutputTokens on reload: only the persisted post-snapshot delta is used, so a multi-call turn whose provider emitted no usage_metadata no longer double-counts earlier output (P2). - Update tokens.spec for AND coverage semantics + incomplete-cost case. * 🩹 fix: Address Codex round 3 — no-usage snapshots, total coverage, provider-less cache - Skip persisting metadata.contextUsage when the response emitted no primary usage event: without a known post-snapshot output the granular gauge would undercount the reply on reload, so fall back to the coarse per-message estimate instead (P2). - Gate the all-branches cost row on totalUsage.costKnown so an incomplete total (a sibling saved without cost) never renders an under-reported figure (P2). - aggregateEmittedUsage/finalCallOutputTokens now normalize per-event with the client's magnitude fallback (normalizeEventUnits) instead of billing splitUsage, so provider-less cached events match live on reload (P2). - Add backend test for the provider-less cached case. * 🩹 fix: Address Codex round 4 — abort attribution, complete cost coverage - aggregateEmittedUsage persists cost only when EVERY call was priced; a partial pricing failure now omits cost so the client treats coverage as unknown rather than reading an under-reported sum as authoritative (P2). - finalizeUsage flushes pending into the response entry only when events were folded this session (eventCount > 0), so a late/second resumable subscriber carrying persisted metadata.usage keeps it instead of being overwritten with an empty pending record (P2). - On user stop, attribute the in-flight pending usage to the partial response (new attributePending handler) instead of discarding it in resetLive — the stopped reply's billed tokens are kept and still can't leak into the next response; resetLive's discard remains for the error path (P2). * 🐛 fix: Persist branch cost across branch switches via sticky usage history Branch cost vanished on switching to a sibling branch (until a new turn) — the cost analog of the granularity bug. buildIndex rebuilds the token index from the messages cache; a sibling generated this session whose cache message lacks metadata.usage (and is transiently dropped from the cache during regenerate) lost its live-flushed usage, so sumBranch found none and the cost row hid. Fix: a sticky per-response usage map (conversationId → messageId → usage), written by setEntryUsage and never rebuilt from the cache — the usage counterpart of snapshotsByAnchorFamily for the breakdown. buildIndex/upsertEntries restore an entry's usage from it when the message carries none; cleared on convo switch and migrated with the index. Add unit coverage for the drop-then-readd regression and an e2e assertion that branch cost survives a branch switch. * 🐛 fix: Re-index on branch switch so branch cost survives the switch The sticky usage history alone didn't fix the reported branch-switch cost drop: on a branch switch no cache `updated` event fires, so the index subscriber never re-ran, and the post-regenerate rebuild was skipped while `isSubmitting` was still true — leaving the index stale and missing the now-viewed branch's response entirely (sticky can only restore entries present in a rebuild). Re-index from the messages cache on every tail change (created/finalize AND branch switch), not just while submitting. The cache holds the full message set at switch time, so the viewed branch's response is re-added and its usage restored from metadata.usage or the sticky history → sumBranch finds it and the branch cost renders. Verified locally: the branch-switch e2e now passes (the cost section shows both the branch row and the all-branches total). Also fixed that e2e assertion to target a single cost value (strict-mode safe). * 🩹 fix: Handle stopped-stream usage — reset pending + persist abort metadata Codex round (stop/abort edges): - Resumable explicit-stop (intentional SSE close) reset UI state but never cleared pendingUsageFamily, so usage folded before the stop leaked into the next response in the conversation. Discard pending on intentional close (resetLive); a resume re-folds via backfillUsage, so nothing is lost. - The abort save path (abortMiddleware) persisted the stopped response without metadata.usage/contextUsage, so its cost + breakdown vanished on reload. Rebuild both from the job's persisted tokenUsage (emitted payloads incl. cost) and contextUsage snapshot — parity with the normal sendCompletion path; breakdown gated on a primary usage event like buildResponseMetadata. Deferred (per scope decision): mid-stream branch-switch transiently shows the streaming branch's pending on the viewed sibling (cosmetic, until finalize). * 🩹 fix: Persist abort metadata on the real agents route + tighten snapshot gate Codex round (corrects last round's wrong-path fixes): - Stopped AGENTS responses are saved by routes/agents/index.js (/chat/abort), not abortMiddleware — so last round's metadata fix never ran for them. Moved the rollup/snapshot builder into packages/api as buildAbortedResponseMetadata (shared, unit-tested) and applied it in BOTH abort save paths, so a stopped agent reply keeps its cost + breakdown on reload. - Persist the breakdown only when the FINAL visible call emitted usage: track a per-response snapshot count and require primaryUsageCount >= snapshotCount. Previously any earlier primary usage event passed the gate, so a multi-call turn whose final call emitted no usage_metadata used an earlier call's output as completedOutputTokens (already counted by the latest snapshot) → reload over-reported. Now it falls back to the coarse estimate. Resumable stop pending-reset (prior round, 3cde6fe035) already flows through clearAllSubmissions → SSE close → the intentional-close handler's resetLive. Deferred per scope: mid-stream branch-switch pending attribution (tracked). * 🩹 fix: Abort breakdown over-count + resume re-fold after pending discard Codex round (on the re-applied abort/snapshot work): - buildAbortedResponseMetadata now persists ONLY the usage/cost rollup, not the context breakdown. The abort path can't tell whether the final call emitted usage (the job stores only the latest snapshot, not a count), so persisting the breakdown risked reusing an earlier call's output as completedOutputTokens (already in the snapshot) → reload over-count. Stopped/incomplete responses now fall back to the coarse gauge estimate, which is safe and apt. - resetLive now also forgets the conversation's folded usage-event identities (clearUsageFolded). Discarding pending on a terminal/intentional close left the folded keys set, so a later resume's backfillUsage saw the persisted events as duplicates and never rebuilt pending — leaving the response's usage missing until a full reload. Clearing them lets the resume re-fold. |
||
|
Danny Avila
|
db7011d567
|
📊 feat: Real-Time Context Window & Token Usage Tracking (#13670)
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: Real-Time Context Window & Token Usage Tracking
* 🧪 fix: Align Pricing Spec Dep Signatures with TxDeps
* 🩹 fix: Resolve Codex Findings for Context Usage Tracking
* 📊 feat: Granular Tool Token Breakdown with Deferred Splits
* 🧪 test: Cover Session Cost in Mock E2E and Scope Usage Selectors
* 🧪 test: Live Host-Pipeline Usage Verification (Env-Gated)
* 🧪 test: Local Real-Provider Multi-Turn E2E Harness
* 🪙 fix: Keep Tagged Usage Buckets Out of the Live Context Estimate
* 🩹 fix: Scoped Token-Config Fallback and Sequential Visibility for Usage Events
* 🩹 fix: Address Usage Review Findings — Cost Timing, Scoped Caches, Finalized Output
- carry the post-snapshot output estimate into the context snapshot at
finalize so the gauge keeps the last response after live resets
- accumulate per-rate billable units and price the session cost at
render, so usage events arriving before the token-config load still
count once it resolves
- pass user-scoped token-config cache keys through loadConfigModels
fetches and drop the controller's unscoped fallback to prevent serving
another user's resolved config
- tag emitted usage events with a per-run seq so resume dedupe never
drops a distinct call with an identical payload
- admit the static tokenConfig override in the custom endpoint schema so
it survives zod parsing into req.config
* 🩹 fix: Align Client Usage Accounting with Backend Cost Semantics
- classify cache tokens by provider (shared inputTokensIncludesCache from
data-provider, consumed by both the backend billing path and the client)
instead of a magnitude heuristic, so Anthropic/Bedrock turns where cache
is smaller than uncached input no longer under-bill input
- mirror resolveCompletionTokens on the client so Vertex-style hidden
thinking tokens are reflected in the Output row and session cost
- prefer endpoint pricing over adapter-provider pricing so a custom
endpoint can price a known model name without built-in rates shadowing it
- carry static cacheRead/cacheWrite overrides through the tokenConfig
schema and buildTokenConfigMap
* 🩹 fix: Honor Static Token Config in Billing; Tighten Usage Freshness
- initializeCustom now uses a static endpoint tokenConfig as the agent's
endpointTokenConfig (billing + balance checks), not just the advertised
UI config — previously the gauge showed admin rates while the agent
billed against built-in tables
- invalidate the token-config query alongside models on user-key add/
revoke so context windows and pricing refresh without a reload
- include maxContextTokens in ChatForm's stabilized conversation memo so
the gauge reflects a changed context-window setting immediately
- feed the live output estimate from the legacy content path (direct and
assistants streams), setting from cumulative part text rather than
accumulating deltas
* 🩹 fix: Resume Usage Dedup, Agent Pricing, and Partial Override Billing
- fold usage events idempotently by (runId, seq) so resume backfill no
longer resets the conversation totals — a mid-stream reconnect keeps the
usage of prompts already completed earlier in the session
- tap replayed pending message/reasoning/content events so output streamed
past the resume snapshot reaches the live estimate, not just the message
- resolve cost against the agent's backing endpoint (Agents conversations
report endpoint `agents` / provider `openAI`, neither of which keys a
custom endpoint's tokenConfig)
- getMultiplier/getCacheMultiplier fall back to the standard tables for
models absent from a partial endpointTokenConfig, so a partial static
override no longer bills non-listed models at defaultRate while the UI
shows the correct pattern rate
* 🩹 fix: Repaired Output in Gauge, Cache-Rate Keys, Config Gate, Usage Cleanup
- live/completed gauge counts the repaired completion (normalized output),
so under-reporting providers don't drop the response from used context
- translate static tokenConfig cacheWrite/cacheRead onto the write/read
keys getCacheMultiplier reads, so cache tokens bill at the configured
rate instead of the prompt-rate fallback
- clear the token index and usage atoms when leaving a conversation, so
visited histories don't accumulate in memory for the tab's lifetime
- wait for startupConfig before mounting the gauge, so a deployment with
contextUsage disabled never briefly mounts it or fires the token-config
query on first load
* 🩹 fix: Move Token-Config Resolution to TS; Key Live Usage by Created Convo
- extract the token-config resolution (override gathering + cache lookup +
buildTokenConfigMap) into resolveTokenConfigMap in packages/api, leaving
the /api controller a thin request-scoped wrapper (CLAUDE.md TS rule)
- getConvoKey prefers the user message's real conversationId once the
`created` event stamps it, so a new chat's first-response live gauge and
totals land under the id TokenUsage subscribes to instead of NEW_CONVO
* 🩹 fix: Clear Stale Redis Job Usage; Live-Tap Legacy Streams; Share Fetched Config
- DEL the Redis job hash before re-creating it so a reused streamId can't
inherit a prior run's contextUsage/tokenUsage and backfill stale usage
- tap the legacy {message,text} stream branch (non-agent OpenAI/Anthropic
streams) into the live estimate, not just the content path
- copy a deduped fetch's token config to every sibling endpoint sharing the
baseURL/key/headers, so /token-config resolves each by its own name
* ⏪ revert: Don't DEL Redis job hash in createJob (breaks cross-replica resume)
createJob is an idempotent join — a second replica calls it for the same
streamId to share an in-flight stream's state. DELeting the hash wiped the
prior replica's persisted created/usage state, so a joining replica missed
the created event (GenerationJobManager cross-replica integration test).
Reverts the F1 change from
|
||
|
Danny Avila
|
3c3837bb7d
|
🧾 fix: Bill Subagent Child-Run Model Usage in Parent Transactions (#13683)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🧾 fix: Bill Subagent Child-Run Model Usage in Parent Transactions * 🩹 fix: Type Subagent Usage Sink Structurally Until SDK Release * 🔧 chore: Update @librechat/agents dependency to version 3.2.35 in package-lock.json and related package.json files |
||
|
Danny Avila
|
793cbd49f0
|
✂️ fix: Deduplicate Skill Bodies Across Fresh Primes and History (#13610)
When a skill is primed fresh this turn (manual $-popover or always-apply) AND also appears in history as a `skill` tool_call, its SKILL.md body was injected twice — once by injectSkillPrimes and once reconstructed by formatAgentMessages. - add `collectFreshSkillPrimeNames` helper (packages/api) — union of manual + always-apply prime names - client.js: pass the set as `skipSkillBodyNames` to formatAgentMessages for both the initialMessages and memoryMessages paths so the body reconstructs once. Names not primed this turn still reconstruct (sticky manual re-prime). Requires `@librechat/agents` with `skipSkillBodyNames` support; the published dist silently ignores the unknown option until upgraded. |
||
|
Danny Avila
|
8fc2314208
|
🧠 fix: Bound Memory Agent Input (#13606) | ||
|
Danny Avila
|
dc42748813
|
🧷 fix: Bind Agent File Context to Current Turn (#13506)
* fix: Bind agent file context to current turn * fix: Avoid duplicating agent file context * fix: Export agent file context prepender * test: Use exported file context prepender * fix: Keep file context transient for memory and counts |