* 🧭 fix: Rebase Activity Phase Bounds Onto Compacted Content
`filterMalformedContentParts` compacts the aggregator's content array —
`Array.prototype.filter` skips holes and drops malformed tool calls — but a
parent phase marker's `activity_start_index`/`activity_end_index` still address
the pre-filter positions. The array is routinely sparse: the aggregator writes
parts at provider-source indexes, so a model turn that emits no text before its
tool calls leaves an empty slot.
Every part after a hole therefore shifts left on persistence while the bounds
stay put, so the stored phase claims the wrong range — the final answer is
swallowed into the parent card and the marker's own slot is counted as a child.
The in-run analogue (`rebaseActivityPhaseBounds`) already rebases after
completion-time reshaping; the final compaction had no such step.
Rebase the bounds as part of the compaction, mapping each bound to the number
of retained parts ahead of it. The mapping is monotonic, so `start <= end <=
markerIndex` survives, and an identity mapping leaves untouched arrays — and
their marker objects — exactly as they were. Markers are copied rather than
mutated so the caller's array keeps its own coordinates, which the live stream
and the resume snapshot still address.
Fixes the `activity-phases` e2e failure on dev and the same defect on the two
resume persistence paths.
* 🔌 fix: Stop Replacing the Env Module in the MCPManager Suite
`MCPManager.test.ts` mocked `~/utils/env` with a factory that replaced the whole
module. #14780 then made `~/mcp/utils` read `ALLOWED_BODY_FIELDS` from that
module at module scope, so importing `~/mcp/oauth` -> `handler.ts` ->
`~/mcp/utils` evaluated `undefined.map(...)` and the suite died at import time.
All 111 of its tests have been silently skipped since; the shard has been red on
dev, on this PR, and on release-v0.8.8-rc1.
Spread the real module and keep only the mock that earns its place.
`processMCPEnv` stays a seam: fifteen cases drive it with `mockReturnValue` /
`mockImplementation` to hand the manager a specific processed config, and one
asserts its call count, so making it real would couple these tests to
env-substitution logic. `isPluginSourced` and `MCP_PLUGIN_SOURCE` were dropped —
the factory restated the real implementations verbatim and no test referenced
either, so they were duplication, not a seam.
111 tests now run and pass.
* 🧪 test: Stop Replacing the Env Module in Three More Suites
Same latent trap as the MCPManager suite: a `jest.mock('~/utils/env', ...)`
factory that replaces the whole module. These three pass today only because
their import graphs never reach `~/mcp/utils`, which reads `ALLOWED_BODY_FIELDS`
from that module at module scope — the next module-scope constant added to
`env.ts` would break all three the same silent way.
Each mock is kept only where it earns its place:
- `activityLabels/host.spec.ts` — dropped. `createSafeUser` was never referenced
and the stub returned `undefined` where the real function returns `{}`, so the
mock was strictly less faithful than the real, pure implementation.
- `run-codeTools.test.ts` — dropped. Neither `resolveHeaders` nor
`createSafeUser` was referenced by any case.
- `run-summarization.test.ts` — `resolveHeaders` is now a spy wrapping the real
implementation rather than an identity stub. One case asserts templated header
values go through it, which only means something if the real substitution
actually runs. `createSafeUser` dropped as unreferenced.
103 suites / 2708 tests green across `src/agents`, `src/utils`, and the
MCPManager suite.
* 📝 docs: Describe the Full Contract of filterMalformedContentParts
Per Copilot's review: the public JSDoc still described the function as only
dropping malformed tool calls, while the implementation also compacts empty
slots and rebases parent activity-phase bounds. The detail lived on the private
helper, so callers reading intellisense saw a stale contract.
State what it actually produces, note that compaction is inherent rather than
incidental (the aggregator writes at provider-source indexes, so the array is
frequently sparse), and add an example of a hole moving a phase bound. The
example was verified against the built runtime, not written from memory.
* 📱 fix: Show Quote Popup for Block Selections and on Touch Devices
The "Add to chat" popup never appeared for two whole classes of selection.
Block-granularity gestures (triple-click, double-click then word-drag) park
the selection's far boundary at the start of the next block. For a message's
closing block that boundary sits outside `.message-render` — on the composer
wrapper or the following message row — while selecting no text there, so the
anchor/focus equality check suppressed the popup. Triple-clicking any earlier
paragraph worked, which is what made this look like an edge case. The range is
now clamped to the message before the check, and selections that really do
carry visible text from another message are still refused.
Touch platforms could not reach the feature at all. A long-press, and every
drag of the native selection handles, emits no mouse event whatsoever — only
`selectionchange` — while the popup was shown exclusively from mouseup,
dblclick and keyup. Showing now also hangs off a settle-debounced
`selectionchange`, gated so an in-progress mouse drag still cannot flicker it.
Accepting was broken independently: the tap is also the gesture that dismisses
the selection, unmounting the button before `click` could land, so touch
commits on `pointerdown` instead. The desktop mousedown path is deliberately
unchanged, since preventDefault on `pointerdown` can suppress the
compatibility mousedown that click depends on.
Two UX consequences of the same code: scrolling re-anchors the popup rather
than dismissing it on the first event (the chat auto-scrolls constantly while
streaming, and a mobile URL bar collapsing fires resize), and touch selections
place the button below the text, clear of the OS Copy/Share callout, with a
44px tap target.
Covered by six e2e tests — three desktop, three on an emulated Pixel 5 with a
real touchscreen — each verified to fail against the pre-fix build.
* 🩹 fix: Address Review Findings and Repair the Scroll Specs
The two failing e2e shards were a defect in the specs, not the component.
`scrollMessages` reached for `.scrollbar-gutter-stable` with a document-wide
query, but the nav and side panels carry that class too, so it could grab a
sidebar list that never scrolls — 0px moved, and only in CI, where the nav
renders differently. The scroller is now reached from the message itself, the
way `MessageNav` does it. The specs also centre the selection first and nudge
by a quarter of the visible height, so the gesture cannot scroll the selection
clean out of view and then blame the popup for going with it.
Review findings, all in `QuoteButton`:
Visibility was tested against the window, but the list scrolls inside a bounded
container, so text can sit clipped under the header or the composer while its
un-clipped rect is still inside the window — leaving the popup floating over
unrelated UI. It is now clipped to the nearest scrollable ancestor.
Touch committed on the press, so starting a scroll on the button, or touching
it and thinking better of it, still added the quote. The excerpt is captured on
the press and committed on the release, and only when that release lands on the
button, restoring the cancellation every button is expected to have. Commit on
press existed because the tap dismisses the selection before `click` fires;
capturing the text up front keeps that safe, and an in-flight press is no
longer allowed to unmount its own target.
A visible popup also described the previous selection for up to the settle
window, so a tap while dragging a native selection handle queued the stale
excerpt. It is dropped as soon as a differing selection starts settling.
Finally, `viaTouch` survived from the last press into keyboard-driven
selections on hybrid devices, which could flip the popup into the touch layout;
keydown clears it.
The cancel path is covered by a new touch spec, verified to fail against a
commit-on-press build.
* 🧵 fix: Reconcile Cancelled Presses, Widen Clipping, Steady the Scroll Specs
Second review round, with one finding taken on trust and flagged rather than
claimed as proven.
A cancelled touch press could leave the popup backed by a selection that no
longer existed. A press deliberately keeps the button alive through a
collapsing selection so the release has a target to be judged against, but a
cancel then dropped the press without ever honouring the collapse it had
masked, so a later tap could add a dead excerpt. Ending a press without
committing now rechecks the live selection and dismisses if it went away.
Visibility now intersects every clipping ancestor of the message rather than
stopping at the nearest. This one is precautionary, not a proven fix: the
review that prompted it describes scroll containers *inside* a message (a wide
table, a code block) shadowing the outer chat scroller, but the walk starts
from the message element, so those are descendants and were never in the chain.
Behaviour is unchanged in the current layout — a spec covering a table-cell
selection passes identically with and without it — and it is kept only because
intersecting the whole chain stays correct if the list is ever nested inside a
further-clipped panel. The comment says exactly this.
The scroll specs were the real instability. They now move the selection between
two positions that are both on screen instead of nudging by a pixel count:
blind nudges kept pushing it under the composer, where the popup correctly
hides, and the chat's own auto-scroll made the landing spot unpredictable. They
also target the opening paragraph, since the closing one is the last content in
the conversation and cannot be carried upward from a list already at maximum
scroll.
The reply fixture gained a table so a selection inside a nested scroll container
is exercised, and a spec covers the cancelled press.
15/15 pass locally.
* 🪟 fix: Judge Quote-Popup Visibility From the Selection, on Both Axes
Third review round. All three findings held up, and each now has a spec that
fails without its fix.
Clipping is now measured from the selection rather than from the message, and
on both axes. A wide table or a long code line scrolls inside its own container
— and `overflow-x: auto` makes the computed `overflow-y` auto, so it clips
vertically too — which means scrolling it sideways carries the selected text out
of view while the message never moves. Walking up from the message could not see
those containers at all, and a vertical-only test could not see that motion.
This supersedes the previous round's precautionary widening, which was kept
without evidence; the evidence is now a spec that scrolls a table past its own
selection.
Publishing a settled selection also checks visibility. Nothing is tracked during
the 300ms settle interval, so a scroll inside that window never reached the
re-anchoring path, and the reading was published off-screen and then clamped
into view — stranding the popup over unrelated UI.
The cancelled-press spec now reproduces the ordering it describes. Collapsing
the selection and cancelling in one synchronous block let the asynchronous
`selectionchange` arrive after the press had ended, which is the ordinary path
and passes either way; it now waits for delivery in between, so the collapse
lands while the press is still masking it. Two other specs needed the same
scrutiny: `toBeHidden` is satisfied by an element that does not exist yet, so
the settle spec sits out the interval before asserting, and it scrolls just past
the container edge rather than to the end of the conversation, because a violent
scroll re-renders the messages and drops the selection for unrelated reasons.
The reply fixture's table is now wide enough to overflow sideways.
17/17 pass, and each new spec was re-run against a build with its own fix
reverted to confirm it fails there.
Follow-up to #14774. Its tests cover `AgentClient.recordTokenUsage` in
isolation, so the `BaseClient` half of the fix was unpinned: deleting the
`transactions` property from the call site restored the bug with the suite
still green.
These cases drive `sendMessage` with a real app config on `req` and assert the
resolved value reaches `recordTokenUsage` — disabled, the default when no
config is present, and the balance-enabled override that force-enables it.
Each fails if either half of #14774 is reverted.
They also isolate `options.endpoint` for the block. `options` is shared across
this file, and an endpoint left behind by an earlier case routes the
balance-enabled arrangement into `checkBalance`.
* 🛠️ refactor: Enhance console color handling and improve deleteNodeModules function
* refactor: Use coloredConsole for consistent console output in invite-user.js
* refactor: Convert year to string format in invite user payload
`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.
* 🫧 refactor: Fading Pre-Response Dot, Legacy Cursor CSS Removed
The pre-response dot keeps its classic 12px shape but now enters on the
streamed-text fade curve (lc-fade-in) and breathes on opacity, replacing
the legacy pulseSize scale throb that read as jittery. One dot for all
users — the interim result-thinking-fade variant and its per-component
class wiring are removed, and the dot goes static under
prefers-reduced-motion.
Clears out the legacy cursor CSS while here: the scaleX/scale3d mixed
keyframes, -webkit- prefixed animation/transform/backface duplicates,
translateZ and will-change layer hacks, subpixel font-smoothing, the
duplicate #0d0d0d fallback, the unused .blink utility, and all three
duplicate @keyframes blink definitions (the auth logo uses Tailwind's
logo-blink; nothing references these).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* 📝 docs: Explain the Pre-Response Dot's Custom-CSS Exception
Per AGENTS.md, custom CSS needs a stated reason a shared primitive cannot
express the requirement: the dot is a pseudo-element gated on an ancestor's
`.submitting` state plus its own `:empty`, kept in CSS so it costs no React
renders per streamed token.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix: treat unrecognized SKILL.md frontmatter keys as warnings
An unknown key in one SKILL.md failed that skill outright, and because the
GitHub sync runner marks a source failed on any validation error, a single
stray key took down every other skill in the repository. Syncing
github.com/cloudflare/skills failed entirely because 2 of its 13 skills
carry a `references:` key.
UNKNOWN_KEY is now a warning, so the skill is stored (unknown keys and all)
and the issue is surfaced rather than fatal. `references` joins the allowed
set with a shallow JSON-safety check instead of a strict kind match: real
files use a string, a list of strings, a list of objects, and a map, and
pinning one shape would reintroduce the same failure.
Malformed frontmatter stays fatal: INVALID_TYPE, INVALID_SHAPE and the
non-plain-object check are unchanged.
* fix: skip individual skills instead of failing a whole sync source
Any error inside the discovery or commit loop reached the outer catch and
marked the entire source failed, so one unusable SKILL.md, one oversized
blob, or one duplicate name cost every other skill in the repository.
Each skill now runs inside its own boundary and a failure is recorded
against that skill. Errors that mean nothing else in the run can succeed
(lock loss, GitHub auth failures, rate limiting) still abort the source
rather than being charged to whichever skill hit them first. Skills are
marked seen before the attempt, so the reconcile pass cannot mirror-delete
the previously synced copy of a skill a later run can repair, and duplicate
names now drop the whole colliding group instead of letting tree order pick
an arbitrary winner.
Status gains `partial` (published some, skipped others) plus a capped
sample of the skipped skills with the reason for each. A run that publishes
nothing and skips something is still `failed`, carrying the first skip's
error. The skipped entries name repository paths, so they follow the same
visibility rule as owner/repo/paths; the bare count does not.
Sync warnings are logged too: a background run has no user-facing surface,
so the log is the only place a maintainer sees why an upstream SKILL.md
looks off.
* test: cover skill sync warnings reaching the log
An unrecognized frontmatter key no longer fails the skill, so a background
sync has nowhere to report it except the log. Every mock in this spec
returned an empty warning list, which left that path unexercised.
* fix: describe nested frontmatter values in the shared skill type
`SkillFrontmatterValue` allowed only scalars and string arrays, while the
server has always stored `hooks` and `metadata` as JSON-safe objects, and now
`references` too. A skill carrying any of them could not be represented by
`TSkill`, `TCreateSkill` or `TUpdateSkillPayload` without a cast.
The type stays free of `any` and `unknown`: values remain JSON-safe by
construction, and the server keeps bounding depth, string length and array
size when it validates them.
* fix: protect moved mirrors and rolled-back counts when a skill is skipped
Continuing past a failed skill exposed two problems that aborting the whole
source used to hide.
A moved skill's mirror keeps its old upstream id until the update lands, and
only the new path was marked as seen, so the reconcile pass read the mirror as
stale and deleted the very copy the skip path exists to preserve. The old id is
now marked as seen too.
Deletion counters were incremented when a stale name-conflicting mirror was
removed, but never undone when the following commit failed and the mirror was
restored. The run no longer stops there, so the status persisted a deletion
that did not happen and the reconcile pass counted the restored row again.
Counters are now rolled back when the restore succeeds.
* fix: bound unknown frontmatter values and keep moved mirrors through duplicates
Tolerating an unrecognized key meant its value skipped the shared JSON-safety
check, so a deeply nested or oversized payload was accepted and persisted under
a key nobody validates. The key stays non-blocking; the value is now held to
the same depth, array and string bounds as every structured key.
A skill that moves into a name another discovered skill also claims is dropped
with the rest of its duplicate group before the sync path can reuse its mirror,
which left the still-published copy unmarked and reconciled away. Both paths now
mark the moved mirror through one helper.
* fix: end the source when a skipped skill fails to roll back
A skill that fails and rolls back cleanly is just a skipped skill. One whose
restore or delete also fails leaves a mirror with half-rewritten files or a
half-created row, and the run now continues past it, so the source could report
partial success while that mirror stayed inconsistent and its pre-marked
upstream id kept reconciliation away from it.
Failed rollbacks now raise a source-fatal error carrying the original failure,
which stops the source the way a lost lock or a refused GitHub token does.
* test: cover a skipped skill discovered at the repository root
A repository-level SKILL.md is discovered with an empty path, so this pins
that a skip recorded against it still persists with the rest of the partial
status rather than taking the whole status row down with it.
* docs: describe unknown skill frontmatter warnings
* fix: preserve mirrors after partial skill sync
* fix: preserve skill validation details during sync
* fix: fail sync when mirror identity cannot be restored
* fix: harden skill sync failure boundaries
* fix: preserve skipped skills on fatal sync
* fix: surface skill sync diagnostics and rollback failures
* fix: preserve skill frontmatter extension keys
* fix: reject skill frontmatter keys that collide when normalized
Frontmatter keys are matched case-insensitively against the canonical
key list, so "Name" and "name" both resolve to "name". Every call site
normalized independently, and the last key in iteration order silently
won, meaning the effective value depended on YAML ordering rather than
on anything the author could see.
Centralize the normalization in normalizeSkillFrontmatterKeys and have
it fail when two recognized keys resolve to the same canonical key,
rather than picking one. parse.ts, deployment.ts and the agent handler
now surface that as a parse error; createSkill and updateSkill surface
it as a blocking DUPLICATE_KEY validation issue. Unrecognized keys are
still passed through untouched so extension frontmatter survives.
deriveStructuredFrontmatterFields and both write paths now run on the
normalized map, so a "Disable-Model-Invocation" key derives the same
column a lowercase one does.
* fix: harden github skill sync against dropped requests and failed cleanup
Three failure paths in the GitHub sync could leave a source looking
healthier than it was.
githubJson only handled HTTP-level errors. A fetch that rejected before
producing a response (DNS failure, socket reset, abort) escaped as a
raw TypeError, so the sync reported a generic crash instead of a typed
sync error. Wrap it as GITHUB_REQUEST_FAILED and add that code to the
fatal set, since a source whose requests never complete cannot be
partially synced.
When a synced file failed to persist, the orphaned upload was cleaned
up on a best-effort basis and the cleanup error was only logged. If the
cleanup itself failed, the source still ended with the original error
and left a real orphan behind. Promote that to a rollback failure so
the source reports SYNC_ROLLBACK_FAILED with the triggering error.
Skill warnings were logged inside commitRemoteSkill, before the file
sync and viewer setup that can still roll the skill back. A skill that
never survived publication therefore emitted warnings as though it had.
Return the warnings from the commit and log them once the skill is
fully published.
* fix: report skipped github skills before credential errors
serializeErrorMessage checked isCredentialError first, and that check
matches on the error text. A skipped skill whose path happens to
contain a credential-ish word, for example skills/credential-helper,
was therefore redacted to "GitHub skill sync credentials are not
available" for admins without credential-metadata access, hiding a
parse failure behind a wrong diagnosis.
Check the promoted skipped-skill case first, since it is identified by
error code rather than by text and is the more specific match. The
credential redaction still applies to everything else.
* 🚣♀️ feat: Smooth Streaming Text Fade-In
Adds a native FlowToken-style fade-in for streamed message text, with no
changes to the streaming data path: a per-block rehype plugin wraps newly
arrived words in one-shot CSS fade spans, using document-order character
offsets so already-visible text never re-animates, even when markdown
re-parsing restructures the tree. Words still inside their animation
window replay identical props so in-flight fades are never cut short.
- New Content/animate.tsx: word splitting (Intl.Segmenter for CJK),
offset-based new-text classification, rehype plugin factory, and an
AnimatedText component for plain-text reasoning content
- MarkdownBlocks: per-block plugin instance appended to the cached
rehype plugins only while animating; block memoization untouched
(bench asserts identical code-block render counts)
- Animation gated on isSubmitting && isLatestMessage and dropped at
stream end, so settled messages render without wrapper spans
- Skips code, pre, math/KaTeX, artifacts, citations, and MCP UI subtrees
- Smooth streaming toggle (default on) under Settings → Chat → Messages;
animation disabled under prefers-reduced-motion
- Extends the streaming bench with a fade variant (+9% render time in
jsdom, 62/62 code-block renders) and adds unit tests for the plugin,
classification, and AnimatedText
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* 🔧 chore: Fix Import Order in Markdown and Settings Registry
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* ✅ test: Cover Kana-Only CJK Word Segmentation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* 🛡️ fix: Harden Streaming Fade for Resume, Concurrency, and Reduced Motion
Addresses Codex review on the smooth streaming fade:
- Hydrated/resumed content becomes the animation baseline: a renderer whose
first run already exceeds FADE_HYDRATION_THRESHOLD (reconnected stream,
conversation switch) no longer re-fades the entire accumulated response
- Classification is now transactional under React 18 concurrency: runs are
staged during render and published via commit() from a layout effect, so
abandoned renders (interruption, StrictMode double-render) leave no trace
- prefers-reduced-motion now disables the rehype transform and AnimatedText
in the render gate, not just the CSS animation, so reduced-motion users
skip the span-wrapping work entirely
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* 🌏 fix: Refine Fade Hydration Signal, Spaceless Scripts, Collapsed Reasoning
Second Codex review round on the smooth streaming fade:
- Hydration is now an explicit signal instead of a per-block length guess:
Markdown captures whether substantial content already exists at the render
where its animate gate flips on, and passes it to each block's plugin. New
blocks mounting later in the stream always animate regardless of size, and
resumed content becomes the baseline regardless of block sizes
- Word segmentation now covers all spaceless scripts (Thai, Lao, Tibetan,
Myanmar, Khmer) in addition to CJK/Hangul, so continuously streamed text in
those scripts keeps fading instead of freezing after the first window
- Reasoning text no longer runs the word transform while the thinking panel
is collapsed; expanding mid-stream starts from a hydrated baseline
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* 🖱️ feat: Hide Streaming Cursors While Smooth Fade Is Active
The word fade itself signals streaming, so the trailing block cursor
(result-streaming) and the pulsing thinking dot (result-thinking) are
now suppressed whenever the smooth streaming fade is enabled. Both
return when the setting is off or the device prefers reduced motion.
Extracts the shared gate into a useSmoothStreaming hook consumed by
Markdown, Reasoning, TextPart, DisplayMessage, EmptyText, and the
legacy loading fallback.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* ✅ test: Pin Cursor Assertions to Fade-Off State
The pulsing thinking cursor now only renders while the smooth streaming
fade is off, so the tests asserting it set the toggle off first (at file
level where earlier renders would cache the atom's first read).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
* 💫 feat: Restore Start-of-Run Dot With Fade-Matched Treatment
The trailing streaming cursor stays hidden while the fade is active (the
fade itself signals text arriving), but the pre-first-token dot returns:
nothing else tells the user the run started before any text exists.
Restyled to match the word fade rather than the classic size throb — it
fades in on the same 250ms ease-out curve, then breathes on opacity, so
"run starting" and "text arriving" read as one system. Applied only when
the fade is active; with the setting off or reduced motion preferred the
dot keeps its original pulseSize behavior, so the cursor assertions in
the existing suites hold unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrUMn8aEKtMY2QRZhpheJB
---------
Co-authored-by: Claude <noreply@anthropic.com>
* 🪝 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>
* 🔐 fix: Preserve Plugin MCP Provenance Through Registry Storage
Agent Plugins MCP servers are tagged `source: 'plugin'` by the plugin
loader so `processMCPEnv` returns them verbatim, keeping any `${VAR}` a
plugin declared literal. The registry derived `source` from the storage
tier alone, so every startup server routed through
`addServer(..., 'CACHE')` was retagged `'yaml'` — before inspection and
again before persistence.
That dropped the marker for deployment plugin servers merged into the
startup MCP config, so `processMCPEnv` treated plugin-authored strings as
operator-authored templates and expanded them from `process.env`. A
malicious plugin declaring `Authorization: Bearer ${OPENAI_API_KEY}`
received the host's key at its own endpoint, at both boot-time inspection
and every runtime connection.
`resolveServerSource` now carries an existing plugin marker through
instead of re-deriving it, and is applied at all four tag sites
(`addServer`, `addServerStub`, `inspectServerUpdate`, and config-tier
lazy init, which hardcoded `'config'` and would have re-opened the same
hole). The marker is only honored for operator-loaded tiers: a DB entry
is user-authored and stays `'user'`, so user input cannot claim plugin
provenance to escape the sandboxed placeholder rules.
* 🔒 fix: Address Codex review — config-override provenance & upgrade re-tag
Two follow-ups from the Codex review of the provenance fix, plus a test
cleanup.
P2 — a Config-tier override that shadows a same-name plugin base inherited
the base's `source: 'plugin'` through the merge in `getServerConfig` /
`getAllServerConfigs`, so `processMCPEnv` stopped resolving the operator's
own `${VAR}` placeholders and silently broke their server. New
`overlaySource` helper keeps an operator override on its own trusted
source when the base is plugin-sourced; all other bases still inherit as
before. Fails safe (never a leak), but the regression is real.
P1 — the init fingerprint hashes only the raw MCP config, which already
carried `source: 'plugin'` before the provenance fix, so the hash is
unchanged by it. On a Redis-backed rolling restart with no config change,
followers short-circuit on the stale `INITIALIZED_CONFIG_HASH` and the
old `source: 'yaml'` plugin entries survive with no expiry — the fix
never takes effect. Fold a `REGISTRY_STORAGE_SCHEMA_VERSION` into the
fingerprint so an upgrade forces exactly one cluster-wide re-init.
Also drop unnecessary `as` casts in the provenance tests (declare the
fixture as `ParsedServerConfig`, assert with `toMatchObject`) and add a
regression test for the P2 override case.
The full-sweep regression gate lints api+client+packages twice — once under
the PR's config and once under the base ref's — inside the same job as ~20
later steps (data-provider/data-schemas/api builds, config migration tests,
unused-i18n scan, and four depcheck passes), all sharing one 30-minute budget.
Two type-aware sweeps of the whole tree cost more than everything else in that
job combined. When they run long the job hits its timeout mid-sweep, so every
step behind the gate never executes and Static checks reports no result at all
— strictly worse than not running the gate. continue-on-error: true hides this,
because the step never fails; it simply never finishes.
Move the gate to its own job with its own budget so it cannot starve the other
checks, and bound each sweep so an over-budget run degrades to a notice rather
than a failure — an unfinished sweep is no evidence of a regression, and the
gate is advisory about config scope. Behaviour on a sweep that completes is
unchanged: coverage loss and new (file, rule, severity) diagnostics still fail.
* 🧩 ci: Close Workflow Path-Filter Gaps
Six trigger-filter gaps found by reading all workflows against the live
dependency graph (AI-1755, codegraph FINDINGS §6l):
- backend-review/frontend-review: root package.json/package-lock.json now
trigger unit tests — a lockfile-only dependency bump previously ran zero
backend or frontend unit tests while every test job installs from it
- agents-integration-tests: widen to the three package src trees it builds
and imports (was only src/agents/**)
- cache-integration-tests: same shape — verified live that
packages/api/src/flow/manager.ts (imported by mcp/oauth) matched neither
integration filter
- docker-smoke: plain Dockerfile had no PR-time validation despite shipping
via dev-images/tag-images; new node-image-smoke job builds it, gated by
paths-filter to Dockerfile/.dockerignore changes
- dev-images/dev-branch-images: add config/**, skill/**, .dockerignore —
the single-stage image COPYs the full build context
- static-checks: eslint.config.mjs now re-triggers the lint job (gap carried
over from eslint-ci.yml in the #14716 consolidation)
- delete generate_embeddings.yml: fired on docs/**, which no longer exists,
and its docs-root-path pointed at the same missing directory
* 🧩 ci: Address Codex Review Findings
- Build caches: all 26 build-* keys across 8 workflows now lead with
root package.json + package-lock.json so manifest-only bumps cannot
restore stale dists (data-provider embeds the root version); unifies
the split key families (playwright already hashed the lockfile)
- static-checks: config changes now gate on the ESLint config loading
and applying to representative files, plus a report-only full-tree
sweep (70 pre-existing errors at dev HEAD block a hard gate for now)
- docker-smoke: the workflow file itself triggers the plain-Dockerfile
build so job edits are validated
- dev-images/dev-branch-images: re-include skill/**/*.md after !**.md
so shipped deployment-skill Markdown rebuilds images
* 🧩 ci: Gate Config Lint Sweep on Regression vs Base Config
Second-round codex finding: the report-only sweep swallowed config-wide
breakage in scoped blocks the representative files don't exercise. The
sweep now lints the same tree under the PR's config and the base ref's
config and fails only when the PR's config produces more diagnostics for
some (file, rule) pair — pre-existing debt never fails the gate, and
fixes are never penalized. Base-config unavailability degrades to the
load gate with a notice. Outcome surfaced in the failure summary.
* 🧩 ci: Harden Config Lint Gate per External Review
- Coverage direction: fail when the PR config stops linting files the
base config covered (set difference on linted files) — a mis-scoped
ignores previously only removed diagnostics and passed both gates
- Severity-aware fingerprints: (file, rule, severity) so warn->error
escalations gate on a clean tree for that rule; downgrades still free
- Hard-fail when the base commit is missing so a future shallow-checkout
change cannot silently disable the gate; annotate fetch-depth: 0
- Tab-separated fingerprint keys (space-in-path proof), --config on
both sweeps, EXIT trap for the base config copy, comment on why it
must live at the repo root (flat-config pattern base paths)
- Narrow skill md re-include with !skill/README.md: top-level README is
documentation-only; runtime skill Markdown still rebuilds images
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.
* 📁 fix: Consistent Export Filenames Across Formats
Conversation exports produced different filenames per format: txt/md/csv go
through export-from-json, whose default formatter replaces only the first
whitespace run (non-global regex), while png/json downloads keep spaces.
Normalize the filename once before export so every format yields
Word1_Word2_Word3.ext; the preset JSON export shared the same defect.
* 🧹 chore: fix import order via sort-imports
* feat(web-search): route outbound search and scrape requests through the SSRF-safe agent
Build the SSRF-safe agents at the web-search tool-assembly site and pass them into the
search tool config so outbound search and scrape connections are validated at connect
time against their resolved IP, on every hop including redirects, consistent with the
other outbound clients.
Add allowedAddresses to webSearchSchema, reusing allowedAddressesSchema, so self-hosters
can permit a deliberately-private search or scrape endpoint (for example a private
SearXNG instance). The field is resolved directly from the webSearch config at the
createSearchTool call site, not through loadWebSearchAuth, because it is config and not
an auth credential. webSearchSchema is flat (providers are chosen by enums, not by
counting keys), so the field is inert with respect to provider selection.
Document the field and its operator warning in librechat.example.yaml, and assert the
wiring in handleTools.test.js: the SSRF-safe agents are threaded into the search tool
config, allowedAddresses is passed through when set, and omitting it still threads the
agents with no exemptions.
TODO awaits @librechat/agents release with the httpAgent hook: this consumes optional
httpAgent/httpsAgent fields on the search-tool config that are not yet in a published
@librechat/agents. package.json is intentionally left at the current version; bump it to
the release that ships the hook before this lands. Validated locally against a revendored
@librechat/agents build, not a published release.
* 🛡️ fix: Apply allowedAddresses to the Web Search SSRF Preflight
The connect-time SSRF agent already honors webSearch.allowedAddresses, but
loadWebSearchAuth ran the isSSRFUrl preflight without it, so an admin-permitted
private search or scrape URL was stripped before the agent could ever use it.
Thread allowedAddresses and the URL's effective port through isSSRFTarget and
resolveHostnameSSRF so the exemption is consistent across both SSRF layers.
* 🛡️ fix: Validate Web Search Destinations and Defer to Configured Proxies
Handing agents to createSearchTool covered only the connect-time DNS lookup,
which Node skips for IP-literal hosts, and a configured proxy connects on our
behalf without running that check. A literal private target such as
http://169.254.169.254 could therefore reach the network.
Route every resolved web-search destination through the existing
applySSRFSafeAgentIfDirect contract so a blocked literal target throws before
any request is made, and withhold the agents when a proxy owns egress, since one
agent pair is shared by every provider and a direct-connect agent on a proxied
connection would break the request while asserting protection the proxy's
network context cannot provide.
* 🛡️ fix: Keep Web Search SSRF Agents Under a Proxy and Restore Pooling
Withholding the agents whenever a proxy was configured removed protection from
every direct and NO_PROXY destination in exchange for preventing a failure that
cannot occur: for an https target Axios substitutes its own CONNECT tunnel, so
the injected agent is never used for the proxy connection. Only a plaintext http
target keeps our agent and repoints it at the proxy, and only a proxy whose
hostname resolves private then trips the connect-time check.
Always pass the agents and exempt the proxy endpoint instead, deriving host:port
from the same PROXY, HTTP_PROXY, and HTTPS_PROXY resolution the rest of LibreChat
uses so the proxy hop stays reachable while destinations remain guarded. Axios
already applies NO_PROXY per request, so bypassed routes keep enforcement with no
extra logic.
Drop the load-time destination validation. It duplicated the isSSRFTarget
preflight for user-provided URLs, rejected admin values that were previously
legal, and threw from inside loadTools, where both loader wrappers swallow the
error and drop every tool for the turn rather than degrading web search alone.
Build the agents with keepAlive and cache them per exemption list. A bare
http.Agent does not pool, so the previous code replaced the pooled global agents
for every search, scrape, and rerank call and allocated a fresh pair per turn.
* 🛡️ fix: Reject IP-Literal Private Targets on Web Search Connections
Node resolves nothing for a literal host, so the connect-time lookup never saw
one: a destination or a redirect target given as http://169.254.169.254 reached
the network. Redirect hops pass through the same createConnection, so checking the
literal there covers both cases and removes the need for a maxRedirects control
that createSearchTool cannot accept.
Gate it behind blockLiteralHosts so only web search opts in. A caller that
reaches a proxy or a deliberate private service by literal address must exempt it
first, and the merged consumers of createSSRFSafeAgents have no such exemption, so
enabling this everywhere would break configurations that work today.
* 🛡️ fix: Keep IPv6 Brackets on Derived Proxy Exemptions
The exemption parser accepts an IPv6 entry only as [ipv6]:port, so stripping the
brackets produced fd00::1:3128, which carries three colons and is dropped as
malformed. An IPv6 proxy therefore stayed unexempted and the connect-time check
rejected it, failing every web-search request routed through it.
Use the URL hostname as parsed, which already carries the brackets.
* 🛡️ fix: Exempt Proxies Configured Through ALL_PROXY
Axios resolves a proxy through proxy-from-env, which falls back to all_proxy in
either case after <protocol>_proxy, so ALL_PROXY on its own is enough to route a
request through a proxy. Exemptions were derived from PROXY, HTTP_PROXY, and
HTTPS_PROXY only, leaving such a proxy unexempted and rejected with ESSRF.
Derive the exemptions from the full set of variables that can put a proxy in
front of these requests instead. The installed proxy-from-env 2.1.0 reads no
npm_config variables, so those are deliberately not included.
* 🛡️ fix: Drop the Unearned PROXY Exemption and Harden the Web Search Guard
Nothing on this path consumes PROXY: Axios resolves proxies through
proxy-from-env, which reads only <protocol>_proxy and all_proxy, and web search
never calls applyAxiosProxyConfig. Exempting it therefore granted a bypass rather
than preserving a working route, and a user-settable search URL that redirects to
that address reached it and returned the body. Remove PROXY and proxy, and skip a
socks endpoint for the same reason, since Axios cannot proxy through one.
Tolerate a non-array allowedAddresses instead of spreading it, which threw out of
loadTools and dropped every tool for the turn. The YAML path is schema-validated
but the admin override path merges without parsing, so the value is reachable.
Separate cache keys with NUL rather than a newline, so an entry containing a
newline cannot collide with two separate entries, and bound the cache. Give the
agents the idle timeout the global agents carry, which keepAlive alone did not
restore. Reject a unix socket, which carries no host to validate. Also treat
fec0::/10 site-local as private, matching the fe80::/10 handling beside it.
Exercise the real resolver in handleTools.test.js rather than mocking it, so the
wiring test now fails if the agents it threads do not actually block a private
target.
* 🛡️ fix: Derive Proxy Exemptions Through Axios's Own Resolver
Unioning every populated proxy variable exempted addresses that never carry a
request. proxy-from-env picks a protocol-specific variable before all_proxy and
lowercase before uppercase, so an ignored value became a trusted host:port that a
redirect onto a direct route could reach. It also normalizes a scheme-less value
such as proxy.internal:3128 to an http URL, where parsing the raw string yielded
an empty hostname and no exemption at all, breaking the proxy hop.
Resolve through getProxyForUrl, the entry point Axios itself calls, so precedence,
scheme normalization, and NO_PROXY match exactly and cannot drift. NO_PROXY
covering everything now yields no exemption, since nothing is proxied. Declared
locally rather than adding a types package, alongside the existing declaration in
the same directory.
Also revert the fec0::/10 site-local change. domain.spec asserts that boundary
deliberately to prove the fe80::/10 mask does not over-reach, and the shared
address schema still classifies fec0 as public, so a runtime block there would
leave operators unable to configure the exemption. It belongs with those two
together, not in this PR.
* 🛡️ fix: Resolve Proxy Exemptions Against the Real Destinations
Resolving against placeholder probe hosts applied destination-specific NO_PROXY
rules to a host nobody dials. With NO_PROXY matching the probe domain but not a
real provider, no exemption was derived even though Axios still proxied the actual
request, so the agent rejected the private proxy hop with ESSRF.
Resolve per configured destination instead, passing the values loadWebSearchAuth
already resolved. Only plaintext http destinations are considered, since for an
https destination Axios substitutes its own CONNECT tunnel and never uses the
injected agent for the proxy connection, which is also why provider defaults need
no exemption: every one of them is https.
* 🛡️ fix: Accept Embedded-IPv4 IPv6 Forms in the Address Exemption Schema
The runtime guard blocks 6to4, NAT64, and Teredo addresses whose embedded IPv4 is
private, but the schema's local copy recognized only ULA, link-local, and the
dotted IPv4-mapped form, so an entry such as [64:ff9b::a00:1]:8080 was dropped as
a public literal. An operator reaching a private endpoint that way could not
configure the exemption at all.
Mirror hasPrivateEmbeddedIPv4 in the schema helper, which the surrounding comment
already asks to keep in sync. Public embedded addresses stay rejected, since an
exemption there has no defensive purpose.
