Conversation
* feat: Sort Modes and Mine Filter for the Agent Marketplace List
Adds two query parameters to `GET /api/agents`:
- `sort` — one of newest | oldest | popular | author, validated against
an allowlist since the value reaches a `$sort`/`$lookup` pipeline.
- `mine=1` — narrows the list to agents authored by the caller, applied
on top of the ACL-resolved accessible set rather than instead of it.
`newest`/`oldest` remain a plain `.find()` but now order on `createdAt`
instead of `updatedAt`. This endpoint refreshes S3 avatars through
`updateAgent`, which advances `updatedAt` — the very field the cursor
compared against — so an agent could cross a page boundary mid-scroll
and drop out of the caller's flattened list. Two indexes back the new
ordering.
`popular` counts favorites through an equality `$lookup` and validates
tenancy after the join. `author` resolves the same contact the card
renders — support_contact, then the user holding the OWNER ACL grant —
so the ordering matches what the reader actually sees. Both compute
their key inside the pipeline, so the cursor compares against the
computed field and the key is stripped before the rows are returned;
the response shape is unchanged.
Behaviour note: a request that sends no `sort` now comes back ordered by
`createdAt` descending rather than `updatedAt` descending.
* feat: Denser Agent Marketplace Grid with Sort and Mine Controls
Rebuilds the marketplace chrome around a denser grid. The hero block
gives way to a sticky header carrying the title, a live agent count and
a compact search field; category tabs become pills; and the grid follows
the available width instead of a fixed two-column layout, so a wide
window shows more than two agents per row.
Adds two controls to the category header, next to the category they act
on: a sort dropdown and an "only agents I created" switch. Both mirror
into the URL (`?sort=`, `?mine=1`) so a filtered view can be linked and
restored, and the default sort is omitted from the URL to keep it clean.
Switching the filter on inside Top Picks moves to All, where authored
agents can actually appear, since nothing writes `is_promoted` yet.
Since `popular` recomputes its count per request, pinning or unpinning
invalidates only the popular pages, and marks them stale rather than
refetching under a reader who is mid-scroll.
* feat: Agent Marketplace Refresh with Shared-Layout Detail Morph
Clicking an agent card now expands that card into the details dialog instead
of fading an unrelated panel over the grid, and the dialog contracts back into
the card it came from.
The morph is a framer-motion shared-layout handoff with `layoutCrossfade`
disabled, so the projection hides the relegated card nodes itself: the card
keeps its box, the grid never reflows, and no field is ever drawn twice. Card
and dialog keep genuinely different layouts and only the fields they share
travel — surface, avatar, name, category pill, creator row and the blurb.
The pill moves from the card's upper right to the dialog's upper left, the
avatar grows, the name zooms between the two type sizes, the creator moves out
of the footer to under the name, and the blurb is handed over at the dialog's
width so the surface's clip reveals the lines the card had no room for. The
dialog's own controls — close, "About this agent", Pin, Copy Link, Start Chat —
are introduced after the geometry is under way and leave the way they came in.
The dialog's content lives inside the morphing surface, whose sections are
projection nodes, so nothing stretches or spills outside the expanding box.
`layoutDependency` pins measurement to selection changes, which keeps the
virtualized grid's per-scroll rerenders from measuring every card. The page dim
is owned by the grid rather than the dialog's overlay so it can run its full
range across a contraction that outlives the dialog's mount, and the source
card is raised above its neighbours until the surface is home.
`OGDialogContent` gains `bare` and `forceMount` so a caller can own a dialog's
geometry and animation while Radix keeps focus trapping, Escape, the scroll
lock and the aria wiring. Reduced motion skips the morph and opens the dialog
outright; focus still returns to the card that opened it.
Also in the marketplace header: the admin settings trigger takes the bordered
`outline` variant so it matches the search field beside it, and the search
field's clear button declares its own colour instead of inheriting the
document's, which made the glyph invisible in dark mode.
* style: Square Off the Search Clear Button's Hover Fill
Its hover fill inherited the button recipe's `rounded-lg`, which on a 32px
square read as a pill inside the search field; `rounded-md` keeps the chip
subordinate to the field's own 8px corner.
* fix: Adjudicate Review Findings on the Marketplace Refresh
Four demonstrated defects from the review round on a10b31c0:
Surface radius came from a literal 16px repeated at both morph endpoints, so
`--theme-surface-radius` could not move the marketplace's corners even though
every other surface follows it. The card and dialog surfaces now carry
`rounded-theme-surface`, and the projection — which can only hold a corner
steady while it scales if it owns the radius as a number — is fed the resolved
token, read once per morph rather than per card or per frame.
The morph's page dim restated `bg-black/80` in feature code, duplicating an
appearance decision that belongs to the dialog primitive. `OriginalDialog` now
exports `DIALOG_SCRIM_CLASS` and both the Radix overlay and the grid's animated
backdrop consume it, so the scrim has one owner again. The scrim's colour is
still a literal in the primitive: making it a theme role changes every dialog
in the app and is not this PR's to decide.
`ErrorDisplay` narrowed on `Error` and kept only `message`, but an `AxiosError`
*is* an `Error`, so `response.status`, `response.data` and `code` — the fields
the classification runs on — were discarded for every real request failure. A
500 was therefore labelled non-transient `generic` with automatic recovery
disabled, and 404s rendered as errors instead of the neutral empty state. Only
a bare string needs wrapping now. Its regression test delivers the failure the
way axios does, as an `Error` instance, which the previous object-literal
fixtures never exercised.
`useAutoRetry.retryNow` did not cancel the pending backoff: resetting `attempt`
to 0 while it was already 0 is a no-op, so the effect never re-ran, its timer
stayed armed, and the scheduled attempt still fired after a manual or
connectivity-driven retry had gone out — a duplicate request that also spent a
backoff step. The schedule is now a value that changes on every retry, so each
one cancels and restarts it.
* fix: Keep a Morphed Card's Corner on the Radius Token
The resolved radius was handed to the surfaces only for the duration of a
morph. The projection keeps a `borderRadius` once it owns it, so dropping the
value on the way out left a morphed card pinned to whatever the theme was when
it was opened, and a later theme change moved every corner in the grid except
that one. The list resolves the token once for all its cards instead and
re-resolves it when the theme rewrites the root.
* perf: Page the Marketplace's Popular and Author Sorts in the Database
Two demonstrated defects from the review round on a10b31c0.
`sort=popular` loaded every accessible agent into application memory on every
page, queried favorites for that complete id set, sorted the whole array in
JavaScript and then kept the requested slice, so a 32-item page scaled with the
whole marketplace and the next cursor repeated all of it. The ordering is now
split at the only place it can be: agents somebody favorited, which is at most
as large as the favorites themselves, and the count-0 tail, which is ordered by
`_id` alone and therefore pages as an indexed range scan with the limit applied
by the database. The favorite counts no longer carry a marketplace-sized `$in`,
and the count key is per tenant, so a missing and an explicitly null `tenantId`
are one tenant rather than two.
Measured on 300 accessible agents, 4 of them favorited, `limit: 10`: page one
returned 314 documents to the process before and 19 after, page two the same
314 before and 19 after. Walking the whole list at `limit: 7` produces the
identical id order in both implementations.
The `author` pipeline joined the owner ACL entry and the owner user in order to
sort by the resolved display name, projected neither, and left
`getListAgentsHandler` to await `attachOwnerContacts`, which queried the same
ACL entries and the same users again for the page it had just ordered. The
pipeline now resolves `owner_contact` on exactly the tiers
`resolveAgentOwnerContact` applies — a support contact means no owner contact,
and without a joined owner account there is none either, even with a
denormalized `authorName` — and marks the row. `attachOwnerContacts` skips
marked rows, strips the marker, and so performs no query at all for an
author-sorted page.
`$$REMOVE` is rejected by DocumentDB, so "no contact" arrives as an explicit
null the list method drops before the row leaves.
The dropped popular-sort test asserted that a row deleted between the candidate
scan and the full fetch still yielded a usable cursor; it drove that state by
mocking `Agent.find` for one call, which the single ordered segment no longer
performs.
* fix: Keep Throttled Loads Recovering and the Skeleton on the Radius Token
Two findings from the review round on cdb91f4.
The marketplace query deliberately retries a 408 and a 429 twice
(`data-provider/Agents/queries.ts`) and leaves the longer backoff to the error
card, but the card classified both as `generic`, where `autoRetry` is false —
so a throttled or timed-out load stopped recovering after those two attempts
even though every layer above had called it retryable. A 408 now takes the
`timeout` copy that already describes it, and a 429 gets its own transient
kind rather than borrowing "Server Error" for a status the server answered
deliberately.
`GridSkeleton`'s placeholder card kept `rounded-2xl` while the real card moved
to `rounded-theme-surface`, so under any theme that sets a different surface
radius the corners jumped the moment the skeleton was replaced.
* fix: Read the Author Sort's Owner From Agent ACL Entries
The author sort joined every ACL entry sharing the agent's _id and picked the earliest full-bit user entry, so a REMOTE_AGENT owner entry left behind by an ownership transfer decided both the sort key and owner_contact. attachOwnerContacts counts only AGENT entries, so an author-sorted row could name a different owner than the same agent's card in any other sort mode. The filter now asserts the resource type.
* chore: Drop the Marketplace Strings the Refresh Orphaned
The rewritten grid, card and error card no longer render the old detail panel's copy, so com_agents_description_card, com_agents_error_loading, com_agents_error_searching, com_agents_results_for and com_agents_see_more had no reader left and the unused-i18n-keys static check failed. Only the English file is edited; the other locales are automated.
* fix: Recover on Window Focus and Move the Marketplace Query Contract Into packages/api
Three findings from the review at 9f59cad8.\n\nThe list query's sort allowlist and mine normalization were behavior in a CJS controller, which api/ is not supposed to hold; resolveMarketplaceListQuery now owns the contract in packages/api beside AgentSortOption, and v1.js keeps the call.\n\nuseAutoRetry promised recovery when the view is looked at again but only listened for visibilitychange, so a window that lost and regained focus with the tab still visible never recovered - terminal once the backoff was spent. It now also tracks focus/blur, counted once per activation so a tab switch that fires both events retries once.\n\nThe aggregation predicate helpers returned Record<string, unknown>, which the repository prohibits for new code and which accepts any misspelled operator; they return a concrete expression union instead.
* fix: Move Owner-Contact Resolution Out of the CJS Service and Open Mine Links on All
`api/server/services/Agents/ownerContact.js` had grown the resolved-row filtering, the
marker cleanup and the owner ACL pipeline, all behaviour, inside `/api`. The service is
now the wiring the boundary asks for: `attachAgentOwnerContacts`
(`packages/api/src/agents/ownerContact.ts`) takes its database reads from the caller, and
the owner query itself is a data-schemas method, `getFirstOwnerIdsByResource`, beside the
`OWNER_ACL_PERMISSION_BITS` the marketplace's author sort already resolves against. The
aggregation casts stringified ids, because `$match` does no schema casting and a caller
outside data-schemas has no Mongoose ObjectId to hand it.
A restored or shared `/agents?mine=1` carries no path category, so it resolved to
`promoted` and answered the caller's own agents with the near-always-empty
promoted-and-mine intersection; turning the same filter on from Top Picks navigates to
`/agents/all`. It now defaults to `all` whenever the filter is on, so both routes to the
intent agree.
The conversation-starter buttons dropped their hard-coded `rounded-xl`, so they take the
shared `Button`'s shape like the dialog's other controls instead of mixing two.
`e2e/specs/mock/scenarios/` carries one tagged Playwright test per acceptance scenario:
sorting and paging, the mine filter and its link, failure recovery, and the card-to-dialog
morph, its focus return, its single scrim and its radius.
* feat: Morph the Agent Description Word by Word and Retime the Whole Transition
The card clamps the description to three lines and the dialog shows all of it,
so handing the paragraph over as one block slid the card's wrapping into place
and then swapped it for the dialog's. Every word the card showed now travels
from where that paragraph wrapped it to where this one does, and the lines the
clamp hid fade in where they land. The words are written to directly through CSS
transitions rather than re-rendered: a description is hundreds of independently
positioned spans, the compositor can carry them without a frame of JavaScript
each, and a transition can be interrupted mid-flight where a keyframe animation
restarts. Assistive technology still reads the copy once, from a flat string;
the spans are decoration and are hidden from it.
The identity block moved into the scroll region, the contact took the old
Pin/Copy Link corner, and those two controls moved next to Start Chat. Title and
description centre on the avatar while the copy is shorter than it, and top-align
once it outgrows it - pure CSS, no measurement.
Timings collapse onto one clock. A single ease-out-cubic and one open budget
drive the surface, the page dim, the identity fields and the words, so nothing
reads as a second animation that happens to have been started alongside the
first:
open 300ms surface, identity fields, dim
words 210ms (0.7 of the open: a word crosses a fraction of the box's
distance, and sliding copy cannot be read)
close 280ms 70ms handover, then a 210ms contraction
word return 70ms the whole handover; nothing else is moving on the way back
The close is two phases because the surface cannot contract until this dialog
unmounts and gives it back. Only the handover is spent with the box still, so it
stays short, and the dim now leaves on the click rather than on the unmount -
that is what answers the click while the geometry is still waiting. The dim
ramps linearly across each direction: an ease-out on an alpha spends two thirds
of its range in the first third of its time, and a scrim at 80% black already
looks black at half of it, so the page read as fully dim while the box was still
growing.
The words move and only move - no blur, no per-word delay, no scaling. A word
broken across two lines has no single box to fly from, so it is left to the
fade, and a paragraph showing different copy than the card (a background refresh
can land one) is refused outright rather than flying every word in from the
wrong place. Reopening inside the handover turns the words around instead of
leaving them parked in the card's wrapping.
* fix: Keep the Loaded Rows Mounted While the Marketplace Retries
A failure or an empty result was rendered in place of the list. With pages
already loaded that collapsed the list's scrollable height, so the browser
clamped the scroll position to the shorter document, and the remount that
followed a recovery ran the layout effect again and started the list at the
top: a transient cursor-page failure sent someone browsing deep in the
marketplace back to the first row.
The rows now stay mounted and the placeholder follows them, which also means
the end of the list stays reachable while a failure is held — so paging is
suspended until it clears, leaving the retry and its backoff to the error card
instead of re-requesting the page that just failed on the next scroll.
* test: State the Morph and Retry Scenarios as What the Reader Sees
Two scenario assertions described the implementation rather than the behaviour.
The detail-dialog description is now painted per word beside one screen-reader
copy of the same text, so matching the copy exactly resolved to both nodes. The
scenario asserts the paragraph that carries the description instead of counting
the nodes that spell it out.
The retry scenario pinned the scroll position to the pixel, which moves by
whatever the card rendered below the list occupies. It now anchors on the row
the reader had reached still being on screen, with the position staying deep
rather than clamped back to the first row.
* fix: Show the Retry Instead of a Skeleton, and Keep Cursors Readable
Two failures a marketplace reader could hit.
A held failure has no data of its own, so the retry that clears it made the
list pending again and filled the viewport with skeleton cards above the error
card. The card's status, countdown and action ended up below the fold, so the
recovery the card was reporting was the one thing not on screen. The skeleton
is now suppressed while a failure is held: the card owns the waiting state for
the request it describes.
The sort cursor carries `primary` and `secondary`, which an instance predating
sort modes cannot read - it reads `updatedAt` and `_id`, builds an Invalid Date
from their absence, and the query Mongo rejects surfaces as a 500 in the middle
of someone's scroll during a rolling deployment. Cursors now carry that pair as
well, describing the same row, so an older instance continues its own
updated-time ordering from where the page ended. The popular mode built its
cursor from a synthetic row, so it now carries the projected document instead.
* fix: Keep a Cursor Readable Only by a Server That Implements Its Order
Four review findings, and one rule behind three of them: state belongs to
whoever owns it, and a cursor belongs to the ordering it describes.
A sort cursor carried a legacy `updatedAt`/`_id` pair unconditionally, so an
older instance reached during a rolling deployment could parse a cursor from
`oldest`, `popular` or `author` and resume its own `updatedAt` walk from a
boundary row that ordering never produced - skipping agents and repeating
others. The pair is now emitted only for `recent`, whose `updatedAt` descending
plus `_id` ascending order is exactly what the legacy decoder implements. Every
other sort omits it and an old instance rejects the cursor outright, which is
the same argument the surrounding comment already made about not inventing a
date.
Marketplace avatars were refreshed from a separate newest-sorted query capped at
1,000 rows, run before the page the caller actually asked for. A user with more
accessible S3-backed agents than that cap saw broken avatars on every
`oldest`, `popular` or `author` page, and the 30-minute cache suppressed the
next attempt. The requested page is now queried first and its own uncovered
S3-backed rows are refreshed; the cache records which ids it covered, so an
entry written for one page no longer stands in for rows it never touched. The
selection, the cap and the cache merge live in `packages/api/src/agents`, with
the controller reduced to the call.
The detail dialog reached into `~/store` to clear the conversations a
multi-conversation session left open. That state is the app's, not the
marketplace's, so `MarketplaceProvider` performs the reset and the dialog asks
for it through the host context; the hook throws rather than defaulting to a
no-op, because a silent miss is the bug it exists to prevent.
With rows kept mounted, a failed background or cursor page put its retry card at
the end of a list several viewports tall - a marketplace that had quietly
stopped loading looked healthy. The card is now pinned to the bottom of the
scroll frame while rows exist, so its status, countdown, retry and reload link
are on screen wherever the reader is; with no rows there is nothing to pin it
over and it remains the whole of the page.
Two findings are rejected rather than patched. Popularity pagination recomputes
counts per request, so an agent that gains favourites mid-walk can cross above
the cursor and be missed; the walk is stable within a count, and backward drift
is deduplicated by the client. Making it exact requires a ranking snapshot, and
the favourites subdocument carries no per-favourite timestamp to snapshot from
(`packages/data-schemas/src/schema/user.ts:163-171`), so the fix is the
materialized counter tracked in berry-13/LibreChat#11; the guarantee is now
stated in the method instead of implied. Author sorting computes its key from
the ACL and user joins, so its cursor predicate cannot move ahead of them
without a stored sortable owner key - a schema change with a backfill, not a
pipeline reordering.
* docs: Track the Popularity Walk's Skipped Row as Its Own Issue\n\nThe method pointed only at the materialised-counter issue, which tracks the cost\nof recounting favourites per page. The row an upward drift drops from the rest of\nthe walk is a correctness problem that survives whether or not the cost is ever\naddressed, so it is tracked separately and the comment cites both: what can be\nmissed, and what pinning a ranking would take.
* fix: Let a Cursor Be Read by the Version That Implements Its Order
The previous round gated cursor *encoding* so only `recent` emits the legacy
`updatedAt`/`_id` pair, and left decoding rejecting it. That breaks the opposite
rollout direction: a page served by an older instance, followed by a request
reaching this one, produced an unreadable cursor, so the walk restarted at page
one - and `fetchAllAgentPages` concatenates without cycle detection, so
alternating instances can loop without finishing. Decoding now accepts the
legacy pair for `recent`, whose ordering this version implements, and still
rejects it for the sorts whose ordering it does not. The invariant is the same
one, read from the other side.
Refreshing a presigned avatar URL went through a timestamped update, so
maintenance advanced `updatedAt` - the sort key of the default walk - and an
agent below an active cursor could cross above it and vanish from the rest of
that walk. Avatar writes now go through `updateAgentAvatar`, which sets the
avatar alone with timestamps disabled.
The avatar orchestration the last round left in the controller - the
page-coverage cache decision, the refresh failure policy, the cache merge and
the persistence call - moved to `packages/api/src/agents/avatars.ts` with the
cache, the refresh and the update injected. The CJS controller holds the wiring
and one call.
On the client, the retry card was pinned to the bottom of the scroll frame and
rendered after the whole list, so it was visible but last in keyboard order:
`handleKeyDown` mounts and focuses each next card, so reaching Retry meant
traversing every loaded agent. It now renders before the rows and pins to the
top of the frame. A bottom pin cannot survive the move - sticky bottom only
holds an element whose flow position is below the viewport - so the pin
direction follows the DOM order rather than the reverse.
Two more from the same reading. `keepPreviousData` leaves the previous scope's
pages in `data` while a replacement loads or fails, and the failure branch
bypasses the pending guard, so a failed "My agents" request could render its
error over clickable agents from the scope the user had left; those rows are no
longer treated as current. And because a virtual row is keyed by its first
agent, a refetch that reorders the list unmounts the focused card: focus is now
restored to the same agent, and only when the unmount is what dropped it.
* fix: Make a Cursor Name Its Ordering, and Refuse the Ones It Cannot Honor
Third round on the same seam, so this stops patching it. A cursor now names the
ordering it was produced under, and is honored only by a reader that implements
that ordering; one that cannot be honored is an error rather than a silent
restart. `recent` keeps emitting the legacy `updatedAt`/`_id` pair, so old
instances still read its cursors, and a legacy-only payload counts as the
`recent` ordering: usable there, a mismatch everywhere else. Decoding reports
usable, ordering-mismatch or unreadable; `getListAgentsByAccess` raises instead
of paging from the start, and the controller answers 409
`{"error":"cursor_ordering_mismatch"}` through a mapper in `packages/api`.
That closes the direction this round found: a first page served by an old
instance for `sort=newest` produced a cursor this version rejected, and the
rejection fell back to page one, so the infinite query appended a second first
page - duplicated and out of order. The client now treats the 409 as "this walk
came from another ordering", drops the pages it holds and refetches from the
first page in the ordering the user asked for, once per request signature so a
mixed-version deployment cannot loop.
The author sort ordered by raw display name, so `Zoe` came before `alice` in a
list whose whole purpose is alphabetical. It sorts on a `$toLower` key now, and
the cursor carries that same normalized value - otherwise the ordering and the
pagination boundary disagree about where the page ended.
Avatar refresh coverage was a flat id list under one renewing TTL, so a page
refreshed at minute 0 stayed marked covered until minute 59 if another page was
refreshed at minute 29, and the set grew for the length of the walk. Coverage is
now a per-id deadline map: an id is covered only while its own deadline holds,
expired ids are dropped on read and write, the retained set is bounded with
nearest-expiry eviction, and the entry's TTL follows the furthest deadline it
still holds.
On the client, the retry card was pinned but in flow, so it displaced every
loaded row by its own height when a late page failed and put them back on
recovery. It sits in a zero-height sticky host now, keeping the three
constraints that pull against each other: first in keyboard order, visible at
any scroll position, and no layout space. Both of the traps that shape it are
written down - a flex child stretches to its container's cross size, so the
card's background collapses without `items-start`, and the stacking level has
to clear the cards' own click overlay.
An agent removed from the list while its dialog was open kept Pin, Copy Link,
the starters and Start Chat live, so Start Chat could navigate to a deleted
agent and Pin could persist a stale id. Absence from the loaded pages is not
proof of deletion - paging and sorting move rows out of the window - so the
captured id is revalidated against the per-agent endpoint, which enforces
access; only 403 and 404 mark it unavailable, the dialog says so, and its
actions go with it. A transient failure leaves the agent actionable.
* fix: Version an Ordering When Its Comparison Changes
Normalizing the author sort changed what "ordered by creator name" means while
still calling that ordering `author`, so the immediately previous
implementation would accept a cursor from this head and resume under
case-sensitive comparison: page one ending at `alice` let a later page skip
`Zoe`, which compares before the cursor under the old rules. The ordering
identity now carries a version alongside the mode - `author` at 2, everything
else at 1 - matched exactly on decode, with an absent version read as 1 so
cursors already in flight keep working for every mode whose semantics did not
change. The rule is written where the version lives and where it is encoded:
bump it when a comparison changes, or an older reader resumes a walk under
semantics that no longer exist. `popular` and `recent` were audited and left at
1; their comparisons are untouched.
A next-page request answering `cursor_ordering_mismatch` reset the walk
correctly, but the grid had already filed that error as a next-page failure,
and it clears those only when the replacement holds more pages than the prefix
it replaced. A restart yields exactly one page, so the error card stayed up over
a perfectly good first page, and its retry called `fetchNextPage`, which does
nothing when that page has no cursor. The query layer now reports the reset it
performed, and the grid drops the held failure when that reset's page-one
request succeeds. Ordinary failures keep the hold-until-success rule that stops
the card from remounting and resetting its own backoff on every attempt.
The dialog revalidates the selected agent when its row leaves the loaded
window, and that request is authoritative - but the dialog kept rendering the
snapshot it opened with, so a refresh that both edited an agent and moved it out
of the window left a stale description and stale starters on screen, and a
starter could launch with text the owner had replaced. It now prefers the row
the list holds, then the revalidated agent, and keeps the snapshot only for the
case neither can answer.
While that check was in flight the action controls were unmounted, so focus on
Pin, Copy Link, Start Chat or a starter fell to a detached element outside the
modal and was not restored afterwards. They stay mounted and are marked
`aria-disabled` for the duration, which is also why they are not natively
disabled: disabling the focused element blurs it, reaching the same bug by
another route. `CopyLink` takes that state and guards its activation, so the
control keeps focus, announces as disabled, and cannot copy a link to an agent
whose existence is still being checked.
* fix: Put a Bumped Ordering's Boundary Where an Old Reader Cannot Find It
Versioning the ordering identity only protects a reader that knows to look at
the version. An instance deployed before versioning existed ignores unknown
fields and reads `primary`/`secondary` straight out of the payload, so an
author cursor from this version - whose boundary is a lowercased name - was
still resumable under the case-sensitive comparison it replaced: page one
ending at `alice` let that reader skip `Zoe`.
An ordering past version 1 now nests its boundary under `boundary`. A reader
that predates versioning finds no boundary it recognizes and fails closed
instead of walking the wrong comparison, and a reader that knows about versions
knows where to look. Version 1 keeps the flat shape, which is what keeps every
cursor already in flight readable across a deployment, and `recent` keeps its
legacy pair. A flat boundary claiming a version past 1 did not come from this
encoder, so it is unreadable rather than trusted.
Verified against the running API: an author page emits
`{sort, version: 2, boundary: {primary, secondary}}` with no top-level boundary
fields, the next page continues it with no overlap, and both a flattened
boundary and a version-1 author cursor answer 409 `cursor_ordering_mismatch`.
* fix: Let an Author Cursor Carry the Key the Database Ordered By
The author sort orders on `authorSortKey`, the `$toLower` of the display name,
and the row handed to the encoder already carries that key. The encoder
lowercased it a second time in JavaScript, which normalizes what the database
does not: `$toLower` is defined over ASCII and leaves `Émile` (U+00C9)
uppercase, so the boundary was written as `émile` (U+00E9) and the next page's
`$match` skipped every key in between - `Üwe` (U+00DC) disappeared from the
walk entirely, with `has_more` false and an empty page to show for it.
The computed key is now serialized as it stands. The ordering's comparison is
untouched - `$sort` still reads the same `$toLower` key - so the version stays
at 2: a cursor already in flight from the previous head decodes to the same
boundary it did before and resumes the same walk, rather than being refused for
an ordering change that did not happen.
Reproduced and verified in `agent.spec.ts`: two agents whose contact names are
`Émile` and `Üwe`, paged one at a time, lost the second row before this change
and both pages now return their row; 231 tests in that suite pass.
* fix: Give the Recovery Card Its Own Band Instead of the Rows' Space
The retry card was a zero-height sticky host painted over the list, so whatever
it covered was still focusable with nothing to show for it: a focus ring under
an opaque card is no focus ring, and on a narrow viewport the card covers the
first row entirely. Reproduced across desktop light, desktop dark and mobile -
the focused card's box intersected the card's box at the top of the list and
again halfway down it.
The host now takes the card's own measured height, so the rows begin below the
band rather than under it, and the scroll position absorbs that reserve: someone
reading deep in the list keeps the row they were on, and at the top - where
there is nothing to absorb it with - the rows are what moves, which is the point.
`scroll-padding-top` covers keyboard navigation, so a Tab into a row the card
would cover scrolls it clear, and a row the reader had already scrolled past is
pulled out from under the card when it takes focus. Only the focused card moves;
the reader's scroll position is otherwise left alone.
The card's height is measured rather than assumed because it grows with its
status, its countdown and its eventual reload link, and the reserve follows it.
Verified on this head with a rebuilt client: `@scenario:focused-card-stays-visible-under-the-retry`
fails before this change and passes after it in all three projects, with
`@scenario:pagination-failure-keeps-the-loaded-rows` and
`@scenario:retry-keeps-the-error-card-on-screen` still passing beside it -
the rows, their height, the reader's place and keyboard order are unchanged.
`client/src/components/Agents`: 13 suites / 155 tests pass; `npx tsc --noEmit` clean.
* fix: Keep the Grid Under an Open Dialog, and the Author Boundary a String
Three findings on one head, two of them variants of the same rule - a recovery
state must not take away what the reader is using.
The pending-results branch replaced the whole grid with the loading skeleton,
and the grid is what owns the detail dialog and the element focus returns to. A
refresh with no data of its own - a reset cache, a revalidation after the rows
were dropped - therefore tore an open dialog down mid-flight. The skeleton is
now one of the placeholders the grid renders *inside* itself, beside the
recovery card and the empty state, so the host stays mounted and the dialog
survives its own results being refetched.
A scope change is different: the grid is keyed on the query scope, so a
debounced search or a restored history entry legitimately remounts the rows and
the dialog goes with the list it belonged to. What must not go with it is the
focus it held: the browser drops focus on `document.body`, so the next Tab
starts at the top of the page rather than at the marketplace. The panel takes
focus in exactly that case - focus already elsewhere, the search field being
typed in, is left alone, because only `body` means nobody holds it.
The dialog also read the revalidated agent as the whole agent, and the per-agent
VIEW response carries no `category` (`api/server/controllers/agents/v1.js`), so
an agent that moved out of the loaded window lost its badge. The revalidated
fields are laid over the captured row instead: fresh where the endpoint speaks,
captured where it is silent.
Last, an author cursor's boundary was coerced with `String()`, and coercion is
not validation: a missing or non-string `primary` became the ordinary key
`'undefined'` and resumed the walk partway through the alphabet instead of
answering 409. The date and number modes fail on their own casts; the string
mode now requires a string.
Each is pinned by a regression that fails on the parent commit: the dialog
surviving a refresh and the panel taking focus back
(`AgentGrid.integration.spec.tsx`), the category surviving revalidation
(`VirtualizedAgentGrid.test.tsx`), and four non-string author boundaries
answering `unreadable` (`agent.spec.ts`). `client/src/components/Agents`:
13 suites / 157 tests pass; `packages/data-schemas` `agent.spec.ts`: 232 tests
pass; `npx tsc --noEmit` clean in both workspaces.
* fix: Merge an Avatar Refresh Into the Entry That Is There, Not the One It Read
Every list page for one user reads the refresh entry before its list query, then
spends the S3 round trips of a whole page before writing back. Two pages in
flight together therefore merge into the same snapshot, and the later write drops
the coverage and refreshed URLs the other one added, so the next response that
contains those agents presigns and persists them again.
The merge now re-reads the entry immediately before the write, which narrows the
overlap to that gap instead of the whole refresh. A read that fails keeps the
caller's snapshot, leaving the previous behaviour in place rather than losing the
refresh that just succeeded.
* docs: Say Which Default the Sort Contract Actually Describes
Both the shared type and the grid prop claimed the server falls back to `newest`
when a request omits `sort`. It does not: `resolveMarketplaceListQuery` leaves an
absent or unknown mode undefined, and `getListAgentsByAccess` then serves
`recent`, the most-recently-edited order the agent selector and the mention menu
depend on. `newest` is the marketplace's default, which is why the grid sends it
explicitly.
The grid now names the constant the dropdown already exports instead of
repeating the literal, so the marketplace default has one definition.
* fix: Keep Rich Agent Descriptions Rendering After the Marketplace Rewrite
The rewrite replaced the card and detail views that `Description` had just been
wired into, so an agent whose description is markup lost it: the card printed the
tags as text and the dialog no longer rendered the links or images at all.
The card strips the copy with `getPlainDescription`, which is what it showed
before and what the word morph can measure, since that morph walks text nodes.
The dialog renders markup through `Description` again and hands the paragraph
over whole rather than word by word, because there are no words to measure in a
sanitized fragment. A plain description still morphs.
* fix: Answer a Marketplace Failure Without Taking Away What Is in Use
Twelve findings over three rounds, and most of them are one rule: a recovery
state must not remove what the reader is holding, and a lever the code invents
belongs to whoever runs the deployment.
Focus, three times. The effect that takes focus back after a scope change fired
on the initial marketplace mount as well, because a fresh navigation also leaves
`document.body` focused - so arriving at the page started reading below the
heading, the search field and the filters. It now compares the scope key against
the previous render and never acts on the first one. A successful Retry removes
the recovery card with the focused button inside it; focus moves to the results
panel first, and only when it was inside the card, so a reader who has moved on
is not yanked back. And when the access check settles 403/404, the dialog's
actions are removed under the same rule: focus goes to the status that explains
why they are gone, which is now programmatically focusable, rather than to the
document.
A dialog opened after the owner edited an agent used to keep the cached copy
while re-enabling its actions, so a conversation starter could launch a chat with
text that had already been replaced. Both selected-agent branches now lay the
revalidated fields over the captured row through one helper - fresh where the
per-agent response speaks, captured where it is silent, which is why it is a
merge and not a replacement.
Two operator levers ship as configuration, each defaulting to today's behaviour:
`endpoints.agents.avatarRefresh.coverageLimit` for how many per-agent S3 refresh
deadlines one user's pages remember, and `interface.marketplace.retryDelaysMs`
for the recovery cadence - an empty sequence leaves only the manual Retry, which
is why the reload affordance goes with it. That closes berry-13/LibreChat#24,
which had deferred the retry policy.
The avatar path also stopped losing work and stopped overwriting owners. Two
pages refreshing the same user's avatars both read before either wrote, so the
later write dropped the other's coverage and both pages re-signed inside the
advertised window; merges for one key are serialized, with the re-read left in
the critical section because that is what makes separate processes converge. A
rejecting cache write used to discard URLs that had been signed successfully and
answer with the expired paths; it is best-effort now. And a viewer's in-flight
refresh could land after the owner replaced that avatar: `updateAgentAvatar`
takes the avatar the caller read and only writes while the stored one still
matches, with the skipped agent's stale URL left out of the response.
Two smaller ones. A malformed `popular` cursor carrying `-1` or `1.5` passed
validation and walked an incomplete or duplicated page instead of answering 409;
counts are discrete, so the boundary must be a non-negative integer. And a rich
description rendered inside the paragraph the plain text morphs in, which is
invalid nesting once the sanitized markup carries blocks of its own - the rich
branch is a block, and both share one set of motion props.
Last, `attachAgentOwnerContacts` took a Mongo `$in` filter and a projection
string, which put the storage engine in `packages/api`'s public contract. It now
takes owner ids, and `findOwnerContactUsers` in `packages/data-schemas` owns the
query.
Every fix is pinned by a regression that fails on the parent commit. Locally:
`client/src/components/Agents` 13 suites / 170 tests, `packages/api`
`src/agents/{avatars,ownerContact}` 29 tests, `packages/data-schemas`
`{agent,user}.methods` 330 tests, `api/server/controllers/agents/v1` 146 tests;
`npx tsc --noEmit` clean in `client`, `packages/api`, `packages/data-schemas` and
`packages/data-provider`.
* fix: Hand Focus Back When a Removal Takes It, Not When a Retry Starts
Recovery moved focus at the moment the request went out, which is the wrong
event: a retry that fails again leaves the card on screen, so the reader lost the
button they were still using. Focus is only in danger when the thing holding it
is removed - and the DOM says so plainly, because the browser drops focus on
`document.body` in that commit.
The grid now watches for that: one effect recovers focus when a scope change
remounted the rows or a cleared failure removed the recovery card, and only while
nobody holds focus. The card's retry goes back to being just a retry, and the
recovery band is the measured wrapper again, its bottom gap included.
Two regressions in `AgentGrid.integration.spec.tsx` pin the distinction: focus
lands on the results when a successful retry removes the focused button, and
stays on the button when the attempt fails again - the latter fails on the
previous commit, which yanked focus to the panel while the card was still there.
`@scenario:recovery-hands-focus-back-to-the-results` states it in a real browser,
where focus after a removal is the browser's decision rather than jsdom's.
`client/src/components/Agents`: 13 suites / 171 tests pass; `npx tsc --noEmit`
clean in `client` and `e2e`.
* style: Declare the Serialization Test's Cache as the Constant It Is
The forward reference the mock cache needed while it was assembled in two steps
is gone, so `let` only tripped `prefer-const` in CI's ESLint lane.
* fix: Bind a Cached Avatar URL, a Joined Owner and a Morph's Styles to What They Describe
Four defects from one round, each in a subsystem this branch introduced, and each
the same shape: a value was keyed by the thing it was near rather than the thing
it was about.
A signed URL belongs to an avatar, not to an agent. Coverage keyed on the agent
id alone meant that when an owner replaced one S3 image with another, the viewer's
page was skipped as covered and the controller pasted the cached URL for the old
file over the freshly loaded path - so that viewer kept seeing a removed avatar
for the rest of the window, while the owner saw the new one. Entries now carry the
filepath they were signed for, coverage only counts while that filepath is still
the one on the row, and a refresh pass records what it checked. Entries written
before the binding existed cannot be checked, so they are treated as uncovered
and signed again rather than replayed.
An owner belongs to a tenant. The author sort joins `aclentries` and `users` with
`$lookup`, which does not run the models' tenant middleware, so a stale or
imported principal id resolved to another tenant's user and that name came back as
`owner_contact` - a name the ordinary resolver, which goes through tenant-scoped
methods, never returns. Both joins now match the agent's tenant, with missing and
null treated as the one tenantless scope this file already uses for favourite
counts.
`support_contact` is `Schema.Types.Mixed`, and `$ifNull` passes a number or an
object straight into `$trim`, which rejects it and fails the whole aggregation. A
single malformed legacy row therefore turned every author-sorted page in that
scope into a 500, while `attachAgentOwnerContacts` shrugged the same value off.
Every display operand is now checked for BSON string type first, which leaves the
tier logic unchanged because it already rejects the empty string as a name.
A compositor hint belongs to the animation that needed it. When revalidated copy
arrived mid-morph, the previous effect's cleanup cancelled the settle timer and
the same-phase guard returned before clearing anything, so every reused word kept
`will-change: transform` and its transition for the lifetime of the dialog. One
cleanup now clears the spans it owns on every path - a same-phase change, a phase
change, an unmount - and the refreshed copy arrives at rest instead of replaying
an opening morph.
Two further reports on this round are accepted rather than fixed, with the
reasoning on their threads: the coverage merge is serialized per key inside a
process and not across replicas, because a dropped merge costs signing work the
next page redoes and never a wrong URL - the comment that claimed convergence now
says so - and the rolling-deployment window where a parent-version server ignores
`sort` and `mine` is transient, read-only and self-correcting, while the capability
handshake its remedy needs does not exist in this platform.
Each fix is pinned by a regression that fails on the parent commit:
`refreshes covered agents when their avatar filepath changes` and two siblings in
`packages/api/src/agents/avatars.spec.ts`, `does not resolve an owner user from
another tenant during author sorting` and `tolerates non-string support contact
values while sorting and paging` in `packages/data-schemas`, and `clears word
animation styles when refreshed copy changes during the morph` in
`AgentDetailContent.spec.tsx`. Locally: `client/src/components/Agents` 13 suites /
172 tests, `packages/api` avatars 26, `packages/data-schemas` agent + documentdb
298, `api/server/controllers/agents/v1` 146; `npx tsc --noEmit` clean in `client`,
`packages/api` and `packages/data-schemas`.
* fix: Cover the Path the Refresh Wrote, and Keep Pin Focusable While It Saves
Two findings, both about state that moved on while something kept pointing at
where it used to be.
Coverage recorded the filepath a refresh *started* from, while the write that
followed stored the newly signed one. The next page therefore loaded a row whose
path no coverage entry named, so a successful re-sign guaranteed another
per-agent pass and another cache write inside the window it had just claimed -
the opposite of what coverage is for. Coverage now names the path that was
persisted, because that is what a later page reads, and the cached URL keeps the
path the page in hand still carries, because that is the row it applies to. The
merge no longer requires those two to agree: they are different questions, and
insisting they match is what dropped the freshly signed URL.
A refresh that found nothing to change writes nothing, so its coverage keeps the
path the row already had; that case is now stated as its own test rather than
standing in for the one above.
Pin stayed a control while it worked. Activating it from the keyboard used to set
`disabled` on the button that had focus, and a disabled control leaves focus
navigation, so focus fell to the document until the favourite mutation settled.
It keeps `aria-disabled` and `aria-busy` with a guarded handler instead - the same
shape Copy link, Start chat and the starters already use - so the reader stays on
the control they pressed and a second Enter cannot queue a duplicate toggle.
Both are pinned by regressions that fail on the parent commit: `covers the path
it persisted, so the row it wrote is not signed again` in
`packages/api/src/agents/avatars.spec.ts` (two signing calls where one is
correct), and `keeps focus on Pin while the favorite mutation is busy and ignores
repeated activation` in `AgentDetailContent.spec.tsx` (focus on the document, no
`aria-disabled`). Locally: `client/src/components/Agents` 13 suites / 173 tests,
`packages/api` avatars 27, `api/server/controllers/agents/v1` 146; `npx tsc
--noEmit` clean in `client` and `packages/api`.
* fix: Join the Author's Owner the Way DocumentDB Can, and Leave Retry Focusable
Two findings, and the first is a compatibility rule this branch already carries as
an invariant. Scoping the author joins to the tenant used `$lookup` with
`let`/`pipeline`, which Amazon DocumentDB 5.0 rejects — this repository records
that in `prompt.getPromptGroup.spec.ts` — so selecting Author on such a
deployment would have failed the aggregation and answered 500. The joins are
plain `localField`/`foreignField` again, and the tenant scope is applied to the
joined arrays with `$filter` afterwards: the same isolation, expressed in
operators that survive the target. `documentdb.spec.ts` now fails if either
marketplace lookup goes back to the correlated form, so the next person to reach
for it is told by a test rather than by production.
The shared retry control had the focus fault the pin control just lost.
Activating Retry sets `isRetrying`, whose native `disabled` drops the focused
button out of focus navigation; when that attempt fails the card stays mounted,
the grid's handoff never runs — it only fires when a cleared failure removes the
card — and the reader is left with focus nowhere, beside a card they cannot Tab
into. `RetryableError` keeps the button focusable with `aria-disabled`,
`aria-busy` and a guarded handler, same label, spinner and dimming as before.
The marketplace's existing assertion that the control is `disabled` while
retrying pinned the mechanism rather than the behaviour, so it now states what a
reader gets: busy, still focusable, and a second press that does nothing.
`@scenario:recovery-hands-focus-back-to-the-results` covers both outcomes in a
real browser — focus stays on Retry when the attempt fails, and moves to the
results when the success removes the card — because jsdom blurs neither and that
is exactly how this escaped.
Pinned by `keeps marketplace author lookups in the DocumentDB-compatible form`
and `flags a correlated $lookup let/pipeline form` in
`packages/data-schemas/src/methods/documentdb.spec.ts`, which report four
offending stages on the parent commit, and by `keeps the retry control focusable
and blocks a second activation while retrying` in
`packages/client/src/components/RetryableError.spec.tsx`. Locally:
`packages/data-schemas` agent + documentdb 300 tests, `packages/client`
components + hooks 292, `client/src/components/Agents` 13 suites / 173 tests;
`npx tsc --noEmit` clean in `client`, `packages/client`, `packages/data-schemas`
and `e2e`.
* test: Ask Where Focus Is, Not Which Box It Sits In
The failed-attempt half of the recovery focus scenario asserted that focus was
inside the card's `role="alert"`, which the retry control has never been in: the
alert is the icon, title and detail, and the actions are its sibling, so the
assertion described the DOM rather than the behaviour and failed on all three
projects.
It now asks the only question that matters - the control the reader pressed still
has focus - and lets whichever attempt lands next, theirs or the card's backoff,
be the removal that hands focus to the results.
* test: Count the Morph's Scrim by the Class Both Layers Carry
The base moved OGDialog's scrim from a literal `bg-black/80` to the
`surface-overlay` role, so the morph's backdrop follows it through the shared
constant. The scenario counted layers by the old literal and by black channels,
which after the move would have found no scrim at all, and could only ever have
caught a duplicate dim in the themes whose overlay is black.
It now counts the class the overlay and the morph's backdrop both carry, and
treats any full-viewport layer at alpha 0.3 or above as a second dim, so the
light theme's gray scrim is covered by the same assertion.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFLKexs5HyzqT6H8B9oQqJ
* fix: Keep the Avatar Cache Contract in packages/api, and the Grid Mounted
The list controller was reading the avatar cache entry itself: it checked that a
cached value carried both a filepath and a url, that the row still named S3, and
that the filepath still matched, then wrote the signed URL onto the row. That is
behaviour in a CJS file under /api, and it split the cache contract across two
layers, so a later change to the entry shape would have had to be made in both.
`applyCachedAvatarUrl` now owns that decision beside the code that writes the
entry, and the controller passes the entry through.
The grid was keyed on the active category. Its own scope key already carries the
category, so the key only forced a remount that the scope change was about to
handle anyway, and it took the open detail dialog and its focus-return target
with it. The replacement grid starts with an empty previousScopeKeyRef, so it
reads its first render as a mount rather than a scope change and never hands
focus back, leaving it on the document.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFLKexs5HyzqT6H8B9oQqJ
* fix: Supply the Marketplace Host From the Shell, and Order Two Keys by What Reads Them
The marketplace provider took the app's conversation reset from `~/store` itself.
Wrapping that in a context did not make it a host boundary, because the feature
was still the side reaching for shell state, and the feature could not have moved
to its own workspace without the conversation store coming along. The route now
builds the reset and passes it in, and the provider only carries it. The base did
not have this coupling; it arrived with the host context.
The favourites index led with `favorites.agentId`, a multikey field the popular
sort only ever tests for existence. With it first, the count could not seek into
the caller's tenant and scanned favourites across all of them, which is the work
proportional to the whole corpus that a page is supposed to avoid. Tenant first
bounds it.
The author aggregation compared ACL timestamps through raw field paths. The
resolver it has to agree with sorts the same three keys, where a missing field
reads as null, but a missing field path in a comparison expression is undefined,
which BSON orders below null. An imported entry that omitted `grantedAt` would
therefore win outright and hand the author page a different owner than every
other list order. The timestamps are now read through `$ifNull`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFLKexs5HyzqT6H8B9oQqJ
---------
Co-authored-by: hoangtrongthai-dtvn <thai.hoang2@dac-datatech.vn>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* 🎨 chore: Enforce Design-System Rules With @shadcn/lint Registers @shadcn/lint for client/src and packages/client/src, resolving\n@librechat/client as the design system so a className that overrides what a\nprimitive owns is reported with the variants and sizes to use instead.\n\nFive rules run at error. no-unknown-classes stays off: it needs Tailwind v4.\nContracts record what callers legitimately own (text typography, skeleton\nsilhouette); eslint-suppressions.json records the 3,141 violations the tree\nalready carried, so the rules gate new code without a tree-wide migration.\n * 🩹 fix: Close the Gaps Review Found in the Design-Rule Gate Five findings against this pull request, and one that killed its CI: - The lockfile entries this change added resolved to a private mirror, so every job died in `npm ci`. They now resolve from registry.npmjs.org, with each integrity hash checked against the tarball npmjs.org serves. - The pre-commit hook ran ESLint twice and the first run had no `--pass-on-unpruned-suppressions`, so it exited 2 on the suppression the fix had just made unused and lint-staged never reached the command that tolerates it. One invocation now does both; ESLint applies suppressions after fixes, so it reports the post-fix state. - `scripts/static-checks.mts` omitted that same flag, so `npm run static-checks` failed locally on a diff the Static Checks lane passes. - `lint:design:suppress` recorded a moved file's new path and left the old one behind, because only `--prune-suppressions` removes it and ESLint refuses both flags in one run. The script now chains the prune, and CLAUDE.md documents the scoped recipe that does not re-baseline the whole surface. - The design rules matched only `.ts`/`.tsx`, so the client's `.jsx` entry points bypassed all five. Both globs now cover `.js`/`.jsx`; the one violation that exposed — an inline `display: none` on App.jsx's silent-audio iframe — is fixed at the source rather than recorded. - A suppressions-only diff started no workflow and nothing read the baseline. It is now a trigger path and its own filter group, and a new `suppressions` check rejects a shape ESLint cannot load, a count no run can reach, a rule the plugin does not define, and a path that no longer exists. The count-neutral swap two reviewers raised is not closed: ESLint's baseline format records a count, not violation identities, so replacing one suppressed violation with another of the same rule in the same file is invisible here. CLAUDE.md now says that instead of implying detection. Four `@scenario`-tagged specs cover the developer-facing behaviour: the report a caller gets on a primitive override, a `.jsx` file being checked, the fixing commit staying green (with the counter-proof that it exits 2 without the flag), the move recipe dropping the old path, and every locked tarball being fetchable from the public registry. * 🧪 fix: Close the Dormant-Entry and Gate-Trigger Gaps The suppressions gate did not run when the file that implements it changed, a count recorded against a path no design-rule lint reaches passed validation, the documented re-record chain could not run under cmd.exe, and the lockfile scenario failed a legitimate SheetJS upgrade. The trigger scenarios also ran four full-tree sweeps inside one test that declared a 180s deadline for work measured at 100-280s per sweep. - scripts/static-checks.mts: `scripts/static-checks.mts` joins the suppressions filter and the whole-record trigger, and a recorded path ESLint never reports on is rejected as dormant. - package.json/CLAUDE.md: `|| node -e ""` in place of `|| true`, which cmd.exe does not define. - e2e: the whole-record triggers are a matrix over a miniature repository (2-6s each instead of 100-280s), the oversized scenario is split into five behaviour-shaped ones, scratch baselines live in os.tmpdir(), and the lint gate runs in one project because it has no viewport. * 📏 chore: Re-Record the Counts Dev Moved Under the Backlog Rebasing onto dev brought #16067, which restyled three files the recorded backlog speaks for: MarketplaceSidebar.tsx gained one arbitrary value and two `no-restyle` violations, ToolsMarketplaceDialog.tsx gained one, and SkillsDialog lost one. Counts above the file fail the lint for whoever edits it next; the one below leaves capacity a later violation would spend. Scoped re-record and prune of those three paths, per the recipe in CLAUDE.md. * 🧭 test: Run the Copied Runner From the Tree It Is Testing The trigger matrix pointed the copied static-checks runner at a miniature repository but invoked it from the real one. The runner derives its repository from its own location and relativizes file arguments against the working directory, so `eslint.config.mjs` resolved to a path outside the miniature tree, the suppressions group read as "not affected by this diff", and the scenario passed a check that never ran (verify recorded it as a failure: exit 0 where a rejection was expected). `run` now takes a `cwd`, and both synthetic-root probes use it. * 🔇 fix: Reject a Disable Comment With Nothing Behind It Yet Comparing the two ESLint runs finds a directive only while it is silencing something. A `/* eslint-disable shadcn/no-raw-colors */` over a file with no raw colour left both runs agreeing, so it landed unseen and then silenced the first violation anyone added to that file. The configured run now also asks for unused directives, which is how ESLint names a disable with nothing behind it: a dormant design-rule disable and a dormant blanket disable are rejected, a dormant directive for another rule is not. Neither root carries one today (0 reports across 2,241 files), so what this rejects is new ones. The same pass also has to see the primitives' variants, so `validateSuppressions` builds `packages/client/dist` before the directive check rather than inside the capacity scan that follows it; without the metadata both runs report strictly fewer design diagnostics and a comment silencing a `no-restyle` violation is invisible to the comparison. * 🧱 fix: Count the Library's Build Inputs as Sources `designMetadataIsFresh` compared `packages/client/dist` with `packages/client/src` alone, so a commit that changed only the library's manifest or its tsdown config left every source older than the build and the runner skipped the rebuild. The design rules then classified against output emitted under the old contract — locally only, since CI always builds before validating, which is the shape of divergence nobody sees until the lane disagrees with the commit that passed. Both files already count as design metadata everywhere else (the suppressions filter, the whole-record trigger); they now count in the freshness comparison too. The scenario runs the miniature repository with an observable `build:client-package`: a current build is not rebuilt, and touching either input asks for one. * ⏱️ test: Set the Times the Freshness Rule Compares The rebuild scenario wrote the build output and then the manifest, and node wrote both inside the same millisecond, so the manifest was not newer than the build and the runner correctly declined to rebuild — the assertion failed on the filesystem clock rather than on the rule. The scenario now sets each mtime it depends on with `utimesSync`, which is what it meant to say. * 🩹 fix: Restore the Write the Import Edit Dropped Adding \ to the import list replaced \ instead of joining it, and nothing local caught it: no tsconfig covers e2e/, and the flat config leaves \ to TypeScript there. The mock harness did — every scenario in the file failed with ReferenceError — which is the point of running it. Typechecked the four scenario files directly this time. * 🚫 fix: Reject Every Inline Escape, Not Only the Ones in Use Codex found three more ways past the gate, one shape each. A blanket \ busy silencing an unrelated rule is used, so ESLint's unused-directive report never names it, and the configured-versus-ignored comparison sees no design delta — it waits for the file's first design violation and takes it. Inline \ over a clean file is invisible the same way. And a nested baseline that drops an entry for an unchanged source was checked against its remaining keys only, so the violation it used to speak for went unread. The directive check now has two halves: ESLint still answers what a comment is silencing today, and the comments themselves are read out of the parser — not the text, so a string that reads like a directive is not one — for a disable naming a design rule, a blanket disable, or inline configuration naming a design rule, whether or not anything is behind it yet. That supersedes \, which could only see the unused ones. Neither design root carries any of the three today. \ unions the entries a diff removes into a nested baseline's file list, and \ joins the design metadata: it carries the \ mapping, the entry emit and the include/exclude that decide what reaches the bundle the rules read, which is why the repository's own build caches key on \. * 🔐 fix: Make the Hook Select What the Lane Selects, and Distrust a Half-Built Bundle Two more shapes of the same mistake — trusting a proxy for the thing itself. The pre-commit group listed individual files while the Static Checks lane's filter also names both design roots, so staging a primitive ran ESLint locally and the whole-record sweep only in CI: the commit passed and the lane failed on callers nobody staged. The hook's group is now the lane's filter pattern for pattern, and the count-edit scenario reads both files and fails when the two lists differ, so the next path added to one has to reach the other. Freshness took the newest timestamp under \ as proof of a finished build. The build empties that directory before it writes, so a build that failed halfway leaves a recent directory over a bundle with no entry point, and every later run skipped rebuilding it. The check now also requires what \ promises inside \ — its entry fields and every string leaf of \ — to exist. * 🧾 fix: Give the CDN Exception to xlsx, Not to the Host The lockfile rule allowed \ for anything and then compared versions against the base branch, so regenerating the lockfile against that CDN could move any package off the public registry at the same version and pass. The exception now belongs to the package — \, at any version, nested or not — and every other entry has to come from the registry, which drops the version comparison and the shallow-clone caveat with it. The rule is a function the scenario applies to the repository's lockfile and to lockfiles written for the purpose: an xlsx upgrade on its CDN passes, lodash rewritten onto the same CDN at the same version fails, a new package from a private mirror fails, and a nested xlsx is still xlsx. * ↩️ fix: One Local Entry Point, Asked of the Runner The pre-commit group I added for the backlog and the design metadata was redundant and slower: \ already ends by running the runner over the staged diff with \, and that run selects its groups from the same filters — including both design roots, and including deletions, which lint-staged never hands to a task. The group made the whole-record sweep run twice for one commit. It is gone. The scenario now asserts the property instead of a copy of the list: the hook still runs the runner over the staged diff, and for every path the lane's suppressions paths-filter names, the runner reports that check as selected (\), while a path outside both roots is not affected. * 🗺️ fix: Assert Where the Rules Are Asked, Not Only What They Found A report cannot say where the rules were asked. Narrowing the design block over a subtree that happens to be clean today — \, say — produces no new diagnostic, no changed count, and no baseline entry to inspect, so every gate stays green while the styling written there next goes unread. The new scenario asks the resolved flat configuration instead: for one representative path of each source extension in every directory under both roots, all five design rules must be at error in the app and at the library's own profile inside \ (overriding a primitive is off there; colour and inline geometry still hold), and server code must carry none of them. 1,184 paths, answered through ESLint's API in one process in under a second — \ would have been eleven minutes of subprocesses. * 🎯 fix: Spend the Budget Once — Reject a Swap Inside the Allowance The recorded count is a budget per file and rule, and until now it could be respent: removing one raw colour while adding a different one keeps the total equal, so ESLint suppresses the new violation inside the old allowance and the count checks see nothing move. The backlog was documented as unable to tell that apart, and it was the one hole Codex kept finding its way back to. What the totals cannot distinguish the diagnostics can. For every design source a diff touches, the runner now lints that file's version at the base — through ESLint's API, one process for the whole diff — and reports a design-rule message the head has that the base did not, naming it. A violation that only moved keeps its message and is not reported; a file with no version at the base has no allowance to hide in and the count checks already answer for it. Measured on the checkout: swapping one `fill="#AFC1FF"` for `fill="#BEEF11"` in a file recording five raw colours is rejected in 4.9s naming the new one, and the unchanged file passes. The scenario proves all three states — recorded, moved, swapped — in a miniature repository with a history of its own, and CLAUDE.md no longer claims the swap is undetectable. * 🧰 fix: The Root Manifests Are Build Inputs Too \ is defined in the root manifest and runs with the toolchain the root lockfile pins, and both already select the suppressions check — but neither counted as an input to bundle freshness, so a commit that changed the build definition could validate locally against output made by the previous one while CI rebuilt. They count now, beside the library's own manifest, build config and compiler options, and the rebuild scenario drives the lockfile case through the miniature repository's observable build. * 🧭 fix: Read the Base From the Merge Base, and Give a New File No Allowance Two holes in the swap check, both about what "the base" means. It read each changed file's prior version from the base branch's tip, while the changed-file set comes from \. With \ moving under a branch, that calls a violation dev added since the branch diverged this change's doing, and forgives one this change added that dev happens to carry. It resolves the merge base once and reads from there. And a file with no version at the base was skipped, so adding a source with a violation and recording it in the same commit passed as "already recorded". A file the change adds has no allowance to inherit: every design violation in it is new. Renames are followed (\), so the documented re-record of a moved path stays green — its violations came with the file. The scenario now walks recorded, moved, swapped, added-with-its-own-entry and renamed through one miniature repository with a history of its own. * 🔗 fix: One Base Commit, Asked Once \ resolved the merge base; \ still read the baseline from the base branch's tip, so the two checks in the same run disagreed about what "before" meant — and an entry the base branch edited after the branch diverged entered \ as though this change had edited it. \ resolves it once and both ask it. * 🔤 fix: A Suppressed Message Carries Its Text \ omitted \, and the swap check reads it — the union of messages and suppressed messages is what a file's diagnostics are, and what the baseline silences is exactly what that check has to see. Nothing local caught it: node strips types without checking them and no tsconfig covers scripts/, so the runner is now typechecked directly (�[41m �[0m �[41m�[37m This is not the tsc command you are looking for �[0m �[41m �[0m To get access to the TypeScript compiler, �[34mtsc�[0m, from the command line either: - Use �[1mnpm install typescript�[0m to first add TypeScript to your project �[1mbefore�[0m using npx - Use �[1myarn�[0m to avoid accidentally running code from un-installed packages, clean). * ⚖️ fix: Let the Record Grow, Out Loud The swap check reported every violation a change added, which also forbade the one path CLAUDE.md sanctions: add the violation and raise the count, so the debt arrives as a line in the diff a reviewer reads. What separates that from a swap is not the violation, it is whether the record said so — a swap keeps the count still. The check now charges each new diagnostic against how much that entry's count grew between the merge base and here, and reports only what the growth does not cover. Measured: a swap at an unchanged count fails, the same violation with the count raised passes, and raising nothing fails naming the class. CLAUDE.md says which of the three is which. * 🚧 fix: A File the Change Adds Brings No Allowance Letting the record grow to cover a new violation is right for a file that already existed — the raised count is the line a reviewer reads — but it also let a brand-new source arrive with its own entry, which is the backlog recording what the tree never owed. Growth applies only where there is something to grow: a file present at the merge base, renamed or not. A file the change adds owes everything in it, and the message says so. * ✏️ test: Assert the Wording the Added-File Rule Uses The scenario still expected "is new here" for a file the change adds, which now gets its own message — the backlog records what the tree owed, not what a new file brings. The harness caught it; the assertion follows the rule. * 🧮 fix: Ask Git What Existed, and Where Its Entry Lived Three faults in the new swap check, one cause: it inferred from content what only git can answer. A tracked empty file at the base reads as absent, so an existing file was treated as newly added and its recorded growth refused. A renamed file's growth was measured at the new key, where the base has no entry at all, so the move itself looked like room to spend. And a base commit missing from the clone made every changed file look new, silently. Existence is now \ succeeding, the allowance is read under the name the entry had (the rename map already resolved the source; it resolves the entry too), and a base commit that is not in the clone is reported rather than guessed at. Measured: a rename carrying a new violation with its count raised at the new key fails, a plain rename passes, a swap at an equal count fails. * 🗃️ fix: A Deleted Baseline Is a Deleted Baseline, Nested or Not The root baseline's deletion failed the check; a nested one was quietly dropped from the target list by the same \ filter that was meant to skip paths never present. What a nested baseline silences goes back to being reported — or stops being — and nothing else reads it, so its deletion is the same failure and now says which file is missing. * 📚 fix: A Baseline in the Diff Is Read in Full The comment said a baseline named in the diff is checked in full; the code checked only the entries the diff moved, because \ made \ non-empty and the full-record branch required it to be empty. So a count edit never revalidated the rest of the record — dormant paths and stale counts in unedited entries went unread. Editing the baseline now reads every path it records. And the rename map keyed on git's raw line, so a CRLF checkout left a \ on the new path and the rename never matched — the file read as added and its inherited allowance was refused. The lines are trimmed. * 🧷 fix: Lint the Base Under the Name It Had, and Compare a Commit With Its Parent Two last seams in the swap check. A renamed file's base contents were linted under the new path, so moving a component out of \ — where overriding a primitive is the library's own business — into \ made the destination's stricter rules apply to both sides: the diagnostics the move itself introduces looked inherited, and a matching entry would have hidden them. The base is linted as the path it had. And \, the documented single-commit mode, resolved its comparison to \ — the commit against itself, which forgives everything. It resolves to the commit's parent. * 🔖 fix: Quotes Do Not Hide an Inline Rule Configuration ESLint reads \ exactly as it reads the unquoted key, but the comment scan required the colon to follow the rule id directly, so the quoted form passed over a file that does not violate the rule yet — and then silences the first violation a primitive or config fan-out creates there. Over a violating file the two-run comparison always caught it; the dormant case is the one that slipped. The scan accepts either quoting, and the scenario carries the quoted spelling beside the rest. * 🧪 perf: A Spec Is Not Component Metadata Both roots the design rules police are full of specs — every primitive in `packages/client/src/components` has one — and editing any of them ran the two-root suppression sweep, because the metadata predicate asked only whether the path was a lintable source under a metadata root. A spec is neither: the flat config turns all five design rules off inside it and the library's tsconfig keeps it out of the bundle the caller rules resolve primitives through, so it carries no `cva` variant and can report no design diagnostic of its own. `packages/client/src/components/Button.spec.tsx` through the runner: 59.7s before, 0.0s after. The same `DESIGN_TEST_SOURCE` test is applied everywhere the invariant holds: the freshness input no longer treats a spec as something `dist` is built from (directory mtimes stay, so an added or deleted source still forces a rebuild), the directive gate no longer fails a commit over a comment in a file where the design rules are already off, and the base-vs-head lint no longer fetches a spec's base version to ask a question with one possible answer. * 🔎 fix: A Build That Reports Success Is Not a Build Two gates trusted a signal instead of the condition it stands for. `buildClientPackage` returned on the build's exit code. A build that exits 0 while leaving `packages/client/dist` incomplete or older than its sources left `designMetadataIsFresh()`'s answer exactly as it found it, and the suppression lint that follows then classified callers against a bundle describing something else — reporting fewer violations than CI, which is the failure the rebuild exists to prevent, reached through the branch meant to fix it. The freshness question is now asked again after a successful build, and a bundle still stale is an error naming `packages/client/dist`. The workflow's `on.pull_request.paths` named only the root `eslint-suppressions.json` while the inner `suppressions` filter names `**/eslint-suppressions.json`. A diff touching only a nested baseline outside `api|client|config|packages|scripts` — `e2e/fixtures`, say — started no workflow at all, so `validateSuppressions()` never ran over exactly the nested baselines it advertises. The trigger carries both spellings now. The miniature repository's `build:client-package` writes the bundle its manifest promises instead of only a marker, so the rebuild rows assert a finished build rather than a claim of one, and a build that writes nothing is its own row. * 🧭 fix: Ask the Tool, Don't Reconstruct Its Answer Four findings on the last head, and three of them were the next variant of a family this branch had already patched once. The shared cause is one habit: the gate re-derived a fact from a cheaper proxy instead of asking whatever owns it, and every round found another case the proxy got wrong. An inline `eslint` configuration comment was matched by the spelling of its rule key, so `shadcn/x`, `"shadcn/x"`, `"shadcn\/x"` and `"shadcn\u002fx"` — one key, four spellings, all of which ESLint honours — needed four regexes. The key is now decoded before it is compared, which ends the family rather than adding the fourth. Bundle freshness reduced a set of outputs to the newest file anywhere under `dist`. A resolver loads one entry, not a directory, so a build that rewrote the ESM bundle and left the CJS one or the type declarations behind passed while `@shadcn/lint` read the stale one. Every declared output is compared now, which is the same reason their existence was already checked one by one. Target discovery read `git diff --name-only` with rename detection, which names one side. `git mv eslint-suppressions.json backlog.json` therefore selected no group that owns the source, and the baseline's disappearance was reported by nothing. `--no-renames` at the single funnel makes a rename two paths; the one caller that wants the pairing asks for it separately. The fourth was the parity assertion added last round, sitting in the mock Playwright lane — whose own trigger excludes `.github/workflows` apart from its own file, so a workflow-only diff never ran the assertion guarding it. The claim moved into the runner, where the file that can break it already activates the group, and the scenario now exercises the runner both ways over a copy of the real workflow instead of reimplementing its parser. * 📝 docs: Say Which Swaps the Ratchet Misses The ratchet paragraph claimed a swap arrives as a failure, full stop. Two do not, and a reader deciding whether to trust the gate needs both. A swap whose diagnostic message equals the one it replaced is invisible, because the comparison keys on the message and some rules name the property without the value — `Inline style sets display.` is one message for `none` and for `flex`. And a swap the diff itself causes, by changing a primitive or the config, is invisible because both versions are linted under the head's design inputs. Tracked at berry-13#70 and berry-13#69. * 📏 chore: Re-Record the Count Dev Moved Under the Backlog `dev` fixed one `shadcn/no-restyle` violation in `client/src/components/SharePoint/SharePointPickerDialog.tsx`, so the rebased branch records 8 where the file now has 7. An entry with slack is exactly what this change added a check for: the unused 1 would silence the next violation anyone writes there, and the gate says so rather than letting it through. `npm run lint:design:prune` drops the one count; the trailing newline it strips is restored by prettier, the way the pre-commit hook's `*.json` group does. * 📝 docs: Drop a Recorded Count That Dev Keeps Moving The backlog total moves with every rebase that picks up new violations from `dev`, so a number written into CLAUDE.md is stale by the next re-record — it read 3,230 against a baseline holding 3,232. The recipe does not need the figure; `eslint-suppressions.json` is the record. * 📏 chore: Re-Record the Counts Canary Moved Under the Backlog Retargeting onto canary changes the tree the design rules read, so five files carry different violation counts than the recorded backlog claims: Wakeup and SubagentConversation shed no-restyle violations and picked up a require-static-classes one, and Ledger, UnifiedSidebar and ChatRoute each gained one. Re-recorded those entries and pruned; nothing else in the baseline moved. * 🧱 fix: Build the Primitives Before Recording What They Own `lint:design:record` was the one exposed design script that linted without building `packages/client/dist` first. The rules read each primitive's `cva` variants out of that bundle, so a fresh checkout or a primitive change left the recording run classifying against a library that does not exist yet and writing counts lower than the ones CI reports. Moved the build onto the record script, where the other three already have it, and dropped it from the `lint:design:suppress` wrapper so the wrapper still pays for exactly two builds. A build failure inside the wrapper is still loud: the record step cannot rewrite the baseline when the build it is chained to fails, and the prune that follows does not swallow its own. * 📏 chore: Re-Record the Counts Canary Moved Under the Backlog Rebasing onto canary picks up PR #15798, which rewrote the agent marketplace components, so eighteen recorded entries no longer describe the files they name: AgentDetail is gone, AgentCard traded its restyle and arbitrary-value violations for inline styles, AgentDetailContent grew to eighteen restyles, and CopyLink, GridSkeleton, Marketplace, MineFilterToggle, SortDropdown and LoadingDots arrived with violations nothing had recorded. Re-recorded and pruned; every entry that moved belongs to a file that commit touched, and nothing else in the baseline changed. * 🧪 test: Assert the Record Asks for a Build Before It Writes The build-freshness scenario covers the runner. Nothing covered the other entry point: `npm run lint:design:record`, which writes the budget every later change is measured against and, until now, linted whatever bundle happened to be on disk. The miniature repository's `build:client-package` appends to a log, so the scenario can ask whether the record built before it wrote, and whether a build that fails still stops the wrapper now that the wrapper no longer carries a build of its own. --------- Co-authored-by: berry-13 <berry-13@users.noreply.github.com>
* 🧵 fix: Restore Classes tailwind-merge Was Silently Dropping tailwind-merge 1.9.1 predates Tailwind 3.3, so its conflict map has no group for text-wrap, touch-action, gradient color stops, or arbitrary bg-* properties. cn() therefore deleted classes the caller wrote, with no error: touch-pan-y on the mobile sidebar, from-* gradient colors behind three overlays, text-balance and text-pretty across the Projects and Prompts cards, and the checkerboard background on the skill file preview. Bumps client and the @librechat/client peer range to ^2.6.1, the line that supports Tailwind 3.x. Tailwind v4 requires tailwind-merge 3.x and moves in the v4 pull request, so this change is the class map alone. * ✅ test: Cover the Classes tailwind-merge Was Dropping From a Browser Three `@scenario`-tagged specs ask the browser what the DOM actually got, one per class `cn()` was deleting: the mobile drawer's `touch-pan-y` beside `touch-pinch-zoom`, the shared-conversation fade's `from-surface-secondary` beside `from-40%`, and `text-balance`/`text-pretty` beside `truncate` and `line-clamp-2`. Each resolves both sides of its comparison in the browser — computed value against the theme custom property, normalised through a probe — so the same spec holds in light mode, dark mode, under a custom theme and after the Tailwind v4 upgrade that follows this change in the stack. The wrapping scenario asserts the Projects header and description rather than `Prompts/lists/ListCard.tsx`: that component is exported but has no JSX call site anywhere in the repository, so its `text-balance` renders nowhere and the pull request's table overstates it. The `Projects` views carry the same pair and do render. * 🔬 test: Ask the Browser for Styles by Their CSS Names computedStyles took camelCase names straight to getPropertyValue, which only answers to the CSS spelling and returns '' for anything else, so every probe but width and height fell through to an indexed read on the declaration that TypeScript only allowed behind an 'as never' cast. touchAction reached the assertions that way. The helper now hyphenates the name it was given and asks once: a property the browser does not know comes back empty and fails its assertion instead of silently taking another path. --------- Co-authored-by: berry-13 <berry-13@users.noreply.github.com>
* 🌊 refactor: Upgrade the Client to Tailwind v4
Moves client/ to tailwindcss 4.3.3 through @tailwindcss/postcss, keeping the
JS config via `@config` so createTailwindColors(), the theme preset and the
high-contrast variant stay exactly as they are.
Renames the utilities whose meaning changed (outline-none -> outline-hidden,
shadow-sm -> shadow-xs), the ones v4 removed (flex-shrink-0 -> shrink-0,
flex-grow -> grow, blur-0 -> blur-none, ring-opacity-N -> a /N color modifier)
and the tailwindcss-radix variants, whose plugin generates nothing under v4, to
the data attributes Radix already sets. Restores the two preflight defaults v4
changed (border-color, ::placeholder) so the upgrade carries no visual change.
Verified by compiling the app's CSS under both versions: every class the source
uses that v3 generated, v4 generates too, and each declaration difference is a
v4 representation (logical properties, color-mix alpha, theme variables) with an
equal computed value.
* 🩹 fix: Keep the v4 Upgrade Invisible Where Review Found It Was Not
Four findings against this pull request, all of them defaults v4 changed under a
tree written against v3:
- v4 moved the blur scale up a step: its `blur-sm` is 8px where v3's was 4px,
and 4px is now `blur-xs`. Nothing in either Tailwind config overrides the
scale, so all six surviving small-blur sites rendered at twice their radius.
They are renamed `blur-xs`/`backdrop-blur-xs`.
- The high-contrast block reset `--tw-ring-opacity`, a variable v4 does not read,
and its comment named `ring-opacity-50`/`ring-opacity-20`, classes this change
deleted. Compiling the converted controls through this tree's own Tailwind
shows the slash modifier baking the alpha into `--tw-ring-color` as a
`color-mix(...)` — the ring measured `oklab(0 0 0 / 0.5)` in the mode that
promises full strength. The block now re-declares the ring colour, and the
same probe measures `rgb(0 0 0)`.
- `ResizableHandle` and its `Alt` twin carry a bare `border`, which v4 paints
with `currentColor`. The app's compatibility shim covers that inside the SPA
but is not part of the published stylesheet, so a standalone consumer drew a
text-coloured hairline around a `border-medium` grip. Both handles now name
`border-border-light` — the same #e3e3e3 the shim resolves to, and it follows
the active theme.
- `tailwindcss` was a devDependency only, while the published components emit
`outline-hidden`, `shadow-xs` and `origin-(--radix-…)`. A Tailwind 3 host
installed the package with no warning and silently lost focus suppression and
popup transform origins. `^4.3.3` is now a peer, like `tailwind-merge`, and the
consumer contract says so.
Two consequences of rebasing onto `dev` are settled here as well. The 28 commits
this branch caught up with added styling to eleven files the upgrade touches, so
their `no-restyle` counts are re-recorded: those violations are `dev`'s existing
code arriving in this pull request's lint scope, not new debt, and the diff shows
which file each belongs to. And `TemplateTable.tsx` goes back to exactly what
`dev` has: it is an unrendered mockup carrying six literal strings the i18n rule
rejects, so renaming its one `radix-state-open:` class would have pulled six
pre-existing errors into this lane for a component nothing mounts. It is the one
file the rename skips, and the reason is here rather than in a suppression.
Six `@scenario`-tagged specs read what the app paints: the 4px blur on the
artifact scrim, the ring resolving to the opaque ink token under high contrast
and to a `color-mix` without it, the handle's border following the theme in dark
mode rather than inheriting `currentColor`, a focused control keeping an outline
under forced colours, the restored v3 border and placeholder defaults, and npm
refusing the packed tarball beside Tailwind 3 with ERESOLVE.
* 🧱 fix: Keep v4's Cascade Layers and Revived Classes Out of the Rendering
Two mechanisms let the upgrade change what the app paints, and each one had more
sites than the reports that found it.
Tailwind 4 emits every utility inside `@layer utilities`, and an unlayered
declaration beats a layered one whatever its specificity. The SPA's hand-written
element and bare-pseudo rules used to lose to utilities by being element
selectors; they started winning. Measured in a browser against the compiled
stylesheet, before and after: a `<p class="my-4">` computed `0px` margins instead
of `16px`, a themed `<select>` fell back to square corners, a white background,
16px text and 40px of native arrow padding instead of `rounded-lg`, transparent,
14px and `px-2`, and a keyboard-focused control painted a 2px black outline over
the ring `focus-visible:outline-hidden` was supposed to leave alone. The `select`
rule, the `blockquote…p, pre { margin: 0 }` reset and the two bare focus rules now
sit in `@layer base`. `.dark :focus-visible`, the `code, pre` font `!important`
reset and the `.hidden`/`.overflow-*` class overrides stay unlayered: those are
meant to beat utilities, and layering them would change behaviour.
Tailwind 3 generated nothing for classes it could not parse, so a number of them
were dead markup; Tailwind 4 honours them. Compiling the same sources under 3.4.1
and 4.3.3 and diffing the generated selectors (2,412 vs 2,435) turns up 27 v4-only
tokens, 12 of which are this upgrade's own renames and 15 of which already existed
on `dev`. Those 15 are restored to what v3 painted: the artifact panel keeps its
150 ms desktop open and 300 ms mobile close rather than 350 ms and 250 ms, the
continue icon stays 12px rather than 76px, agent avatars keep their borderless
edge, file rows and the files side panel stay fluid rather than jumping to 25rem
and 7.5rem, a dialog that sets no `max-w` stays full width, the source rails keep
the scrollbar they had, the composer's control row stops declaring a container,
and the dropdown keeps the height, transform origin and destructive icon colour it
actually had. Each one is a design decision someone wrote and never saw; they are
catalogued for deliberate adoption in berry-13#67 instead of switching
on inside an invisible upgrade.
The reverse direction is clean: of the 70 selectors only v3 emits, every one is
extractor noise — JavaScript negations (`!visible`, `!table`, `!running`) its regex
mistook for important-prefixed classes, and a bare `animate-none` it split out of
`motion-reduce:animate-none`. No class is lost.
Four scenarios cover the two invariants: a paragraph keeps its margin utility, a
focused control shows only its ring, a native select keeps its token styling, and
the classes v3 ignored stay inert — the last one reading the artifact panel's
150 ms and then walking the stylesheets the page loaded to assert no rule exists
for any of the ten revived tokens.
`AddMultiConvo.tsx`'s `no-restyle` count drops 10 → 9, the entry the previous
commit's class removal left unpruned, and `AgentDetail.spec.tsx` loses the blank
line that deleting an assertion left behind — both were failing CI on the previous
head.
`FileListItem` is deleted rather than edited. It carried one of the `w-100` rows,
and nothing renders it: `FileList.tsx` imports it beside a commented-out line and
renders `FileListItem2` instead. Keeping it would have kept a `w-100` rule in the
stylesheet the scenario asserts is empty, and dragged its six unlocalized literals
into this lane — the same reason `TemplateTable` went earlier. Its dead import and
`no-restyle` entry go with it, as does an unused size map in `agents.tsx` that the
avatar never read.
* 🧹 fix: Stop the App's Own `.hidden` From Outranking `md:flex`
`client/src/style.css` declared `.hidden { display: none }`, a duplicate of the
utility Tailwind already emits. Unlayered, it outranked every layered variant that
reveals an element, so `hidden … md:flex` and `hidden … md:block` stayed hidden at
every width: the header's desktop cluster — export/share, the trace button,
temporary chat — never appeared, and the composer's MCP control lost the label its
accessible name comes from, which is why `e2e/specs/mock/activity-labels.spec.ts`
timed out waiting for a button named "MCP Servers" while the same suite passes on
the base branch.
Measured in a browser against the compiled stylesheets, `<span class="hidden
md:block">MCP Servers</span>` inside a button at 1280px:
| Stylesheet | computed display | button name |
|---|---|---|
| Tailwind 3 | `block` | `MCP Servers` |
| Tailwind 4, rule present | `none` | *(empty)* |
| Tailwind 4, rule deleted | `block` | `MCP Servers` |
Deleting it is the whole fix: Tailwind generates the identical `.hidden`, inside
`@layer utilities` where the responsive variants can win. `.overflow-x-auto` and
`.overflow-y-auto` stay unlayered with a note — they deliberately override the
utility of the same name with `overlay`, no source pairs them with a variant that
would need to win back, and both computed values are identical under either major.
`@scenario:the-desktop-header-shows-its-controls` opens a chat at 1280px, requires
the export/share control to be visible, narrows to 500px where it must be hidden,
and widens again.
* 🔗 fix: Let a List Link Keep the Utility That Unstyles It
`client/src/mobile.css` paints every list link blue, bold and underlined. `li a`
is two element selectors, so under Tailwind 3 any class beat it — which is how
the source cards in `verticals.tsx` and the skill file viewer, both carrying
`no-underline`, actually looked. Unlayered under v4's cascade layers it outranked
the utility instead, and those links came back underlined and bold.
`li a` moves into `@layer base`. `p.whitespace-pre-wrap a` does not: at (0,1,2) it
already outranked a single class under v3, and layering it would be the behaviour
change. Measured in a browser against both compiled stylesheets, a
`<li><a class="no-underline">`: Tailwind 3 computes `text-decoration-line: none`
with `font-weight: 700`, and the fixed Tailwind 4 build computes the same, while
the paragraph link keeps its underline in both.
`@scenario:a-list-link-keeps-its-no-underline-utility` paints both links through
the stylesheet the app loaded and asserts the utility wins on the carded one, the
plain one keeps its underline, and the bold the rule owns survives.
* fix: Carry the v4 Upgrade Across What Dev Added
Rebasing onto dev brought seven commits, and three of them land styling and a
test this upgrade has to account for.
ToolCall.tsx and McpOAuthDialog.tsx arrived written in v3 class order, which
prettier-plugin-tailwindcss rejects under v4. Both are reordered, with no class
added or removed. Textarea.tsx already carried the same drift and was the only
file in this branch's own diff against dev still failing the rule, so it is
reordered here as well.
dialog-scrim-role.spec.ts pins the new OGDialog scrim by string-comparing
getComputedStyle().backgroundColor against an rgba literal. Tailwind 4 compiles
a slash modifier to color-mix(in oklab, ...), which Chromium reports as
oklab(L a b / alpha), so all four scenarios would read oklab(0.463999 ... / 0.8)
where they expect rgba(89, 89, 89, 0.8). The paint is unchanged and only the
serialization moved, so readScrim now rounds the computed color through a 1x1
canvas, which returns the same sRGB channels for either spelling. The
expectations stay written as the rgba a reader can check against the palette,
and the spec keeps testing the color rather than its text form.
Dev's change also deleted the overlay this file used to render beside its
content, which was one of its recorded no-restyle violations, so the backlog
entry drops from 8 to 7.
* fix: Give the Primitives the Focus and Elevation Their Callers Were Restating
The design-rule gate reads a renamed utility as a swapped violation, so every
v3 name the upgrade rewrote on a primitive (outline-none to outline-hidden,
shadow-sm to shadow-xs, radix-state-open: to data-[state=open]:) failed it.
Fix the call sites instead of re-recording them:
- Button, Input, Textarea, SecretInput and AnimatedTabs already hide the
outline on the focus state that shows one, so the restated class goes.
- TextareaAutosize, TooltipAnchor, OGDialogContent, OGDialogClose,
OGDialogTitle and CollapsibleTrigger take focusOutline="hidden" for a
caller that draws its own indicator.
- Alert takes elevation="raised" for the auth notices and message errors
that carried a shadow.
- The presets trigger's open fill moves to the Radix Trigger that stamps
data-state; the parameters anchor's never matched, so it is dropped, as is
the MCP config inputs' one-off shadow.
Two controls had lost their keyboard outline under v4, where outline-hidden
sets --tw-outline-style: none and outline-2 reads it: the files table's name
cell gets it back, and the admin settings trigger drops the dead outline for
the ring Button already draws.
* test: Compile the Code Typography Spec With Tailwind v4
The spec dev added in #16146 ran tailwindcss as a PostCSS plugin, which v4
moved to @tailwindcss/postcss. Compile style.css the way the app does, with
source(none) and an inline source standing in for the raw content the v3
config override supplied, and bridge Node's structuredClone into jsdom for
the compiler. The assertions are unchanged and still fail when style.css
restates the code font with !important.
* test: Cover the Focus Outlines the Primitives Now Own
Adds two scenarios beside the upgrade's other focus checks: the files table's
name cell draws its 2px keyboard outline again, and a control that opts into
focusOutline="hidden" drops the browser outline while forced colors still
repaints the transparent one.
* fix: Keep OGDialog's Entrance From Sliding In Half a Width
v4 compiles translate-x-[-50%] and translate-y-[-50%] to the translate
property, which tailwindcss-animate's enter and exit keyframes compose with
through transform instead of replacing as they did in v3. The surface's
slide-in-from-left-1/2 and slide-in-from-top-[48%] therefore started every
OGDialog half its width to the left of centre and slid it across, where v3
started it 2% of its height below its resting place. Name only that 2% in
each direction, which reproduces v3's motion over the centering translate.
Found by #16068's dialog-scrim-role scenarios, which sample the dialog's
position mid-entrance and failed against the upgraded build.
---------
Co-authored-by: berry-13 <berry-13@users.noreply.github.com>
…ead (#15025) * feat: add unseen reply indicators, reply notifications, and mark as unread A reply that lands while the user is elsewhere now leaves a trace. The conversation carries a durable reply stamp written with the assistant message that backs it, the sidebar row shows an unread dot until that message is actually rendered, and an optional tab badge, desktop notification and chime announce arrivals in a tab that is away. Rows can also be marked unread by hand from the row menu, in the archived view as well as the main list. The stamp is written by a compare-and-swap so concurrent replies order on the database rather than on the application hosts' clocks, and only once output that a reader can actually see is persisted: a stopped turn with nothing to read, an empty synchronized row, or a failed message write never lights a dot that cannot be cleared. Read intent survives archives, metadata edits and preset applies, and a later read always outranks a delayed unread. * fix: gate reply alerts on config and announce only readable replies Three review rounds kept finding the same two shapes, so both are answered once rather than patched again. The alert capabilities are now operator-configurable through `interface.replyNotifications` in `librechat.yaml`: the tab badge, desktop notifications and the chime each have a gate, and the away poll's page size is a field rather than a constant, with defaults that reproduce the shipped behavior. A capability the operator turns off reads as off whatever the device stored, and its settings toggle is not offered. Deciding whether a persisted turn may raise an indicator now lives in one place in `packages/api`, with the Responses API controller, the abort route, the Assistants thread sync and the client's override path all calling it instead of carrying their own copy. That closes the last case where a reply nobody can read lit a dot nothing could clear: a `/v1/responses` completion whose output held only reasoning or tool calls persists an empty row, and stamping it left an unread conversation that opening could not settle. Also: the retention backfill no longer advances `updatedAt` past a reply stamp, which an away tab reads as a metadata-only promotion and withholds the alert for; arrival evidence is swept of conversations no cached list still holds, so a long-lived tab stops growing with its lifetime number of replies; and generated Lighthouse profile directories are ignored where the runner leaves them. * test: open the sidebar by what the drawer paints, not by its test ids The drawer became a mounted, sliding panel on canary, so the controls this helper read no longer say what it assumed. `close-sidebar-button` answers from the off-canvas copy before the expanded state commits, which returned the helper with the conversation list still hidden behind the pane, and the panel's own `new-chat-button` has a rail copy that a plain locator reaches first. Both readings passed on the previous drawer, which unmounted when closed. Settling on the close control's accessible name, and on the first candidate that is actually painted, is what a reader perceives and survives either drawer. One click is one toggle and the opener sits in the pane the drawer marks inert for the whole of its travel, so each attempt is given that travel to itself rather than reissued into it. Also serves `interface.replyNotifications` in the operator-gate scenario with a fresh body instead of the upstream response, since reusing its encoding headers for a re-serialized payload leaves the client unable to read the config, which looks exactly like an operator who configured nothing. * fix: gate the assistant save on renderable output and stop overselling the poll limit Two gaps this round's own work left behind. `saveAssistantMessage` still decided to stamp from the message id alone, so an Assistants run that persisted an empty row raised a dot that opening the conversation could never clear. It now asks `isAnnounceableReply`, the same predicate every other persistence path asks, which is the point of having moved that decision into one place. `pollLimit` advertised a range up to 1000 while `getConvosByCursor` clamps every conversation read to a hundred-row page, so an operator who set 500 would have been told it applied and silently served a hundred. The schema now stops at the page the server will actually return; covering a deployment whose replies outpace one page needs the server-side unseen query, not a wider page. * test: let the mobile scenarios use the surface they are actually asserting on Three scenarios opened the drawer on a page they only ever send a reply from. On the mobile project that drawer sits over the composer and marks the pane inert, so there was nothing left to type in and the run waited out its whole budget. They ask for the composer now, which is what they were always about. The two that read a row after leaving the list open it again first. A reply sent from another tab closes this one's drawer, because `sidebarExpanded` is stored per browser rather than per tab, and opening a chat closes it the way it does for a reader. Both are what someone would do to look at the row, and neither changes what is being asserted. Mark as unread is dispatched rather than clicked, like the row controls around it: the menu is portaled, and behind the drawer's scrim it inherits `pointer-events: none` and goes click-dead. * test: dispatch the row menu's unread item like the controls around it The portaled menu sits under the mobile drawer's scrim, which intercepts the pointer, so a real click retried until the budget ran out. The sibling scenarios and the row's own controls already dispatch; these two were the last real clicks. * fix: hold reply alerts until the deployment answers, and read the resolved temporary state The capability gate treated a startup config that had not loaded yet the same as one that set nothing, so a device that stored "on" could badge, poll or notify in the window before an operator's `false` arrived. Nothing is permitted now until the config is in hand; only a loaded config that lacks the field reads as the shipped default, which is the case a backend predating the setting produces. The Responses API announcement read the temporary flag from the request body alone, while the conversation save beside it resolves it from the stored conversation first. A temporary chat restored or resolved on the server without the flag in the body would have been announced through the unread indicators; both completion branches now resolve it the same way the save does. * fix: route the last reply stamps through the shared owner and hand back failed claims Moving the announce decision into packages/api left three callers still deciding for themselves. The agents controller re-stamped an unfinished turn whose terminal persistence was skipped, and the resumed legacy turn stamped its directly saved row, both from the message id alone, so a preempted or stopped turn that persisted nothing readable lit a dot that could never be acknowledged. Both ask isAnnounceableReply now. The non-agents abort path carried an inline copy of the same predicate; it calls announceReply instead. Error turns keep their direct stamp: the error card is what renders them. A notification claim was written to shared storage before the notification was constructed, so a constructor that throws left the reply claimed in every tab with nothing ever shown. The claim is handed back when construction fails, removed only while it still names that exact stamp. * fix: judge the normal reply save by the shared predicate and hand back failed chimes The regular BaseClient save still stamped any assistant reply that persisted, readable or not, because the conversation write took a bare reply id. It takes the persisted reply now and asks isAnnounceableReply before it stamps, which leaves the error turns as the only direct stamps, and those are rendered by the error card. saveTurnConversation is exercised against a real store in both directions. announce.ts derives its write-context type locally so the two modules do not import each other. The chime claimed its replies before building the tones, so an output that failed while they were being scheduled left them claimed in every tab with nothing played. Those claims are handed back on failure, the same way a notification that could not be constructed now releases its own. * feat: let operators set the reply polling cadence The away poll and the focused refresh were fixed at 30 seconds and five minutes, which are request-rate levers on the conversation list and nothing an operator could tune. Both are fields on `interface.replyNotifications` now, bounded and defaulting to the values they had, and the watcher reads them through the same capability hook as the rest of the block. * fix: keep reply identity through list merges and drop rows the server no longer has The list merge carried three of the four read-state fields by hand and left out `lastResponseMessageId`, so an SSE update on an unseen conversation erased which branch the reply landed on, and a missing identity reads as visible. It uses the same `preserveReadState` helper as the other two merge paths now, and the test that covered the other three fields covers this one too. A local cache merge was only inspected on a list's first page for new arrivals. A title or oldest-first sort leaves a replied row where it is, so its first reply on a later page went into the alerts baseline without an arrival and its chime or notification was lost. Local merges scan every loaded page; server responses keep the first-page rule, and only rows already known are considered. An unread request that matched nothing means the conversation is gone: the server's owner-scoped update returns the row even for a no-op. The mutation rolled back the read fields and kept the row; it removes it from the caches now, as a delete does. * fix: discard a job completion that resolves after the watcher has gone The fetch that settles a finished job had no teardown guard, unlike the away poll. Signing out or switching accounts unmounts the watcher, and a response still in flight for the previous session would then merge into the caches the next one reads. The result is dropped once the watcher has unmounted, tracked on unmount rather than in the jobs effect's cleanup, which re-runs on every change to the running set while its earlier completions are still wanted. * fix: move error-turn stamps into packages/api and settle overlapping unread no-matches Error turns were stamped in CJS: the error middleware checked eligibility, called the stamp and shaped the event's conversation snapshot itself, and the agents controller stamped its own. Both call announceErrorTurn now, and the skipped-persistence restamp goes through announceReply, so no stamp is issued directly from /api any more. The existing sendError tests pass against the real helper unchanged. An unread call that no longer owned the row dropped a server no-match with the rest of its stale answers, so a deleted conversation could be restored by a newer call's rollback. The no-match now evicts before ownership is consulted. It stays after the superseded check: a superseded call never reached the server, and its synthetic `modified: false` is not a no-match. Also: Settings offers the reply toggles only once the startup config has loaded, matching the capability hook; a cancelled reply-discovery snapshot is restarted even though nothing observes it; and the focused refresh is capped at the five minutes the discovery snapshot stays authoritative, since that refresh is what renews it. * fix: settle the assistant final in packages/api, announce attachment-only replies Both assistant controllers carried the same persistence barrier: write the response, check the row and the conversation both persisted, and unwrap the settled conversation for the final event. That is behaviour, and it was duplicated in CJS; settleAssistantFinal owns it now and the controllers call it. Their existing tests pass unchanged. The announce predicate ignored attachments, so a reply made only of files, such as a resumed workspace artifact with empty content and text, raised no dot, badge or alert. The message body renders attachments and acknowledgement checks the body, so such a reply can be cleared as well as announced; every announce path now passes them through. The stopped-reply announcement took the storage engine's own id type in its exported signature. It takes plain string ids now, and saveConvo accepts them alongside its own, since the update casts them against the schema; the id type stays inside data-schemas. * fix: normalize the stopped reply's written ids in packages/api The abort route filtered and stringified the ids of the rows it had written before handing them to announceStoppedReply. That is data shaping, and it sat in CJS; the announcement takes the ids as the caller holds them and drops the unwritten ones itself. * style: order the conversation row classes for Tailwind v4 The v4 upgrade on canary changed the class order Prettier enforces; these three rows carried the v3 order. * fix: replay replies a dead tab's focus lease held back, keep the focused refresh at a full page A tab killed without firing blur or pagehide leaves its focus lease for up to a minute. A reply arriving under it was baselined and then suppressed, and nothing reran when the lease lapsed, so its chime and notification were lost. Arrivals held back only by another tab's lease are kept and rechecked when that lease expires; a lease the focused tab keeps refreshing still holds them. The focused refresh reused the away-poll limit, so an operator who lowered it also shrank the discovery page that keeps the unseen count complete behind a filtered list. The limit now bounds only the away poll. * fix: announce a held reply as soon as the focused tab releases its lease A tab that blurs normally clears its lease long before the lapse, and a reply held back for it is due as soon as nobody is looking. Lease changes from other tabs now trigger the recheck, not only the expiry timer.
Nothing outside client/src/components/Files imported anything inside it: the four views at its root (FilesListView, FileDashboardView, VectorStoreView, FilesSectionSelector) are not routed and not referenced, so the whole subtree below them was unreachable. The live file manager lives under Chat/Input/Files and SidePanel/Files, and the FilePreview and ActionButton still in use are different files in those trees. Takes 49 eslint errors with it, all of them literal strings in screens no one could open. The client is left with 14, none in code this branch touches.
The primitives carried the weight the screens then inherited: boxed empty states, outlined table shells, raised composer pills. Softening them here means a screen gets the quieter treatment by composing what it already composes rather than by overriding it. Dark moves its border roles one step off the page instead of two, so an edge separates a surface from the one behind it without reading as a seam. The values live in style.css for the pre-hydration paint and in dark.ts for the runtime theme, so both change together. Checkbox gains CheckboxGlyph, the presentational half of the control, for rows that are already a button and must not nest a second one inside it.
Every management panel was building its own title row, so the title, the search field and the create button sat at a different size and spacing in each one. PanelHeader is that row, and the panels adopt it next. PanelContent gains a fade at its foot, fed by useScrollFade, which watches the viewport and its content and reports whether anything sits below the fold. A list that is exactly as tall as its box shows no fade, and the gradient is dropped under prefers-reduced-motion. The sidebar keeps a right edge in every theme now that it shares a surface with the content beside it, rather than only in high contrast.
The main content now sits on the same surface as the sidebar, so the app reads as one plane instead of a page floating on a backdrop. With the fill gone the containers had nothing left to justify them: cards, outlines and double borders come off the MCP, Skills, Bookmarks, Memory, Prompts and Scheduled Chats lists, and spacing and weight carry the hierarchy instead. The management screens adopt PanelHeader, so title, search and create sit at one size and one rhythm across all of them, and Schedules gains the search its siblings already had. Secondary row actions appear on hover and focus rather than resting in the row. Two rows also stop nesting interactive controls: the skills row splits into sibling buttons under a focus-within ring, and the memory and prompt toggles render CheckboxGlyph inside their button rather than a second control.
…sharing The list query takes five new facets. Dates and endpoints narrow it directly; hasFiles matches conversations that carry at least one file; sharedOnly resolves the user's live share links and matches their conversation ids, since a share expires and a denormalized flag on the conversation would outlive it. parseConversationListFilters owns what a valid facet is, so the route keeps the call and nothing else. A malformed date fails the request rather than being dropped: a filter that is quietly ignored answers with the conversations the user asked not to see. The endpoint list is bounded at 50 names of 128 characters to keep an unbounded $in out of the query. Two indexes back the new paths: user, archived, endpoint, updatedAt on conversations, and user, conversationId on shares. Cursor pagination is unchanged, so a filtered list pages the same way an unfiltered one does.
The old menu listed every option at once. It now opens on three rows that state where the list stands, Show, Sort and Filter, each with its current value and a submenu holding the choices. Reading the list's state no longer means reading the whole menu. Filter holds the five facets and a search field that looks inside them, so typing openai finds the endpoint rather than the category that contains it, and typing a category name offers everything under it. Reset sits in the header next to the title, and the active facet count rides on the trigger. The facets are one derived atom, so the query key and the request are built from the same description of the filter. Date cutoffs snap to local midnight: taken from the current instant they would differ on every render and refetch the list each time. The panels also fade their content at the foot when a list runs past the fold, and the chat list's loading label shimmers the way every other in-flight label in the app does.
A chat assigned to a project was listed twice in one sidebar: once under the project and once in Chats. The chats list now asks for the chats that belong to no project, which the list query and its cache helpers already knew how to answer. Search and the archived view keep asking for everything. Projects is not rendered while a search is on, and an archived chat has no project row to appear under, so excluding them from either would leave a chat with no way back to it.
5 tasks
berry-13
added this pull request to stack #16249
September 23, 2026 15:15
berry-13
marked this pull request as ready for review
September 23, 2026 16:42
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71f2c0e8ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+116
to
+120
| updatedAfter, | ||
| createdAfter, | ||
| endpoints, | ||
| hasFiles, | ||
| sharedOnly, |
There was a problem hiding this comment.
Adding these facets to allConversations query variants also makes every existing cache writer facet-sensitive, but conversationInsertVerdict and conversationBelongsToListQuery in client/src/utils/convos.ts still consider only project, archive, tags, search, and sort. Consequently, while an endpoint/files/shared/date filter is active, upsertConvoInAllQueries, duplication/fork mutations, or SSE updates can insert or retain a conversation that the server excluded—for example, an unshared conversation appears in a sharedOnly list after an SSE update. Treat these facets as server-reconciled predicates or teach the writers how to evaluate them.
Useful? React with 👍 / 👎.
Comment on lines
+94
to
+95
| const updatedAfter = rangeCutoff(get(updatedRangeAtom)); | ||
| const createdAfter = rangeCutoff(get(createdRangeAtom)); |
There was a problem hiding this comment.
When either date range is selected, this derived atom computes the cutoff only when one of its source atoms changes; Jotai caches the derived value rather than reevaluating it on every render. If the app remains open across local midnight, “Today” continues using yesterday's midnight and the other windows likewise grow by a day, even when React Query refetches on focus, until the user changes or resets the facet. Schedule an update at the next local midnight or derive the cutoff from a day-valued atom that advances then.
Useful? React with 👍 / 👎.
| }; | ||
| }); | ||
|
|
||
| export const resetFacetsAtom = atom(null, (_get, set) => { |
There was a problem hiding this comment.
These new atoms are global session state, but endSessionClientState in client/src/hooks/AuthContext.tsx resets only resetChatFilterSessionAtom; resetFacetsAtom is currently invoked only from filter UI actions. After logout or an in-app account switch, the next user therefore inherits the prior user's date, endpoint, attachment, and sharing filters and can land on an unexpectedly empty chat list. Wire this reset into the existing authentication-boundary cleanup.
Useful? React with 👍 / 👎.
| /** Which dead end this is decides both the line and the glyph above it: a search | ||
| * that found nothing, a filter that matched nothing, an empty archive, and an | ||
| * account with no chats yet are four different situations wearing one sentence. */ | ||
| let emptyLabel: TranslationKeys = 'com_ui_no_chats'; |
There was a problem hiding this comment.
Once the default request is scoped to projectId: 'unassigned', an empty response no longer proves that the account has no chats: it also occurs whenever every chat belongs to a project. In that common case the project rows are rendered above this state while the Chats section still displays com_ui_no_chats, contradicting the branch's stated “account is empty” meaning. Suppress this account-level state for the scoped list or use copy that explicitly describes the absence of unassigned chats.
Useful? React with 👍 / 👎.
|
|
||
| /** Searching reaches into the categories, so both lists have to be here rather than | ||
| * inside the submenu that normally owns them. Both are cached queries. */ | ||
| const { data: bookmarkData } = useGetConversationTags(); |
There was a problem hiding this comment.
For users whose role lacks BOOKMARKS:USE, showBookmarks is false and all bookmark UI is hidden, but opening the Filter submenu still runs this query unconditionally. The /api/tags router applies checkBookmarkAccess to every request, so these users receive a 403 (with React Query's default retries) merely for opening an otherwise permitted filter menu. Pass an enabled: showBookmarks option or mount the query only in the authorized branch.
Useful? React with 👍 / 👎.
danny-avila
added
the
🗺️ Chat UI Shell
This branch has not been deployed
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary
Part 4 of 6 of the quieter-layout stack, based on #16245 (
berry-13/convo-list-filters-api).The chat list filter listed every option at once. It now opens on three rows (Show, Sort, Filter), each with its current value and a submenu holding the choices. Filter holds the five facets from the previous PR plus a search field that looks inside them. Reset sits beside the title, and the active facet count rides on the trigger. The facets are one derived atom, so the query key and the request come from the same description, and date cutoffs snap to local midnight so the list does not refetch on every render.
A chat assigned to a project was listed twice in the sidebar: under the project and again in Chats. Chats now asks only for chats that belong to no project. Search and the archived view still ask for everything, so no chat is left with no way back to it.
Type of change
Testing
Tested environments/configuration:
canaryAutomated tests:
npx tsc --noEmitinclientandpackages/clientnpx eslinton every changed file, including theshadcn/*design rulesnpx jest --findRelatedTestsover the changed client files: 437 suites, 5,437 tests passing at the top of the stackScreenshots / recordings
Before is
canary; after is the top of this stack, so an image can also show changes from later PRs in the chain. Chromium, 1440x900 desktop and 390x844 mobile.Risk / compatibility
Depends on the list filters from the previous PR in the stack.
Checklist