[pull] main from danny-avila:main - #253
Merged
Merged
Conversation
* 🪟 fix: Lift Portaled Popovers Over the Dialog They Belong To `Select`, `Combobox` and `HoverCard` portal their content to the body and carry the shadcn default layer — `z-40` / `z-50`. `OGDialogContent` sits at 140 over an opaque overlay at 130, so inside a dialog those popovers open *behind* both: invisible and unclickable, while the dialog around them keeps taking clicks. Reported for the Run Code tool settings, whose two environment selects and help hovers moved into that dialog in #15722. `usePopoverZIndex()` already solves this for `DropdownMenu`, `DropdownPopup`, `Tooltip` and `ControlCombobox`. These three could not simply adopt it: four consumers raise `HoverCardContent` to `z-[999]` by class to clear the legacy `Dialog`, and an unconditional inline z-index would silently outrank them. So `useNestedPopoverZIndex()` returns the layered value only inside a dialog and `undefined` outside it, leaving the CSS layer — and every override of it — exactly as it was. The regression test asserts the popover's own inline z-index against the dialog's own constant, and pins the unchanged behaviour outside a dialog. Verified failing with the fix removed. `Combobox` carries the identical one-line change without a case of its own: no dialog hosts one today, and driving Radix Select's open state through jsdom hangs the suite. * 🖱️ fix: Restore Pointer Events on In-Dialog Popovers Codex was right on both counts, and they share one cause: the app carries THREE copies of `@radix-ui/react-dismissable-layer` (and two of `react-focus-scope`), because `react-dialog` is pinned at 1.0.2 (#11023) while `react-select` and `react-hover-card` resolve to their own newer ones. Radix coordinates nested layers through module-level state, so layers from different copies never see each other: the dialog parks `pointer-events: none` on the body and the popover from another copy is never granted the `auto` that Radix would otherwise give it. Measured in jsdom, dialog open, popover open: as installed body none / card "" → inert deduped body none / card auto → interactive `useNestedPopoverZIndex` becomes `useNestedPopoverStyle`, carrying `pointer-events: auto` alongside the layered z-index — the same trade `DropdownPopup` already makes for Ariakit menus. Clicking a portaled option does not dismiss the dialog: React portals bubble through the React tree, so the dialog's own "pointer down inside" check still sees it (asserted). A select gets `auto` from its own copy either way, since it disables outside pointer events for its own stack; the hover card is the one that was inert. Focus is the half this cannot reach: the dialog's focus trap lives in yet another copy's scope stack, so a nested popover's scope never pauses it and keyboard selection is still lost. Deduping the packages fixes that too — verified by re-running the same probe with both copies mapped to one — but that means unpinning `react-dialog`, which is what #11023 pinned to fix modal tooltip and dropdown regressions. Left for a follow-up that can be validated in a browser.
* fix: separate temporary and general chat retention * fix: inherit retention when branching messages * fix: harden retention lifecycle boundaries * fix: preserve subagent retention context * fix: trust stored chat retention for uploads * perf: overlap generated file retention lookup * fix: derive retention from trusted chat state * fix: preserve retention across the full turn lifecycle * test: accept the conversation retention read adapter * fix: carry retention deadlines through durable job saves * fix: resolve retention at cached and new chat admission * fix: resolve direct tool retention before execution * fix: preserve retention across assistant writes and direct tools
* 🕰️ fix: Settle Parts the Response Has Moved Past Two live treatments outlived what they were reporting on: a reasoning block kept shimmering with its thought peek scrolling, and a question group kept reading "Asking 1 question", both long after the response moved on. **Reasoning.** Activity phases split one response into several bodies, and `PartWithContext` handed `Part` the per-body `isLastPart` as `isLast` — while `showCursor` was correctly the body-level `isLast` AND'd with it. So every settled phase's trailing part claimed to be the live one, and `Reasoning` shimmers on `isSubmitting && isLast`. Both props now carry the same verdict: the part holds the message's cursor. Grouped and parallel-lane paths already AND'd the two, so only the phase path changes; `ToolCallGroup`'s own `isLast` gets the same treatment for consistency. **Questions.** `ask_user_question` writes no output onto its `tool_call` part until the turn finalizes, so `allCompleted` stayed false and the header held the present tense for the rest of the turn. Position cannot settle this one: a LIVE pause appends its interactive card AFTER the group, so the group is not the message's last part exactly while the question is open — the three label tests that failed when I tried it that way are what caught it. The answer itself is the signal, read through the same locally-recorded fallback the durable record card uses, because the streaming handler's message copy can overwrite the optimistic output stamp mid-stream. Both regression tests were verified to fail with their fix reverted. * 🫥 fix: Settle the Group Glyph With Its Label The header's glyph and stacked icons pulsed on their own `!allCompleted && isSubmitting`, so an answered question group would read "Asked 1 question" while still pulsing beside it. Label, glyph and icon animation now read one `isGroupLive` verdict — they are a single control. Found by auditing the rest of the class rather than stopping at the two reported symptoms. The other live treatments in message content are gated on a per-part signal that does settle: `Summary` on its own `summarizing` flag, the interim skill cards on `hasRealContent`, `SubagentCall` on its progress status, and every tool card on `resolveToolCallPhase`, whose whole-message `isSubmitting` fallback only applies where no `runStepStatus` was emitted.
`dev` is red on `Tests: @librechat/client`. The comment I added in #15738 cites the pull request that pinned `react-dialog` as `(#11023)`, and the shared-component guardrail's `/#[0-9a-f]{3,8}\b/i` reads five hex-range digits as a color. Every branch built on dev inherits the failure. Fixed on both sides: the comment cites the number without the `#`, and the pattern now matches only CSS-legal hex lengths (3, 4, 6, 8), so no issue reference can trip it again. The guardrail keeps its teeth — the same test now asserts it still matches `#fff`, `#ff0000` and `#aabbccdd`, and leaves "closes #15738" alone. Also relaxes one assertion in `popoverLayering.spec.tsx`: it pinned the ABSENCE of an inline `pointer-events` on a hover card outside any dialog, which is Radix's to set, not the hook's. Whether a sibling select's `disableOutsidePointerEvents` reaches that card depends on whether the two share a copy of `react-dismissable-layer` — a packaging fact that changes under #15742. The test now asserts only what the hook itself contributes.
* 🎯 fix: Keep Scroll-to-Bottom Clear of Composer Panels The `ask_user_question` popover and the tool-approval review float up from the composer (`bottom-28`, measured from the composer's bottom) while the scroll-to-bottom control floats up from the thread's edge (20px above the composer's top), so the two meet on the panel's footer whenever the composer is taller than 92px — it is 96px empty. Each panel now registers itself as an open composer overlay for exactly as long as it renders, and the control stands down while any is open. The in-flight steer stack keeps its existing lift: it publishes its height and stays short enough for that to read well. The overlap also exposed a stagger that was there all along: the control sat flush with the column edge at 40px, while every composer control is 36px on the `theme-control` tokens and Send is inset `mr-2`. It now shares Send's inset and geometry, so the two read as one rail rather than two edges. `ScrollButton` moves out of `MessagesView` into its own module so the gate can be exercised against a stubbed IntersectionObserver. * 🎯 refactor: Keep the Scroll Control on the Shared Button and Host Surface Codex round 1. The control goes back on the shared `Button`, which gains the composer row's geometry as `size="icon-theme"` (`size-theme-control p-0`) and `shape="round"` (`rounded-theme-control-round`) instead of a feature- local class list. The overlay count and its registration hook move out of the global store into `Chat/Input/overlay.ts`, since only chat panels write it and only the chat thread reads it. The extracted `ScrollButton` and `ScrollToBottom` no longer read `showScrollButton` / `maximizeChatSpace` from the app store: `MessagesView` takes both from the host's `ChatSurface` and passes them down, and the subagent activity host does the same for the column width.
Social providers that do not return a profile picture cause sign-in to fail when a non-local file strategy is configured. `handleExistingUser` and `createSocialUser` pass the incoming `avatarUrl` straight to `resizeAvatar`, which throws "Invalid input type. Expected URL, Buffer, or File." on null. `socialLogin` catches that error and forwards it to the Passport callback, so authentication fails instead of the avatar simply being skipped. Apple is affected by definition: `appleStrategy.js` sets `avatarUrl: null` with the comment "Apple does not provide an avatar URL". Any Google or OIDC account without a picture reaches the same path. Guard the three call sites so that a missing avatar skips avatar processing and leaves the rest of the sign-in flow untouched. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* 🧬 fix: Collapse Radix Layer Packages to One Copy Radix coordinates nested layers and focus scopes through module-level state, so a dialog and a popover inside it cooperate only while they import the same copy of `react-dismissable-layer` / `react-focus-scope`. This tree had four and three of them: dialog -> react-dialog/node_modules/… 1.0.2 (exact pin) select -> react-select/node_modules/… 1.1.10 menu -> react-menu/node_modules/… 1.1.0 hover -> hoisted 1.0.5 Split that way, a popover never learns the dialog disabled body pointer events and the dialog's focus trap is never paused, so in-dialog selects and hover cards are inert and keyboard-dead (#15738). The exact `1.0.2` pin on `react-dialog` and `react-alert-dialog` came from #11023, which downgraded them from ^1.1.15 to fix modal tooltip and dropdown regressions — the same split seen from the other side, when the dialog was the new package and everything else was old. Unpinning both and refreshing the other Radix packages inside their existing carets collapses the tree to one copy of each. No `overrides` entry: the resolution stands on its own, and pinning transitive versions would only hide the next split. `radixLayers.spec.ts` reads the lockfile and fails if either package is ever nested again; `dialogPopoverFocus.spec.tsx` asserts the behaviour that invariant exists for — a portaled option holding focus inside a dialog, which fails on the old tree and passes on this one. Verified with a real install in a worktree: one copy of each resolved from dialog, select and hover-card alike; `packages/client` 46 suites / 476 tests and `client` 323 suites / 3201 tests green; both workspaces typecheck. Needs a manual pass over the app's modals before merge — what #11023 was guarding cannot be proven by jsdom. * 🍞 fix: Give Escape Back to the Dialog Under a Toast Radix routes Escape to the highest dismissable layer only, and `Toast.Root` registers one — mounted after whatever dialog is already open. Splitting `react-dismissable-layer` into per-package copies hid this: toasts and dialogs were never in the same stack. Collapsing them to one copy puts a status toast above every open dialog, and it eats the Escape meant for the dialog beneath. Measured in the running app (mock e2e, instrumented): the dialog's own `onEscapeKeyDown` was never called, the layer above it was `<li class="toast-root" data-radix-collection-item>`, and a SECOND Escape closed the dialog. That is `agent-skills.spec.ts:709`, which failed deterministically here across three attempts. The frontmost open dialog now takes Escape back while a toast is present, closing through a hidden `Dialog.Close` so controlled and uncontrolled dialogs behave alike. Scoped tightly: no toast on screen leaves Radix's arbitration untouched, and only the frontmost dialog acts, so an inner dialog still closes alone. The toast is matched whatever its `data-state`. A toast that has begun closing keeps its layer registered until the exit animation ends, and that is exactly the case that failed — the first attempt keyed on `[data-state="open"]` and missed it, which the instrumentation caught (`FB toast= null allLis= 1`). `agent-skills.spec.ts` now passes locally on the deduped tree: 4 passed. Regression test verified to fail with the fix reverted. * 🚪 fix: Close the MCP Menu With the Dialog It Opened Ariakit takes Escape for a menu only when the event target is the menu, its trigger, or `body`: if (event.key !== "Escape") return; if (event.defaultPrevented) return; ... if (isElement(target) && target.tagName === "BODY") return true; if (contains(dialog, target)) return true; if (contains(disclosureElement, target)) return true; return false; // ariakit dialog.tsx:496-517 While the MCP config dialog holds focus none of those is true, so the Escape that closes the dialog leaves the menu open behind it — and `disabled={isOpen}` on the trigger then makes it unclickable. The reader is stranded with a menu they cannot reach and a button they cannot press; `mcp-oauth-readiness` times out clicking it, three tests over. Keyed on the dialog CLOSING, not opening. The first attempt closed the menu as the dialog opened and broke a fourth test that reads the server rows from the still-mounted menu underneath (`unmountOnHide` deletes them): `element(s) not found`. Closing on the way back out leaves that untouched. Local mock e2e on the deduped tree: `mcp-oauth-readiness` 5 passed (was 2/5), `agent-skills` + `mcp` 5 passed, `client` 3204 tests green, typecheck clean. * 🛡️ fix: Keep the Escape Fallback Inside Its Lane Codex found three, all real. **P1 — the fallback overruled the guards it should obey.** Any Escape while a toast was up clicked the dialog's close button, including when a select inside the dialog owned that Escape (WCAG 2.1.1) or when another layer had already answered it. A reader closing a listbox would have lost the whole dialog and anything unsaved in it. It now bails on `event.defaultPrevented` and on the same popup conditions the Radix-provided handler honours — extracted into one `escapeBelongsToPopup` helper both call, so they cannot drift apart. **P2 — the frontmost check read inline z-index only.** `ImagePreview` is a raw `DialogPrimitive.Content` carrying `z-[250]` as a CLASS, so it scored zero and the My Files dialog underneath was mistaken for the frontmost — Escape in the preview would have closed it too. Ranking now reads the resolved `getComputedStyle().zIndex`. **P2 — the published contract did not carry the singleton.** `@librechat/client` still admitted `react-toast@1.1.5` and `react-popover@1.0.7`, which depend on the older dismissable-layer/focus-scope line, so a consumer could reassemble the very split this PR removes. Radix minimums raised to the versions the lockfile resolves; the tree still holds exactly one copy of each. Three regression tests, each verified to fail against the pre-fix component. `packages/client` 484 tests, `client` 3204 tests, both typecheck clean, local mock e2e `agent-skills` + `mcp-oauth-readiness` 9 passed. * 🎚️ fix: Let the Consumer and the Alert Dialog Have Their Say Two more from codex, both real. The fallback never ran the dialog's own `onEscapeKeyDown`. Radix calls that only for the highest layer, which the toast is — so a dialog whose consumer cancels Escape (`event.preventDefault()`) closed anyway, taking whatever was in it. The fallback now invokes the supplied handler and honours the cancellation, exactly as the Radix path does. The frontmost ranking looked only for `role="dialog"`. `OGDialogContent` is also used as an alert dialog — the shared-link delete confirmation (`SharedLinkButton.tsx:426`) — so its content was never found, `frontmost` stayed null, and the fallback declined to act: that confirmation still needed two Escapes. Alert dialogs are ranked too. Two more regression tests, both verified to fail against the previous version. `packages/client` 486 tests, `client` 3204 tests, typecheck and static checks clean.
…ialog (#15751) Two findings from the last codex round on #15742, which merged before these landed. Both are real; the third report was the same ref finding filed three times. The ref wrapper `OGDialogContent` gained there swallows a callback ref's return value. Under the React 19 peer range a consumer's teardown never runs when the node is replaced, stranding whatever it had attached — the direct forwarding it replaced had no such gap. The cleanup is passed on now, and only when it really is a function, because React 18 warns about any other return value. The Escape fallback also called the consumer's `onEscapeKeyDown` before establishing that this dialog was the one the Escape belonged to. Every mounted dialog runs that listener, so a dialog underneath fired its handler for someone else's keystroke — and a `preventDefault()` there would have stopped the frontmost dialog from closing at all, since the lower listener registers first. The call now happens after the frontmost check. The gating has a regression test, verified to fail without the fix. The ref cleanup does not: React 18 ignores the returned function outright, so the repo's own test runtime cannot observe it. `packages/client` 488 tests, typecheck and static checks clean.
* fix: Load user keys without expiry metadata * fix: Avoid unused Azure user credential lookup
* 📐 docs: Ask Pull Requests to Show the Mechanism The template asked for a brief summary and a test process, so descriptions land as a list of changed behavior with no view of how the change works. Reviewers then rebuild the call order from the diff. Summary now asks for the trigger and the resulting behavior. A new optional How it works section offers four views — a focused diff, a call tree, a shallow file tree, and a Mermaid sequence — with the instruction to pick one or two and delete the section when the summary already covers it. All guidance rides in HTML comments, so an unfilled template renders exactly as it does today. CLAUDE.md and AGENTS.md carry the same rule for agent-authored descriptions. Adapted from HumanLayer's show-me skill and an internal ClickHouse PR-template proposal. * 🧭 docs: Add Review and Completion Standards Both files describe how to write a change and how to test it, but not how a pull request gets from opened to done. That gap is where review rounds stall: a finding gets patched, the next review runs against an older head, and a working code path ships without its empty, failure or restored-session behavior. The new Review and Completion section states the review-cycle invariants — inline threads are the source of truth, findings are judged against the code, a review counts only for the commit it ran on, and repeated findings mean the subsystem needs a sweep — followed by a definition of done covering the observable experience, compatibility, and honest reporting of what was actually run. The reviewer and its trigger phrase are named as the part expected to change, so that subsection can be rewritten without touching the invariants around it. Nothing here restates the existing Testing, Typechecking or Frontend rules; it points at them. * 🧹 docs: Drop the Shipped Insights Design Note `docs/agent-insights-access-design.md` was the pre-implementation spec for #15549, and it says so: "design reference for work based on upstream/dev after PR #14898". Everything it specifies shipped — PermissionBits.VIEW_INSIGHTS = 16, /api/insights, the server-owned initial_agent_id — so the code and its tests are the source of truth now and the note can only drift from them. Nothing in the repository links to it. docs/skills-management-api.md stays: it describes a machine API surface that has no equivalent on librechat.ai, and its details still match packages/api/src/skills/management.ts.
…ndencies (#15754) * 🧶 docs: Keep the Breadcrumb Carve-Out in AGENTS.md The template and CLAUDE.md both say that naming the merged pull request which caused a bug is history the reader needs, not a breadcrumb. The condensed AGENTS.md version dropped that half, leaving a flat rule against referencing earlier work, which is the file most agents actually read. * 🧱 docs: Hold the Database Boundary and Make New Levers Configurable Workspace Boundaries said where database logic lives but not what may cross the line, so Mongoose types travel outward in exported signatures and make the storage engine part of each module's public API. That is the tax a second engine would pay, and it is cheaper to stop widening than to unwind: packages/api already imports mongoose in dozens of non-test files, while client carries none. Nothing in either file mentioned configSchema or librechat.yaml either, so a new limit or toggle lands as a constant by default and an operator cannot reach it. Both rules now sit in Workspace Boundaries, with the configurability half cross-referenced from the definition of done. Condensed into AGENTS.md as Module boundaries and configuration. * 🔌 docs: Take Dependencies, Do Not Reach for Them Client State Ownership already tells a frontend feature to accept app-global state through props rather than reaching into the store, and gives the reason: it is what lets the feature move to its own workspace later without a rewrite. The backend had no equivalent, and neither file mentioned dependency injection at all. Backend modules now carry the same rule one layer down, with createModels(mongoose) as the shape to copy and the static singletons under packages/api/src/mcp as the shape to stop extending. Integrations get it explicitly: an injected provider SDK, storage backend, vector store or OAuth server makes a second implementation a new argument instead of a new branch, and lets a test substitute at the boundary rather than mocking the module that holds it.
* fix: Settle Background Tool Polls Reliably * fix: recover interrupted background result delivery * fix: preserve independent background fallbacks * fix: align background claim types * chore: sort background task imports * fix: preserve claimed result narrowing
Both migration rules described an end state and left the trigger implicit, which is why they get skipped. "Keep /api changes to the absolute minimum" reads as "make a small edit here", the opposite of what it means, and it is phrased for new code while the common case is editing an existing CJS file. It now says /api holds wiring and not behavior, that minimum describes how much behavior /api gains rather than diff size, and that lifting the function out is the larger and correct diff. MCPRequestContext.js is named as the shape. "Convert the areas you touch" never defined an area, and never said an atom cannot be half converted, so mixed Recoil and Jotai imports in one file read as permission. The rule now states that new state is always Jotai, that the unit of conversion is one atom plus every reader and writer, and that an atom with a consumer outside the feature stays on Recoil and gets passed in. jotai-utils.ts is named for persisted atoms.
* fix: Skip test modules during structured tool discovery * fix: Keep tool discovery validation in TypeScript
* fix: expose tenant index upgrade migration * fix: retry tenant migration index builds * docs: run tenant migration from the image root
Covers the one leg the credential-free mock suite cannot fake: a file attached through the UI's Code Environment target must land in the live Code API, get delivered into the sandbox, and stay readable across turns. - fake-model: E2E_EXEC_UPLOADED:/E2E_EXEC_PERSIST: markers emit real bash_tool calls through the production tool pipeline (the sandbox surface stateful agents register; execute_code stays host-wired and never appears in the tool registry), plus an env-gated dump of each run's advertised tools for future spec authoring - spec: attach uploads a CSV to the Code API, turn 1 reads it back from /mnt/data and drops a proof file no upload contained, turn 2 (sent without an attachment) reads both — only possible when the run reuses the same stateful runtime session - e2e config: agents capabilities gain stateful_code_sessions, and the mock profile pins CHECK_BALANCE=false so a developer's local .env can never fail every send with zero-balance users - skips unless LIBRECHAT_CODE_BASEURL is provided (kept out of CI)
* fix: stop refreshing MCP caches on every chat message * fix: reconcile MCP caches independently of chat turns * fix: scope MCP polling to visible controls * fix: refresh status in the unpinned MCP submenu
* fix: Patch gRPC xDS DoS dependency * fix: Tidy patched gRPC module graph
…15770) * chore: bump agents sdk to v3.8.5 * chore: bump nodemailer to v10.0.1 * chore: bump sharp to v0.35.4 * chore: bump js-yaml to v4.3.2 * chore: bump hono to v4.13.7 and @hono/node-server to v2.1.1
* feat: Select Attached Code Workspaces * fix: Preserve Workspace Boundaries Across Agent Graphs * fix: Validate Lazy Agent Workspace Bindings Before Persistence * fix: Include Handoff Environments in Workspace Selection * fix: Close Workspace Admission and Remote Ingress Gaps * fix: Preserve Workspace Selection In Chat Runtime Envelopes * fix: Authorize Deployment Workspace Status And Match Readiness * fix: Preserve Ephemeral Chats And Pin Runtime Worker Identity * test: Complete Deployment Pairing Fixtures
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )