mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
96 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
21ba9d3f30
|
🔁 fix: Rerun a Message the Editor Has Not Changed (#15212)
* fix: allow rerunning a message the editor has not changed The submit button in both message editors was disabled until the draft differed from the persisted message, so reissuing a request after a cancelled response, a failed generation, or a backend restarted on different parameters meant typing a throwaway character and deleting it first. The button now reads "Rerun" while the draft is untouched and "Update & rerun" once it differs, and neither state disables it. An untouched assistant turn regenerates instead of replaying its own content as an edit: editedContent retains the existing content and appends the new completion, so replaying it would return the old answer with a second one glued onto it. Two ask options were dead on arrival. editedText was declared on TOptions and passed by EditMessage but never destructured by ask, and isResubmission was never set or read anywhere. Since no submission can carry a text-level edit, and editedMessageId regenerates the row in place, an assistant turn in the plain-text editor now always reads "Rerun" and its status slot says where an unsaved edit is about to go. Fixes #15205 * fix: address PR review bot findings chatgpt-codex-connector: - Route a plain-text assistant rerun through regeneration. It kept the edit-resubmission options, so it replaced the response in place, and with no targetResponseMessageId the submission resolved the NEWEST answer for that turn: rerunning an older sibling pruned the wrong subtree from the optimistic thread while appending a placeholder keyed to the older sibling's own id. It now sends isRegenerate with targetResponseMessageId, matching the hover action and EditContentParts, and leaves the sibling index alone. - Let an empty answer reach the rerun handler. The field is registered as required so Save cannot blank a message, and routing the rerun through handleSubmit meant a response cancelled before its first token had an enabled button that did nothing. The answer's draft is never submitted, so it no longer gates the rerun or the disabled state. The status hint changes with the behavior: rerunning discards an unsaved answer edit and generates a new response rather than replacing this one. |
||
|
|
e9936b8ad2
|
🎢 fix: Restore Schedule Dialog Scrolling So Save Stays Reachable (#15225) | ||
|
|
1489623fa3
|
🎬 test: Record-Once/Replay-Forever Model Fixtures for Mock E2E (#15210)
* 🎬 test: Record-Once/Replay-Forever Model Fixtures for Mock E2E The mock e2e lane's only credential-free model is a hand-authored script: `fake-model.js` decides responses from ~60 `E2E_*` prompt markers. That covers scripted shapes well, but no scenario replays a *real* recorded provider conversation through the assembled chain, so real streaming shapes — provider chunk cadence, reasoning deltas, usage metadata — are only ever approximated. This adds a record-once/replay-forever tier alongside the marker routing. Record (`E2E_MODEL_FIXTURES=record`, needs a provider key): the run hook appends a LangChain callback handler to every agent context's `clientOptions.callbacks` instead of overriding the model, so the REAL provider streams while each invocation's `AIMessageChunk`s serialize to `e2e/fixtures/model-replay/<name>.jsonl` — text deltas, tool_call_chunks, reasoning kwargs, and genuine usage metadata. Only the latest human text is recorded for binding; system prompts and tool schemas never enter the fixture. Replay (default, keyless): `fake-model.js` consults `tryBindReplay` ahead of marker routing, binding a conversation whose prompt matches the next unconsumed invocation. The replaying model is not hand-assigned — it is registered as SDK provider `librechat-e2e-replay` via `registerProvider` and constructed through the SDK's own `initializeModel`, so registry lookup, constructor clientOptions, and real `bindTools` all run the way a live provider's would. Recorded chunks therefore stream through the same createRun → graph → SSE → persistence chain. Consumption is enforced rather than assumed: every invocation re-checks its prompt against the recording, an invocation past the end of the script throws, and a per-fixture ledger lets the spec assert at teardown that every recorded invocation and chunk was drained. Streaming incrementality is asserted from that ledger, not by sampling transient DOM, which is a race by construction. The credential-free profile is unchanged when not recording: the record provider and its selector entry are template markers that stay comments, and no existing spec's routing is touched (a fixture only binds on an exact prompt match; everything else falls through). Verified: record vs the real DeepSeek API 1 passed (15.2s); keyless replay 1 passed twice (11.4s, 12.7s) with the ledger fully drained (2/2 invocations, 10/10 chunks, no overruns or mismatches); app-load, completion, and chat 10 passed unchanged. * fix(e2e): rebind a replay fixture from the top for a new conversation The replay cursor is process-global while the web server outlives a Playwright retry, so a fully consumed fixture left the retry unable to bind its first prompt: it fell through to marker routing and failed deterministically, burning every configured CI retry. A partially consumed attempt failed the same way. Binding now restarts the fixture when the incoming prompt matches its first recorded invocation, resetting the ledger with the cursor so the new attempt is judged on its own consumption instead of accumulating the previous one's counts. Continuing an in-progress binding still outranks restarting, so a fixture whose opening prompt repeats later in the script advances rather than rewinding. The over-consumption guard is untouched — it fires inside the stream when the cursor passes the end, not at bind time. * fix(e2e): close three replay-lane gaps found in review Restart the recorder on a retry. Its state is process-global like the replay cursor, so a failed attempt that had already recorded invocations left the counter advanced: the retry appended 2/3 after 0/1, or kept the previous attempt's `error` line, and the fixture was unusable for replay. Recording now truncates and restarts when the opening prompt reappears, mirroring the replay side's rule and its caveat. Retain a consumed binding for the conversation that drove it. An extra user turn past the final recorded invocation found no next invocation and fell through to ordinary fake-model routing, so it was answered with a mock reply: the over-consumption guard never ran and the already-drained ledger still passed. Such a conversation is now recognized by its human turns opening with the fixture's recorded prompts, and stays bound so the stream raises the overrun. Continuing an in-progress binding still outranks restarting, which outranks retaining a consumed one, so a retry's fresh conversation rewinds rather than being read as an extra turn. Validate the fixture the recording actually wrote. Record mode honors `E2E_MODEL_FIXTURE_NAME`, but the spec always inspected the committed `deepseek-two-turn`; another name wrote elsewhere while the assertions read the pre-existing file, and because the prompts are fixed the stale answers could match and green a run that verified nothing it produced. * fix(e2e): make replay binding correct for tool and subagent fixtures Round two's retry and consumed-binding fixes both assumed one model invocation per user turn. A turn that calls a tool breaks that: the model is invoked again after the tool result under the same latest human message. Identify a retry by the conversation boundary, not the prompt. The recorder ran per invocation and truncated whenever the opening prompt reappeared, so a tool round trip looked like a retry and discarded the recorded tool-call invocation. Restart detection now sits in `installRecorder`, which runs once per `createRun`: a turn whose history holds no prior human message begins a conversation. Compare consumed bindings against user turns, not invocations. Several recorded invocations can share one prompt, so a one-to-one comparison could not recognize the originating conversation — invocations `[A, A, B]` against history `[A, B, C]` failed on both length and elements, and the extra turn fell through to the fake model with the drained ledger still passing. Fixtures now carry their collapsed turn sequence. Override the subagent model too. `graph.overrideModel` is not inherited by child executors, so a fixture recording a subagent call — record mode captures child invocations already — would leave the child on its configured provider: an underrun, and a real provider request in a lane that must stay keyless. Reject ambiguous prompt matches. Binding order followed filesystem enumeration, so a second fixture sharing a prompt could silently redirect a scenario to the wrong chunks and ledger; the spec's choice never reaches the server-side loop, so ambiguity fails instead of picking a winner. Fixture identity is the file name for the same reason — a recorded `meta.name` is descriptive, and trusting it let a copied fixture collapse onto another's registry key and ledger. Prove the recording is fresh. The spec removes the selected fixture before driving, so a run whose hook never installed the recorder fails instead of greening against a stale artifact whose answers still match these deterministic prompts. * fix(e2e): rewind a replay fixture at the conversation boundary Consecutive invocations can share a prompt — a tool call produces exactly that — so an attempt stopping mid-turn left the cursor on an invocation whose text still equalled the opening prompt. Matching the cursor first meant a retry's fresh conversation resumed after the tool call instead of rewinding, consuming the post-tool invocation and silently replaying a different script than was recorded. A conversation boundary now outranks an in-progress cursor: a fresh conversation whose prompt opens the fixture rewinds even when the cursor would have matched. Continuing still outranks restarting within a conversation, so a turn that calls a tool advances to its post-tool invocation rather than rewinding on its own repeated prompt. * fix(e2e): refuse cross-conversation binding and prove content streaming A fresh conversation could steal a partly consumed fixture's later turn. Only a conversation opening with the fixture's first prompt was treated as a boundary, so after `[A, B]` had consumed `A`, an unrelated new conversation whose first message was `B` matched the cursor, received the recorded second-turn response, and advanced the shared cursor without ever having driven `A`. A conversation start may now only rewind a partly consumed fixture, never continue it; continuation within a conversation is unaffected. The incrementality assertion counted empty frames. Providers emit empty initialization and usage-metadata chunks around the content deltas, so a total chunk count above one was satisfied by a single delta: the previous fixture's closing turn had four chunks and one content-bearing delta carrying the whole answer, and both modes stayed green without proving incremental assistant-content streaming at all. Fixtures now track content-bearing chunks separately, the closing prompt asks for prose rather than a number, and both modes require several content deltas on that turn. Re-recorded: the closing turn now carries 28 content deltas. * fix(e2e): scope record mode to the fixture spec `E2E_MODEL_FIXTURES=record` replaces the fake-model hook globally, so an unfiltered entry point such as `npm run e2e:mock` sent every spec under specs/mock to the paid real-provider endpoint, while each fresh conversation truncated and rewrote the one selected fixture — leaving an artifact from whichever scenario happened to run last. Record mode now matches only the fixture spec: an unfiltered recording run lists one test instead of 203. Replay mode is untouched and still collects the full suite. * 🪪 fix: Bind Replay Fixtures by Conversation, Not Prompt Text Prompt text was standing in for conversation identity, and three review rounds found the same class of defect underneath it: a tool call repeats a prompt across invocations, a retry repeats it across attempts, and a resumed run has neither prompt nor history because `createRun` is rebuilt with no messages while state comes from the checkpoint. Each fix in that space created the next gap. Thread the identity instead. `createRun` accepts a `conversationId` and passes it to the run hook, which the agents controller supplies at both call sites — the same value it already uses as the checkpointer's `thread_id`. The field is optional and the hook is env-gated, so nothing changes when the harness is not in use. Binding then collapses to ownership. A fixture is owned by the conversation that claimed it, and its cursor is authoritative wherever it stands: an extra turn reaches the over-consumption guard rather than falling through to the scripted fake model, and a resumed run keeps replaying with no prompt to match. A different conversation may claim the fixture only by opening it, which rewinds — what a Playwright retry looks like. Everything else is refused, so an unrelated conversation can no longer continue someone else's partly consumed script by repeating a later prompt. The prompt is still re-checked on every real turn; only a resume, which structurally carries no human message, is exempt. The previous text-and-history rules remain as a fallback when identity is absent. The recorder keys the same way: a new attempt is a new conversation, so a resume no longer truncates the fixture mid-turn and discards its tool-call invocation. Record summarization too. The summary provider runs on its own model with its own callback list, so a scenario crossing the context-pruning threshold recorded the agent's invocations but not the summariser's, leaving a fixture that could not reproduce the pruned context. * 🧾 fix: Harden Record Mode and Make the Rendered-Text Assertion Honest CI caught what local runs had not: the committed fixture was never replayed locally, because the record run overwrote it after the replay check rather than before. Re-recording and replaying in that order is what surfaced the rest of this. The DOM assertion compared raw recorded text against rendered markdown. The previous answer opened with `52.`, which Markdown renders as an ordered-list marker, so those characters never appear in the DOM and the match failed on all three CI attempts while replay itself was correct. The closing prompt now asks for prose beginning with a word, a leading enumerator is stripped before matching, and only a prose prefix is compared. Derived configs discarded the record-mode restriction. `config.redis.ts` and `config.mermaid.ts` spread this config and then replace `testMatch`, so `e2e:mock:redis` in record mode would still send its specs to the paid provider. A restriction expressed as an overridable value cannot hold, so record mode now refuses any config but the mock one. Superseded recording callbacks could write across a reset. A failed attempt with a provider call still in flight keeps its handler on the old graph; after the retry reset, that call would allocate an invocation from the new counter or append an `error` entry with a cleared mapping. Handlers now carry the recording generation they were installed for and ignore everything from an older one, and attachment dedupes against the current generation so a graph carried across a restart is not left with an inert handler. * 🚧 fix: Make Summarization an Explicit Boundary, Not a Half-Feature Recording summarization invocations without replaying them is worse than ignoring them. Replay routes the agent model and subagents only, so a recorded summarization entry takes a slot in the fixture sequence that replay never consumes, and the next primary call reads the summariser's chunks — a prompt mismatch or, worse, silently wrong content. The attachment was also aimed at the wrong shape: the SDK reads `summarizationConfig.parameters`, not `.parameters` nested under `.config`, so the previous attempt would have attached to nothing in a real run. Its test passed only because the test built the shape the code expected rather than the shape the SDK provides. Rather than ship a half-routed feature, recording now fails the moment summarization runs, naming the reason. Both shapes are guarded so the guard cannot miss the way the recorder did. Summarization fixtures need replay routing for that model before they can be supported. The derived-config guard added alongside it was itself broken: workers do not carry `--config`, and the argument lookup fell through to `process.argv[0]`, so every recording run aborted claiming the node binary was an unexpected config. The flag is now located explicitly and absence is treated as "not the process that parsed the CLI". * 🔒 fix: Close the -c Config Alias and Pin the Recorder's Fixture Name Playwright documents `-c` as an alias for `--config`, so record mode launched as `playwright test -c e2e/playwright.config.redis.ts` slipped past a guard that recognised only the long spelling. Both spellings and both `=` and space forms are now parsed. Accepting arbitrary fixture names also worked against the ambiguity check. This spec drives one fixed prompt pair, so recording under another name left two fixtures sharing those prompts; replay then refused to bind either and the keyless lane stopped working — a successful documented recording run could disable the suite it exists to serve. The spec now records only the fixture it owns and says so when asked for another. * 🔧 test: Record a Real Tool-Call Turn and Replay It Through the Tool Node The fixture format carried `tool_call_chunks` and the binding advanced through a turn's invocations, but nothing had recorded a real tool-calling conversation end to end — the path was covered only by hand-written synthetic fixtures, and it is the first one a new scenario would exercise. This records one: the provider calls the `remember_fact` MCP tool, the tool runs, and the model is invoked a second time with its result. That is the shape a single prompt cannot express — one user turn spanning several model invocations, all sharing one prompt — so it is what proves the turn-vs-invocation distinction the binding rules were built around. Replay drives the real tool node rather than replaying its output, so the tool executes again and the assertion checks its live result. Two fixtures now coexist, which the record path had to grow for: the config keeps an allowlist so an unknown name is still refused, record mode collects every replay spec, and each spec records only the fixture it owns and stands down for the others. MCP tools reach the model under a server-qualified name (`remember_fact_mcp_e2e-memory`); that qualification has changed before, so the assertions match the base name as a prefix rather than pinning the suffix. Verified: record 1 passed (15.6s, real API) then replay 1 passed (13.6s) against that fixture, ledger drained 2/2 invocations and 35/35 chunks; both replay specs together 2 passed; app-load, completion, chat and mcp-ephemeral 12 passed. |
||
|
|
290b8664d9
|
🧾 feat: Track Authoritative Agent Event Outcomes (#15213)
* feat: track authoritative agent event outcomes * fix: isolate agent event outcome types * fix: declare agent event handler result * fix: simplify agent event status selection * fix: preserve authoritative event outcomes * fix: preserve terminal event evidence * test: use completed run-step envelope * test: scope deferred HITL question locator * fix: settle every agent event terminal path * style: sort terminal host action imports * fix: fence agent event terminal evidence * fix: recover agent event terminal settlement * fix: scope terminal retry hints by generation * fix: settle terminal host actions exactly |
||
|
|
e0d5e11cdf
|
⏱️ feat: Show Elapsed Time Under the Streaming Response (#15167)
* ⏱️ feat: Show Elapsed Time Under the Streaming Response A minimalist elapsed-time indicator (5s, then 1m 5s) occupies the footer slot the hover actions vacate while a response generates, anchored to a per-index submission-start timestamp so remounts (new-conversation id hydration, navigation) never reset it. The once-per-second tick is component-local state, so streaming rows never re-render on its account. * 🧭 fix: Keep the Original Elapsed Baseline When Reattaching a Stream Codex round 1: resume-on-load restamped the anchor at reattach time, so navigating away from a still-streaming conversation and back restarted the reading at 0s — the exact reset the atom exists to prevent. Resume paths now leave the anchor alone: a same-session return keeps its ask baseline, and a reload (atom empty) falls back to the indicator's mount time, which is what the stamp produced anyway. * 🪗 fix: Scope the Elapsed Timer to Its Own Generation, Localized and Spoken Codex round 2, all four findings: - The anchor is cleared on every terminal path (final, error, abort fallback), and resume-on-load only fills an empty one — so a run another client started never inherits a stale baseline, while a same-session reattach still keeps its original start. - The indicator additionally requires the newest sibling position: latestMessageId follows the selected branch, so a settled older sibling paged to mid-regeneration satisfied the latest+submitting gate and got a counting timer under settled content. - Visible digits now come from the shared run-step duration formatter (Intl.NumberFormat per locale), replacing the raw-number interpolations. - The compact reading is aria-hidden with a spoken 'N seconds elapsed' equivalent beside it, per the house duration-label pattern; still no aria-live, so the tick never announces. |
||
|
|
2ac7986947
|
🫂 fix: Route Subagent Activity Through the Chat Renderer (#15137)
* fix: align subagent activity with chat UI * fix: preserve subagent activity boundaries * fix: preserve subagent panel state semantics * fix: preserve subagent activity metadata * fix: preserve live subagent event metadata * fix: scope subagent phases by message step * fix: retire closed subagent message phases |
||
|
|
caa938fec6
|
🎬 test: Cover Detached Subagent Activity Lifecycle (#15117)
* test: cover detached subagent activity lifecycle * test: strengthen detached activity lifecycle gates * test: tighten detached activity assertions |
||
|
|
67b7b441b2
|
🛂 feat: Filter Model-Bound Content by Source (#14425)
* feat: introduce optional content protection seam * feat: enforce source-aware content filters * feat: complete source-aware content enforcement * test: activate skill file-text fail-close fixtures * fix: harden source-aware content filters * fix: harden model-bound content filtering * fix: preserve legacy filters and generated files * fix: inspect shared scalar metadata * test: align mocks with current dev dependencies * feat: add persisted content filter safeguards * feat: complete source-aware content filter enforcement * fix: move resume content preflight into TypeScript * fix: close content inspection edge cases * fix: harden content protection boundaries * fix: complete content protection safeguards * test: align persisted memory filter coverage * fix: reconcile content protection with current dev * fix: reconcile content protection with latest dev * fix: close content protection review gaps * fix: enforce source-aware provider boundaries * fix: preserve legacy PII preflight semantics * test: stabilize stored branch preflight fixture * fix: defer agent writes until protected model admission * perf: harden source-aware model-bound filtering * fix: canonicalize provider lineage before validation * fix: satisfy model-bound callback type checks * perf: Bound content protection filtering work * fix: Bound submission array traversal * fix: Stabilize bounded content snapshots * fix: Scope model-bound traversal overflows * fix: Preserve scoped content inspection * fix: Accumulate aggregate traversal scopes * fix: centralize content policy boundaries * test: align deferred tool policy context * test: align controller policy mocks * style: normalize content protection imports * fix: close content policy review gaps * fix: narrow active skill policy config * fix: address content protection review boundaries * fix: retain exact provenance overflow sentinel * fix: preserve literal and scoped provenance updates * fix: narrow persisted edit provenance * fix: isolate exact overflow attribution * fix: centralize stored prompt protection * fix: fail closed on incomplete transcript evidence * fix: align canonical transcript routing * refactor: centralize content policy preflights * fix: isolate upload policy error typing * style: sort policy preflight imports * refactor: centralize content policy boundaries |
||
|
|
199de92c51
|
⚡ perf: Warm Feature Catalogs in the Background After First Paint (#15047)
* perf: warm feature catalogs in the background after first paint Prompt groups and MCP server/tool queries no longer fire on the app startup path. A catalog warmup store releases them after first paint on browser idle, staggered with jitter so a fleet of clients does not burst the API all at once. Panels opened before warmup activates their catalog immediately and fall back to their existing loading states. The prompts list endpoints now also run their independent access lookups in parallel instead of in three serial rounds. * perf: gate MCP icon observers and re-arm warmup across sessions MCP icon/name observers mounted from rendered messages now wait for the warmup gate like every other server-catalog consumer, so conversations with MCP tool calls no longer pull the server list onto the first-render path. The warmup schedule resets on logout so a second login in the same tab warms on its own stagger instead of releasing every catalog at once. Panel mount activations now require a visible sidebar, since a persisted active panel stays mounted while hidden. * perf: void stale warmup callbacks and gate the agent panel tools query Reset now bumps a generation captured by every idle callback and its stagger timer, so callbacks pending across a logout can no longer release catalogs into the next session. The agent form's MCP tools query keeps its own readiness gate so a hidden persisted panel cannot pull the tools request ahead of its stagger once the server list resolves. * perf: reset warmup on Root unmount and honor the insights route collapse Root can unmount in the same render that flips authentication on logout, so the warmup effect now resets from its cleanup as well as the unauthenticated branch. Panel activations mirror UnifiedSidebar's panelExpanded condition, treating the insights route as collapsed instead of reading the raw sidebar atom. * test: re-expand the approval tool card the saved message remounts The helper opened the card once and then waited on its body. Saving the response swaps the placeholder message id for the persisted one, which rekeys every part in the turn: the card remounts collapsed, its body unmounts, and the output assertion waits out its timeout against a disclosure nothing is going to reopen. The redis transport lane pays a round trip per stream event, so its finalization lands late enough to catch the helper mid-assertion. Wait for the closing model turn before expanding anything, then re-open the group and the card on each attempt until the scoped output is on screen. * Update AgentPanelContext.tsx import order --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
|
6757c65a54
|
✨ feat: Context Gauge Hover Reveal and Breakdown Motion Polish (#15038)
Show the context breakdown on hover instead of click, shrink the gauge, open and close the popover with a scale-and-fade transition, ease the collapsible with decelerating open and accelerating close curves, render the Messages segment solid, and pair legend row hover with a dimmed meter via a new highlightId prop on SegmentedMeter. |
||
|
|
8c14f03432
|
🗂️ feat: Scope Scheduled Chats to Chat Projects (#15056)
* 🗂️ feat: Scope Scheduled Chats to Chat Projects Adds an optional chat-project destination to a schedule, plus the operator config to require one — or to pin every scheduled run to a specific project. Feature: - `chatProjectId` on the schedule row, accepted on create/update, projected on the wire, and carried into the run's conversation through the durable trigger envelope's `run` context. - `interface.schedules.requireProject` refuses schedules that are not filed under a project; `interface.schedules.projectId` pins every run to one project and implies the requirement. - Dialog gains a project picker (required when configured, a read-only row when pinned); the card shows the destination and the new disabled reasons. Invariants: - ONE resolver (`resolveScheduleProjectId`) decides the destination for the write handler, the fire path, and the wire projection alike, and an operator pin OUTRANKS the stored id in all three. Tightening the config therefore redirects — or stops — existing schedules instead of grandfathering where their runs land. A pin implies `requireProject` for the same reason: without it, a row created before the pin would keep firing with no project at all. - Create/fire precheck symmetry, mirroring `resolveAgentFireAccess`: a write this handler accepts is one the next fire also accepts. Any edit leaving a schedule ENABLED re-validates its EFFECTIVE (possibly stored) project, like the existing stored-agent and cadence-floor rechecks. A DISABLING edit skips the requirement, or a schedule auto-disabled for `project_required` could never be turned off. - Fire-time enforcement auto-disables rather than filing runs loose, matching agent_deleted: new `project_required` (requirement raised after creation) and `project_deleted` (gone, or pinned to a project this owner does not have) reasons, both refused BEFORE a billed generation is dispatched, and both advancing so a schedule can never wedge on the occurrence. - `computeCreateDigest` appends the field only when present, so a payload without a project digests byte-identically to one from before this change — a create retried across the upgrade still matches its own row instead of reading as key reuse. - Project reads are scoped to the owner, so ownership and existence are the same lookup; a read ERROR propagates instead of failing closed, so a Mongo blip retries the fire rather than auto-disabling the schedule. The trigger idempotency key hashes principal/event/target and never `envelope.run`, so the added run field cannot destabilize delivery identity. * 🗜️ fix: Keep the Schedule Dialog Inside Its Height Budget The project picker landed as a new ROW in the schedule dialog, which broke the e2e edit spec: `md:overflow-visible` turns off the template's scrolling from `md` up, so the dialog's content must fit the viewport. The extra row pushed the footer's Save button below a 720x1280 window, where Playwright reported a visible, enabled button it could never click — 226 scroll-into-view retries and a 2-minute timeout, on all three attempts. Measured against `dev` at 1280x720 (Save button's bottom edge, viewport 720): dev 688 (32px slack) project row ~790 (off-screen, CI failure) 3-column row 704 (16px slack — half the budget spent) this commit 686 (34px slack, 2px better than dev) The identity row is now three columns — name, agent, project — and its caption moved out of the agent cell to sit full width beneath the row: at a third of the dialog that sentence wraps an extra line, and the row is the tallest thing competing for the budget. The caption is grouped with the row rather than left to the form's own 4-unit rhythm, which spent more height on the gap than the caption occupies. The e2e spec now asserts the button is in the viewport before clicking it, so the next field that overflows this dialog says so in one line instead of a two-minute timeout on a visible element. FOLLOWUPS.md records what the planned dialog controls (multi-day weekly, timezone, attachments) need first: give `ControlCombobox` the `portalElement` prop `Dropdown` already has, portal the popovers into the dialog content, and let the form scroll again. * 🧹 fix: Address Codex Review on Scheduled Chat Project Scope Four P2 findings, all real. Unreachable clearing path (ScheduleDialog). The picker only held live projects, so `com_ui_schedule_project_none` was a PLACEHOLDER — nothing selectable. Once a schedule had a project the owner could never take it away, leaving the server's `chatProjectId: null` path reachable only by API. The picker now carries a real "No project" option whenever a project is optional, and omits it when one is required, where there is nothing valid to select. Placeholder shown for a real project (ScheduleDialog). A stored or pinned project outside the first loaded page had no name in the paged map, and the combobox renders its placeholder for an empty display value — telling the owner a scoped schedule had no project. That one project is now read by id, with the raw id as a last resort: a poor label, but an honest one. Project policy skipped at the resume boundary (service.ts). `claimScheduleResume` re-applied the schedules gate, the revision fence, the kill switch and SCHEDULES:USE, but not the project policy this PR added. Approving a paused run whose project was deleted — or whose owner now sits under a requirement or a pin it no longer satisfies — billed a continuation the very next scheduled fire would refuse and auto-disable the schedule for. The effective-project resolution now runs there too, refused before the lease and the capacity slot so a policy refusal costs nothing and leaves no state to unwind. NOTE: agent access and balance are still not rechecked on resume; that gap predates this PR and is left alone. Per-card project derivation (ScheduleCard). Every card ran the projects hook and rebuilt the full option array, name map, and one icon element per project, to use a single name — O(schedules x projects) per render and per project-list refresh. The hook is split: `useChatProjectNames` (map only, for the panel, which resolves every card's name once and passes it down) and `useChatProjectPicker` (options and pagination, for the dialog's one combobox). The panel skips the query entirely until some schedule actually has a scope. Tests: four at the resume boundary (verified failing without the gate) and four in the dialog spec. The picker selection in one existing test now goes through the search field — the popover's VIRTUALIZED renderer sizes its window from a scroll height jsdom always reports as 0, so with three options it materialized only two. Full schedules e2e re-run green against the rebuilt client. * 🎯 fix: Settle Project-Policy Refusals and Keep Create Retries Idempotent Second Codex round, four P2s. Three were consequences of the resume gate added in the previous commit, which was half-built: it admitted where it should not and stranded the run where it refused. Project policy moves from `claimScheduleResume` into `isScheduleLive`'s `policy` branch. Both entry points consult that branch FIRST, and both already route its refusal through abort-and-settle — so a policy stop now settles the occurrence instead of answering a bare 409 while the job stays `requires_action`, the card keeps reading "Needs approval", and every retry repeats the same 409 until expiry. No change to resume.js: the existing branch does the work. The rule is deliberately NARROW. It refuses only where no valid destination is left — the requirement is on with nothing satisfying it, or the schedule's own project is gone (which also unset it on the conversation). It does NOT refuse because an operator's pin moved: the paused conversation cannot be rebound (`chatProjectId` is excluded from the resume context and the continuation reuses the same conversationId), so refusing would strand a pending approval over a destination it can never reach, for a pin that governs only where the NEXT run lands — which the fire path already redirects. Create retries are idempotent again. Project policy had been applied BEFORE the `clientRequestId` replay lookup, so a raised requirement, a deleted project, or a moved pin could answer 400 for a create that already committed — pushing the client to rotate its key and create a DUPLICATE schedule, the exact failure the key exists to prevent. Policy now applies only to a genuinely new insert, and the digest is computed from the CLIENT's payload rather than the resolved destination, so today's policy can no longer re-digest a genuine retry into a mismatch. An explicit `chatProjectId: null` under a pin is refused rather than silently resolved to the pin. The payload contract defines `null` as clearing the scope; answering 201 while filing under the pin reported success for the opposite of what was asked. Only an OMITTED field takes the pin silently. Tests: five on the policy branch (both refusals verified failing without it, plus guards that a moved pin and a live project still admit) and two on the handlers (the pinned explicit clear, and a committed create recovered by retry after the policy tightened — verified answering 400 without the reordering). * 🧭 fix: Converge the Stored Project on the Destination a Fire Resolved Third Codex round, four P2s. The root confusion behind the resume findings: an operator pin outranks the stored id at fire time, `fireSchedule` sends the pin in the trigger envelope, and the row keeps its old value. The row therefore LIED about where that occurrence's conversation went, and every later re-validation — the resume boundary above all — checked a project the conversation was never filed under. A schedule storing A, pinned to B, with B later deleted and A still live, was admitted for resume into a conversation that had just been unscoped. Fixed at the source rather than at each reader: a fire that resolves a destination different from the stored one writes it back, claim-token fenced like every other worker-side write and deliberately WITHOUT a configRevision bump — this is the server reconciling itself to policy, not an owner edit, and a bump would fence an in-flight occurrence off its own run. Written only AFTER the destination validates, so an unusable pin never lands in the row, and best-effort: the envelope already carries the right destination, so a failed write costs accuracy on a later recheck, never the run itself. The wire projection already reported the pin, so this also stops the row and the UI disagreeing. An explicit `chatProjectId: null` under a pin is now refused on the DISABLING edit path too. The pin check and the requirement are independent rules, and folding them together let `{enabled: false, chatProjectId: null}` skip the pin check entirely, unset the row, and answer with a wire projection still naming the pin. Only the requirement is waived for a disabling edit. The dialog no longer requires a project for an edit that leaves a schedule DISABLED. The server waives the requirement there precisely so a row auto-disabled for `project_required` can still be renamed or tidied up; requiring it in the form made that unreachable, and an owner with no projects could not edit the stopped schedule at all. FOLLOWUPS.md records the two residual gaps with their exact triggers: the sub-second deletion race inside the resume claim window (which needs the effective project persisted per OCCURRENCE plus a distinct policy conflict routed through abort-and-settle), and the fact that convergence happens only when a schedule fires. Tests: four on convergence (pin written, no write when unchanged, no write for a destination that failed validation, fire survives a failed write), two on the disabling-edit pin rules, two in the dialog. Schedules e2e re-run green. * 🔑 fix: Keep an Explicit Project Clear Out of an Omitted Field's Digest `computeCreateDigest` appended `chatProjectId` on `!= null`, so an OMITTED field and an explicit `null` produced the same digest. Because the replay lookup and `matchesCreateIntent` deliberately run before project policy, a request could reuse a pinned create's `clientRequestId` while explicitly sending `chatProjectId: null` and receive 201 describing the pinned row — success reported for the opposite of what it asked, and the pinned-clear refusal the normal create path applies never reached. Now `!== undefined`: an omitted field still digests byte-identically to a payload from before project scope existed, so a create in flight across the upgrade still matches its own row, while an explicit clear is a distinct intent and digests differently. A pre-scope client never sent the field at all, so nothing legacy can carry an explicit null. * 📍 fix: Validate a Paused Run Against the Project Its Own Occurrence Used The schedule-wide convergence from the previous commit was not enough, and the reason is the single-active run index: it covers `status: 'started'` only, so a PAUSED run does not block the next occurrence. While run 1 sat paused in project A, a pin move plus one later fire rewrote the schedule row to B — and the resume policy then validated B while run 1's conversation was still filed under A. Delete A and the continuation was admitted into a conversation that had just been unscoped. That window lasts as long as the pause, not the sub-second race the previous commit documented. The reservation now records the destination THIS occurrence used, and `isScheduleLive` validates that record when given the occurrence's `scheduledFor` — which `resume.js` already reads two lines above the call. No new conflict type and no settlement-path surgery: the refusal rides the abort-and-settle branch that check already has. An ABSENT record falls back to the schedule-level resolution rather than reading as unscoped. A pre-scope occurrence, or one whose row is gone, must never be treated as evidence to stop a run — the fallback keeps legacy paused runs behaving exactly as they do today. Schedule-wide convergence stays: it keeps the row honest for the UI and for every check that has no occurrence in hand. FOLLOWUPS.md now describes the one remaining gap accurately — a deletion inside the claim window, which needs a distinct policy conflict routed through abort-and-settle rather than the bare 409 an `inactive` conflict produces. Tests: three on occurrence-vs-row precedence (the decisive one verified failing without the lookup), two on what the reservation records, and the resume controller spec now pins `scheduledFor` in the policy call. Schedules e2e re-run green. * 🏷️ fix: Tell a Deliberately Unscoped Occurrence From an Unrecorded One The occurrence fallback added in the previous commit conflated two different absences. A post-upgrade run that deliberately went unscoped omitted the field exactly like a row written before the field existed, so both took the fallback — and a paused unscoped run was then validated against the schedule's CURRENT project. Under a requirement or a pin added while it sat paused, that admitted a billed continuation into a conversation satisfying no present policy. The reservation now ALWAYS records its decision, `null` for unscoped, and the read reports `recorded` from key PRESENCE rather than truthiness. Only an unknown record — a pre-scope row, or no row at all — falls back to the schedule-level resolution; a recorded null is the genuinely unscoped occurrence it says it is, and is refused once a project becomes required. The distinction rests entirely on a stored `null` surviving as a present key while a never-written field stays absent, so that is asserted against real Mongo rather than assumed: if it ever stopped holding, unscoped runs would silently start being validated against the schedule's current project again. Tests: two against mongodb-memory-server (recorded null vs never-written vs missing row), plus refusal of a recorded-unscoped occurrence under a new requirement and the preserved fallback for a pre-scope one. Schedules e2e green. * 🔁 fix: Validate an Initial Scheduled Start Against Its Own Occurrence The initial-start policy check in `request.js` called `isScheduleLive(..., { policy: true })` without `scheduledFor`, so it fell back to the schedule-level resolution even though the run row already carries the occurrence's recorded scope. An occurrence reserved unscoped, with a pin introduced while its loopback request sat queued, was therefore admitted against the new pin — producing a billed unscoped conversation under a requirement it does not satisfy, from an envelope already built without a project. `scheduledFor` was already in scope there. Passing it makes the initial start and the resume validate the same way: against the destination the occurrence itself recorded. |
||
|
|
81783b2d52
|
🧲 refactor: Consolidate Claude Prompt Cache and Context Checks (#15008)
* 🧭 fix: Unify Future Claude Capabilities * 🐛 fix: Cover Claude Capability Edge Cases |
||
|
|
c5276fc63d
|
⏱️ feat: Run Scheduled Chats Through Durable Agent Triggers (#14939)
* feat: Scheduled Chats — agent-centric scheduled runs creating real conversations
Squash of the full review-hardened branch (PR 14540, supersedes 14373) onto
latest dev, preserving the exact verified tree. History prior to this commit
lived on the pre-squash branch; every invariant below survived 25 Codex review
rounds plus two external audit rounds (R26) with regression tests that fail
without their fixes.
Feature:
- Schedules CRUD + side-panel UI (cadence dialog, run cards, Run Now), roles/
permissions (SCHEDULES:USE), interface.schedules availability, per-user limits
and capacity slots, timezone-aware cadence with DST-conservative floors and
misfire grace.
- Engine: single-process claim/fire loop with leases, loopback POST dispatch
(signed schedule-fire JWT claims, per-occurrence idempotency key inside the
route's clientRequestId charset), overlap/balance/capacity/duplicate skip
policies with auto-disable streaks (too_many_failures, insufficient_balance),
reconciliation from retained terminal-job evidence, erasure sweep.
- Scheduled runs create real conversations through the resumable agents chat
path: HITL pauses surface on the card (requires_action), resumes re-apply the
fire boundary's admission policy (revision fence, enabled, global kill switch,
SCHEDULES:USE, availability) before continuing a billed generation.
Correctness invariants (the audit surface):
- Settlement discipline: every persistence-producing write happens-before a
run's terminal outcome write; Stop/complete/pause race through single-winner
terminal CAS claims (dev's TerminalJobClaim substrate) with retained,
completedAt-less evidence for scheduled fires plus an owner-intended outcome
stamp (scheduleOutcome) the reconciler prefers over re-derived success —
round-tripped through the Redis hash mapper.
- Swallowed generation failures (client error content parts) classify to
error/skipped_balance instead of success on both initial and resumed paths;
stale stamps are refreshed evidence-first when persistence plus the Mongo
outcome write both fail.
- Abort honesty: delivery judged by generation ownership and the CAS's actual
from-status; republication escalates to the transport's acknowledged variant;
the Stop route settles only genuinely paused runs.
- Account deletion: one-way barrier (deletionRequestedAt) with auth-cache
tombstone-before-stamp, boundary rechecks across all auth strategies, quiesce
of scheduled + interactive work with durable per-stream abort fences
(positive-evidence acknowledgement only), owner-side finalization markers for
post-terminal billed writes, deferred-deletion sweep (explicitly ensured
partial index) that completes cascades autonomously. Remote OpenAI-compatible/
Responses requests are documented as outside the quiesce and tracked in issue
14594.
- Store compatibility: finalization markers optional on the legacy IJobStore
contract with coherent degradation (registration fails -> synchronous title
fallback; count reads 0).
* fix: fence terminal response persistence from deletion; retain stale-pause evidence
Two blockers from the third external review round.
Terminal persistence visible to account deletion:
- The finalization marker was registered only when post-terminal TITLE work was
possible, but every persistence-owning terminal CAS opens the same window: the
claim drops the job out of the active set BEFORE the response save (and the
background user-message/convo saves), so a deletion quiesce landing there saw
neither an active job nor a marker and could cascade while the admitted request
could still recreate messages. Both controllers now register the marker before
every persistence-owning claim — the fresh-turn path and the HITL resume — and
release it only after their pending saves (and any post-terminal title) have
landed, on success and failure paths alike. The TTL bounds a crash.
- settleAbortFence no longer clears a complete/error fence while
`terminalPersistencePending` is set: terminal at the CAS is not settled while
the owner is still persisting. The job facade now surfaces the flag.
- The marker trio is REQUIRED by the runtime store contract (assertJobStoreV2
refuses a store without it at configure time, keeping the failure loud and
deterministic) while remaining optional on the legacy public IJobStore type for
source compatibility. The silent degrade path from the previous round is gone —
it was not deletion-safe.
Stale-pause recovery retains scheduled evidence:
- Three crash/timeout recovery paths — ApprovalLifecycle.failStalePausePersistence
and the InMemory/Redis stale-pause cleanups — unconditionally stamped
`completedAt`, putting a scheduled fire's failed-pause error terminal on the
short completed TTL. A Mongo outage longer than that TTL erased the evidence and
the reconciler recovered the run as `interrupted` instead of `error`. All three
now follow the controller-observed path from the previous round: scheduled jobs
omit `completedAt` (retained-evidence TTL) and stamp the error outcome.
The PR description now explicitly narrows the deletion guarantee for the remote
OpenAI-compatible/Responses paths (tracked in issue 14594).
* fix: deletion-fence marker protocol — generation-scoped, atomic, fail-closed, all terminal paths
One consolidated pass over the finalization-marker mechanism, per the fourth
external review round. The invariant it establishes: NO persistence-owning
terminal CAS runs without a durable, generation-qualified marker covering the
window it opens, and every consumer treats a pending terminal as unsettled.
- Generation-scoped markers. Entries were keyed (userId, streamId), so a
Stop-superseded generation finishing late could clear the marker its
replacement registered on the same conversation. Marker fields are now
qualified by the generation's createdAt; clears must present the same
identity, and an unqualified legacy clear cannot drop a qualified entry.
- Atomic Redis registration. HSET-then-EXPIRE loses the fresh marker when the
user's existing hash expires between the two commands (or the process dies
there); registration is now a single Lua script carrying both.
- Fail closed everywhere. Registration failure (after one retry) now REFUSES
the terminal CAS instead of proceeding uncovered: the completion claim throws
into the error path, the error path skips completeJob and leaves the job
ACTIVE — deletion-visible by itself, recovered by the stale-running reaper —
and abortJob returns a new retryable `fence_unavailable` failure the Stop
route answers with 503 and the deletion quiesce treats as an unacknowledged
stop (fence kept). The previous round's log-and-proceed is gone.
- Every terminal path enrolled. abortJob now owns its window (register before
the abort CAS, clear in its finally — the Stop route's checkpoint prune and
partial save run inside beforePublish, between CAS and publication); the
interactive and background generation-error paths register before their
completeJob; the resume controller's error finalization registers before its
completeJob. A lost or thrown claim releases the marker after pending saves
flush instead of holding the user's deletion behind the TTL.
- Every pending terminal unsettled. settleAbortFence defers on
terminalPersistencePending for ALL statuses — including `aborted`, whose
route-side persistence the previous guard missed.
Also: a direct Redis regression for scheduled stale-pause retention (the P2
test gap), and the PR description no longer claims to carry every commit.
* fix: lease-token lifecycle fences — same-generation isolation, admission fence, undelivered-Stop retention, legacy abort enrollment
Fifth external review round; four P1s, handled as the requested consolidated
lifecycle-fence pass.
- Lease tokens. Marker fields were (streamId, createdAt), shared by every
contender on the same generation — completion and Stop, or two racing Stops —
so a losing contender's cleanup erased the winner's still-live marker. Every
registrant now carries a unique lease token in the field and may only ever
clear its own lease; unqualified legacy clears cannot touch qualified entries.
- Admission fence. Authentication can pass before the deletion barrier goes up,
and the durable createJob is several async steps later — a deletion quiesce in
that window saw neither an active job nor a marker and could cascade before
the admitted request created its job. The controller now registers an
admission lease and THEN rereads the deletion barrier: the ordering guarantees
either this request observes the barrier (403, lease released, slot/claim
cleanup) or the quiesce observes the lease and defers. Held until createJob is
durable; released on every refusal and initialization-error path. Fail closed
when the lease itself cannot be registered (503 retryable).
- Undelivered-Stop retention. abortJob released its lease in a finally even when
delivery AND publication had provably failed — the job reads terminal
(invisible to active-set scans), a user Stop writes no durable abort fence,
and the remote owner keeps generating and will persist its abort-catch writes
whenever the signal finally lands. The lease is now retained in exactly that
case, and each resignal attempt heartbeats a fresh lease so the fence outlives
the TTL for as long as delivery is still being driven. The abort-winning
turn's own loser-side pending saves are additionally fenced in the controller
catch (best-effort — those writes are already in flight).
- Legacy abort enrollment. abortMiddleware (assistants abort route fallback for
non-assistants endpoints) awaited abortJob and then spent usage and saved the
stopped response AFTER the abort's lease was released. Both writes now run
inside `beforePublish`, between the abort CAS and publication, covered by the
same lease as every other abort.
Barrier tests, each verified to fail without its fix: same-generation lease
isolation (store), racing two-Stop loser cleanup (manager, stale-read forced
CAS race), undelivered-Stop lease retention, resignal heartbeat, admission
refusal with lease-before-reread ordering plus release-on-durable-create, and
legacy-abort persistence inside beforePublish.
* fix: heartbeat-backed owner-lifecycle leases close the settlement handoff races
Sixth external review round: the remaining P1 interleavings were one structural
problem — lease handoffs that were not atomic — resolved as the requested
consolidated lifecycle-lease pass.
- Quiesce reads leases BEFORE the active-job scan. The admission-lease -> durable
-job handoff is only atomic against a reader in the OPPOSITE order of the
writer: writers hold the lease strictly until the job is active-set visible,
so leases-first shows every interleaving either the lease or the job.
Jobs-first allowed a request to create its job after the scan and release its
lease before the count — hiding both, cascading, and letting the new
generation persist into a deleted account.
- The abort acknowledgement is fenced by the owner-lifecycle lease. Redis ACKed
the moment the owner's AbortController tripped; the stopping side released its
lease on that ACK while the owner's asynchronous abort-catch persistence was
still ahead. The transport now awaits a manager-installed pre-ACK hook that
registers a DETERMINISTIC owner lease (exactly one owner exists per
generation, and determinism is what lets the signal-time registrant and the
owner's catch-side release agree across processes) before the acknowledgement
is persisted or published; an owned same-replica abort bridges to the same
lease before tripping its local controller. Both generation-owner catches
(fresh turn, resume) release it once their writes land.
- A failed replacement handoff no longer orphans the predecessor. The atomic
replacement removes it from active storage, and an unconfirmed handoff
terminalizes the replacement too — leaving nothing a quiesce could discover
while the predecessor's provider may still be generating. Its owner lease is
now retained at the point the receipt fails delivery; the owner replica renews
it through the pre-ACK fence when the signal finally lands.
- Leases HEARTBEAT while held. The five-minute store TTL only bounds a crashed
holder; live persistence — a stalled save, a long deferred title — must never
outlive its own fence. holdUserFinalization registers and renews every minute
until released; the controllers' completion/error/admission leases all hold.
The undelivered-Stop retention moved to a deterministic `stop` lease that
every resignal attempt renews and the first successful one clears (no more
opaque leases accumulating to TTL), and a THROWN abort transition releases the
contender lease instead of leaking it.
- The user-document abort-fence mutations now invalidate the auth user-doc
cache, matching every other user-doc write.
Barrier tests, each verified to fail without its fix: quiesce lease-scan
ordering (plus the observed-lease defer), pre-ACK fence ordering at the
transport, owned-abort owner-lease bridging, replacement-handoff predecessor
retention, held-lease heartbeat past the TTL, and failed-then-successful
resignal reaping the retained stop lease.
* fix: one manager-owned owner-lease span across every abort delivery path
Seventh external review round; four lifecycle-fence gaps, closed by making the
owner-lifecycle lease a single manager-owned, heartbeat-held span.
- Fail-closed acknowledgements. The pre-ACK hook registered a one-shot lease and
the transport ACKed even when it failed; a same-replica owned abort likewise
swallowed registration failure. The hook now acquires a HELD owner lease
(heartbeat until the owner's catch releases it via releaseOwnerLease) and a
rejection SUPPRESSES the acknowledgement — the stopping side stays retryable
behind its retention lease, and every resignal re-drives the handler. The hook
also stopped gating on `job.createdAt === generationId`: during a replacement
handoff the store holds the replacement while the abort targets the
predecessor, and that gate silently skipped exactly the generation being
acknowledged (the store job is owner identity, never a generation gate). A
local owned abort acquires the same held lease before tripping its provider;
post-CAS the trip cannot be withheld, so acquisition failure downgrades
delivery and the retention handoff keeps the user fenced.
- Committed-but-lost-reply disambiguation. A thrown abort transition released
the contender lease as if nothing had happened, but a Lua CAS can commit and
lose its reply — an aborted job invisible to active-set scans whose provider
was never signalled, with no fence left. The throw path now re-reads the exact
generation: only a job still live under the caller's identity proves no
commit; committed or ambiguous outcomes hand the fence to the deterministic
stop lease (kept on the contender lease if even that fails) before rethrowing.
- Replacement handoff covered end to end. A LOCAL replacement abort acquires the
predecessor's held owner lease before the trip (failure reports the receipt
undelivered, engaging retention). Failed-handoff retention is no longer a
swallowed one-shot: it heartbeats with the durable acknowledgement proof as
its renewal predicate — acquisition failures keep retrying for as long as the
fence is needed, and the retainer stands down (without clearing the shared
field) once the remote owner ACKs and thereby holds its own lease.
- A LOCAL resignal delivery hands off to the owner lease BEFORE clearing the
retained stop lease, and keeps the stop lease when that handoff fails.
Barrier tests: hook rejection suppressing the ACK, commit-then-lost-reply
retention with its provably-uncommitted counterpart, local-resignal owner
handoff ordering, pre-ACK owner lease held past the store TTL until release
(and provably stopped after), and failed-handoff retention retrying on its
heartbeat — verified fail-before/pass-after by stashing the fixes.
* fix: finish scheduled chat lifecycle hardening
* fix: close scheduled chat review follow-ups
* fix: generation-fence abort recovery evidence
* test: wait for settled approval tool output
* test: preserve scheduled init reconciliation option
* refactor: rebuild scheduled chats on durable agent triggers
* test: reset MCP cache mock between cases
* test: isolate scheduler startup in server specs
* fix: harden scheduled run lifecycle
* fix: normalize schedule capacity conflicts
* test: type schedule collision fixture
* fix: re-fence scheduled resume and expiry
* fix: fence scheduled resume handoffs
* fix: release superseded manual schedule leases
* fix: release failed run-now claims
* fix: release superseded engine claims
* fix: repair schedule dialog interaction and rework its form
The agent picker was unusable: ControlCombobox portals its popover to the
body by default, which lands it outside the dialog's Radix focus trap. Clicks
passed through it, it could not be tabbed into, and the trap fighting Ariakit
for focus locked the page up on selection. The prop is documented for exactly
this case — pass `portal={false}` and give the dialog `overflow-visible`, as
ProjectButton already does. The time and day dropdowns defaulted the same way.
Alongside that:
- Extract the agent builder's instructions editor (special-variable menu plus
expand-to-fullscreen) into a controlled `VariableEditor` and use it for the
schedule prompt. Insertions now route through `onChange`, so react-hook-form
sees them — the schedule PATCH is built from `dirtyFields`, and a `setValue`
that skipped dirty tracking would have dropped an inserted variable silently.
- Replace the hand-rolled frequency buttons with the shared `Radio`. They marked
the selection with `bg-surface-hover` on an outline button whose hover is the
same token, so the selected option was indistinguishable from a hovered one;
`Radio` is also a real radiogroup rather than four `aria-pressed` toggles.
- Wrap the fields in a real `<form>` and associate the footer button by id, so
Enter submits. Group the frequency, day and time controls in fieldsets.
- Add placeholders for name and prompt, match the textarea fill to the other
fields, and label the hourly case as minutes past the hour.
- Widen the dialog to `md:max-w-3xl` and pair name with agent so the form fits
without scrolling on desktop.
- Move scheduled chats below skills and above prompts in the side nav.
The new dialog spec fails when the portal fix is reverted.
* test: cover scheduled and subagent deletion drains
* refactor: own the form-control appearance in the client primitives
Addresses the codex finding on ScheduleDialog: a feature-local `FIELD_CLASS`
restated the `Input` primitive's border, radius, height and background so it
could be pasted onto the schedule dropdowns, leaving those controls with no
connection to the primitive they were imitating.
Move that appearance into `packages/client/src/components/Field.ts` as the
single source `Input` and `Textarea` now compose, and give `Dropdown` and
`ControlCombobox` a `variant="field"` that applies it. The schedule dialog
passes the variant and carries no class strings of its own.
This also repairs a break the dev merge would otherwise have introduced: the
newer `Dropdown` splits `className` (wrapper) from `triggerClassName`, so the
old pasted classes would have landed on the wrapper and left the triggers
unstyled.
The semantic-token guard now watches the shared module and asserts each
primitive still composes it, which covers more than the two files it read
before.
* fix: keep an explicit schedules disable from becoming an opt-in
`use` is two things at once for a dual-purpose runtime interface field: a
permission bit, which DB overrides strip, and the runtime disable signal that
`getLimits` reads. Stripping it from `{ use: false, maxPerUser: 2 }` leaves an
object, and `getLimits` treats any object without `use: false` as enabled — so
an admin override written to stop scheduled billing for a role or user started
it instead.
Collapse an explicit disable to the boolean form before the strip, on both
paths that accept it: the `interface.schedules` field patch, which admitted the
object wholesale because bare runtime paths deliberately bypass the permission
gate, and the overrides merge, which reached the composite-field branch and
kept `maxPerUser`. Objects that only narrow limits are untouched, so a
principal can still be given a smaller cap.
Both regressions fail without the normalizer.
* fix(schedules): Wave A — null-balance CAS, atomic paused-card clear, clustered erasure sweep
Slice 1 (thread r3804518381): route the existing-null balance initialization
through a { user, tokenCredits: null } compare-and-set instead of a blind $set,
so a concurrent initializer/charge landing between the preflight read and the
write is never handed back its spent starting balance. On a CAS miss the
preflight re-reads the winner. The absent-record $setOnInsert path and the
credited-record refill-config sync are unchanged. Adds the initializeNullBalance
adapter (no upsert) and regression coverage for winner/miss/sync cases.
Slice 2 (thread r3804518388): updateScheduleById now drops a `requires_action`
lastRun projection atomically with the configRevision bump. Any pause present at
edit time was projected under the pre-edit revision and can never be replaced by
its own revision-fenced terminal outcome — a disabling edit would strand the
card on "Needs approval" forever. Implemented as classic-operator CAS branches
(DocumentDB rules out a conditional pipeline $unset), fenced on the card STILL
being the pause so a terminal outcome or newer occurrence that races in is
preserved. Terminal history survives untouched.
Slice 4 (thread r3803826204): expose initializeScheduleErasureSweep from the
schedule runtime facade and start it in every clustered (experimental) worker
after Mongo is up. It re-drives eraseScheduleIfDrained for soft-deleted rows so
a hidden prompt cannot outlive its drain when the delete/erase-on-settle
attempts miss. It arms nothing else and never infers owner death from a
process-local missing job (isTopologySafeToArm gates that).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.1 — reversible account-deletion schedule suspension
Thread r3804518383. Account-deletion quiesce marked every schedule `deleting`,
disabled it, and cleared nextRunAt destructively. When a later cascade step (or
the drain itself) failed, the controller cancelled the user-deletion fence —
restoring the user — but the schedules stayed `deleting` and were erased by the
sweep, silently losing all of a live user's scheduled prompts.
Replace the destructive marking with a REVERSIBLE, token-fenced suspension:
- suspendUserSchedulesForDeletion(userId, token) snapshots each schedule's prior
enabled/nextRunAt under a per-attempt token, then fences firing (disable, clear
nextRunAt, rotate claimToken). It never sets `deleting`, so a suspended row is
not erasure-eligible. Snapshotting reads then bulkWrites (a classic update
cannot copy field values under DocumentDB), fenced per row so an already-
suspended/soft-deleted/edited row is left alone; idempotent per token.
- restoreUserSchedulesFromDeletion(userId, token) reverses it, re-enabling and
re-arming only rows still carrying the exact attempt token and not independently
deleted — so an owner-deleted or newer-attempt-suspended schedule is never
resurrected.
- deleteUserController generates the attempt token, passes it to quiesce, and on
any failure that cancels the user-deletion fence restores the suspended rows. A
successful deletion hard-deletes them (and their snapshots) via the existing
cascade and never restores.
Adds a `deletionSuspension` embedded field (excluded from the wire schedule),
data-method regression tests (suspend/restore/fence/idempotency/no-resurrect),
and controller tests (restore on drain-false and post-quiesce cascade failure,
no restore on success).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.2 — wire the interactive Stop persistence protocol
Thread r3804255932. The durable Stop primitives (requestRunAbort 'stop',
getScheduleRunAbortState, markRunAbortPersisted) already existed but production
only used requestRunAbort(..., 'deletion'). An interactive Stop flipped the job
to `aborted` and then persisted its partial message + checkpoint inside
`beforePublish`, without ever stamping the schedule Stop or acknowledging it —
so reconciliation, the generation owner, or a concurrent schedule/account
deletion could terminalize the run and release its capacity (and erase data)
mid-write.
Wire the request -> persist -> acknowledge -> settle barrier through the
schedule runtime (the route never touches raw Mongo):
- Expose beginScheduledStop / acknowledgeScheduledStopPersistence on the service.
- The abort route stamps the Stop BEFORE signalling abortJob (a serialized
'in_progress' loser returns 409 STOP_IN_PROGRESS without a second abort),
acknowledges only after beforePublish persistence succeeds, and on a
persistence failure leaves the barrier unresolved so the run stays preserved
(client retries; stale-owner timeout is the bounded recovery). A failed abort
releases the stamp it placed so a replacement/retry is never blocked through
its predecessor.
- recordScheduleOutcome (the owner settlement path) now waits, bounded, for the
Stop acknowledgement before terminalizing; a resolved/non-stop/stale marker
proceeds immediately. The paused Stop settles only after its own ack.
Adds service-layer barrier tests and route-level ordering/persistence-failure/
in-progress tests; the data-layer serialization is already covered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave C.1 — reconcile durable trigger delivery with the reservation
Threads r3803826192 (manual limiter) and r3804255924 (PII/moderation), plus the
independently-found long-Retry-After race. fireSchedule reserves a `started` run
and a global capacity slot BEFORE the durable trigger delivery reaches the chat
route, where an interactive limiter (manual Run Now), PII, or moderation can
reject it before any generation job exists — dead-lettering the delivery while
the run sat `started` until the 30-minute orphan sweep mislabeled it interrupted.
And a valid delivery deferred by Retry-After (up to 24h) could be orphan-settled
and have its capacity released, then fire anyway.
Translate durable delivery state into the schedule outcome:
- Store the deterministic trigger deliveryKey on the ScheduleRun reservation
(computed from the envelope BEFORE enqueue, so an ambiguous commit still has it).
- Add a getTriggerDelivery engine dep (wired to the merged trigger service's
getDelivery) that reads the durable delivery by key.
- Schedule reconciliation, for a jobless `started` run: staging/pending/leased →
admission is live, never orphan; dead → record `error` from the durable
lastError and release capacity promptly (no 30-minute wait), through the
ordinary outcome/auto-disable path; succeeded or no record → the existing
legacy orphan policy (interrupted only past the cutoff); a delivery lookup
failure defers rather than orphaning a possibly-live delivery. Limiter/PII/
moderation middleware writes no schedule state.
Adds reconcile state-mapping tests (dead/pending/leased/staging/succeeded/none/
lookup-failure) and a fire test that the reservation's deliveryKey equals the
enqueued delivery's idempotency key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): Wave C.2 — durable retry/ack for terminal host lifecycle actions
Thread r3804518375. Approval expiry won the `requires_action → aborted` CAS and
then invoked the host hook best-effort: `runApprovalExpiredHandler` swallowed a
failure, and because later sweeps enumerate only `requires_action` jobs, the now-
aborted job was never offered again. In the clustered entrypoint (no schedule
reconciler) the ScheduleRun stayed `requires_action` and its retained job
persisted indefinitely.
Make the host lifecycle work durable rather than schedule-specific:
- Add a generic `terminalHostActionPending` marker, set ATOMICALLY in the same
terminal transition (ApprovalLifecycle.expireWithIdentity), only when a host
adapter is installed.
- Retain and index such jobs: both stores keep them out of terminal reaping and
expose getTerminalHostActionJobs(); Redis adds a set + extended (24h-bounded)
TTL, in-memory a bounded 24h retention so a permanently-failing hook cannot leak.
- The manager clears the marker only after the adapter acknowledges success,
fenced by generation identity (clearTerminalHostAction), so a replacement
generation can neither clear nor execute its predecessor's action.
- cleanup()/expireStaleApprovals() enumerates unacknowledged terminal host actions
across restarts and replicas and retries the idempotent hook; the relay only
re-invokes while the marker is unacknowledged, so a successful ack prevents
duplicate work. Store-won expiry marks it too, so a loser-replica relay still
crosses the hook.
- Terminal SSE notification continues regardless of host-hook outcome.
Covers in-memory behavior (retry after failure, restart/other-replica retry, ack
prevents duplicates, identity fence, terminal notification on failure, no marker
accumulation for non-scheduled jobs) and updates the Redis cluster-membership
contract test for the new index.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* style(schedules): satisfy import sorting in fire.ts and fire.spec.ts
CI "Static checks" failed on IMPORT_SORT for the two files Wave C.1 added imports
to (the AgentTriggerEnvelope type import and getAgentTriggerIdempotencyKey).
Applied scripts/sort-imports.mts to exactly those files — imports-only reordering,
no behavior change. Other files reported by a repo-wide check are pre-existing on
dev and deliberately left untouched so this PR is not widened.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): repair the deletion CLI and order restore before the fence release
Addresses three findings from the fresh Codex review.
P1 — config/delete-user.js called methods.disableUserSchedulesForDeletion, which
Wave B removed in favor of suspendUserSchedulesForDeletion. The file is
@ts-nocheck and its spec mocked the removed name, so neither typecheck nor tests
caught it; the real CLI would throw a TypeError before deleting anything and then
only unwind the fence. The CLI now uses the tokenized protocol: it mints a
suspension token, suspends with it, and restores that exact attempt's rows in its
finally block when the deletion does not commit. Its spec mocks the real methods,
so the breakage can no longer hide.
P2 — both the HTTP controller and the CLI released the user-deletion fence BEFORE
restoring schedules. That fence is what refuses new schedule writes/claims, so the
gap let an owner PATCH edit a still-suspended row and have its enabled/next-run
state overwritten by the older snapshot, and let a second deletion attempt
re-suspend under a new token — making the first restore a no-op and stranding the
disabled snapshot permanently. Restore now runs first, while writes are still
fenced.
Hardening for the same defect class across a crash: suspendUserSchedulesForDeletion
now ADOPTS an existing suspension's snapshot when re-suspending a row abandoned by
an earlier attempt, instead of re-capturing the row's current (already-suspended)
state. Without this, an attempt that died before restoring would have its
successor snapshot "disabled, no next run" and permanently strand the schedule.
Tests: CLI restore-before-fence ordering and no-restore-on-success; the same
ordering assertion on both controller post-quiesce failure paths; a data-method
regression that a second attempt adopts the abandoned snapshot and restores the
original enabled/next-run state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge dead deliveries in topology-safe maintenance
Codex finding: the `dead` delivery mapping added in Wave C.1 lives only inside
startScheduleEngine's reconciler, but the clustered entrypoint arms no engine — it
runs erasure-only maintenance. A delivery queued before a restart into clustered
mode and then rejected before generation creation (interactive limiter, PII,
moderation) dead-letters while its ScheduleRun stays `started`, holding a global
capacity slot indefinitely for an ordinary non-deleting schedule.
Add a dead-delivery convergence pass to the erasure sweep, so every topology that
runs schedule maintenance settles it. The pass is POSITIVE-EVIDENCE-ONLY and is
therefore safe where absence-based reconciliation is not: a `dead` delivery is
durable shared state proving no generation owns the reservation. It settles only
when the job is confirmed absent or identity-mismatched (an identity-matched job
still owns the run), defers on an unknown job lookup, on an in-flight abort, and
on an in-flight resume hand-off, ignores legacy reservations with no deliveryKey,
and applies a short grace so an accepted delivery still creating its generation is
never settled mid-handoff. Auto-disable policy is deliberately left to the armed
engine; this path records the failure and frees the slot.
Deliberately does NOT touch api/server/experimental.js — the clustered entrypoint
already starts this sweep, so the convergence arrives through the existing
initializer and the shared-file footprint stays as-is.
Tests: settles a dead delivery as error under an explicitly UNSAFE topology,
leaves live deliveries alone, never settles under an identity-matched running
generation (delivery is not even consulted), defers an in-flight abort, and
ignores a reservation with no deliveryKey.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): refresh host-action retention on each retry attempt
Codex finding: unacknowledged terminal host-action evidence was capped at the
24h pause TTL, so a host dependency (Mongo) unreachable for longer than that let
the Redis key — and its pending marker — expire with no generation-fenced
acknowledgement, stranding the ScheduleRun where no reconciler is armed.
Measure retention from the LAST retry rather than from the terminal transition:
enumerating a pending host action IS the retry attempt, so both stores refresh
its retention as they hand it to the hook (Redis re-EXPIREs the job key; the
in-memory store stamps terminalHostActionRefreshedAt and bounds from it). Evidence
therefore survives as long as some replica is still actively retrying, while a
deployment that stops sweeping entirely still lets it age out — so this does not
reintroduce the unbounded leak the cap existed to prevent.
Test: after a failed hook, a later cleanup pass keeps the marker pending and moves
its retention basis forward.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): defer settlement when the Stop barrier times out
Codex finding: waitForStopPersistence returned after its 5s poll budget even when
the Stop was still fresh and unacknowledged, and recordScheduleOutcome then went
straight on to terminalize the run — releasing its capacity, deletion, and erasure
barriers while beforePublish may still have been writing. Slow checkpoint cleanup
is indistinguishable from a dead route on that signal, so the timeout was being
treated as if the barrier had been satisfied.
The poll budget now means "undecided", not "clear". On timeout with a fresh,
unacknowledged Stop the barrier DEFERS: recordScheduleOutcome returns false
without recording, leaving the run active/preserved. Settlement then happens
either when the route acknowledges, or once the existing stale-owner cutoff
(ABORT_OWNER_PRESUMED_ALIVE_MS) authorizes a later attempt — which the loop
already treats as clear-to-settle. Callers with durable retry (the approval-expiry
host action, reconciliation) re-drive it, so a deferral converges rather than
stranding the run.
Test: a fresh Stop that never acknowledges within the budget reports not-settled
and records no outcome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): require definite delivery failure, extract its message, converge deferred Stops
Third Codex round. Three findings in code from this closeout, plus one pre-existing
P1 that is a one-line operator-safety fix.
P1 — dead-delivery settlement demanded too little evidence. `dead` does not prove a
request was rejected: the trigger host marks response timeouts and invalid success
responses `certainty: 'ambiguous'`, and the engine dead-letters those once retries
are exhausted. The erasure sweep treated every dead letter as positive evidence, so
an ambiguous one sitting over a generation a peer had accepted could terminalize the
run and release its capacity mid-flight. It now settles only on a DEFINITE rejection,
unless job absence is deployment-authoritative (safe topology), where the
confirmed-absent job is itself the evidence.
P1 — `lastError` is an `AgentTriggerDeliveryFailure` object, not a string. A
duplicated local interface declared it `string` (against CLAUDE.md's no-duplicate-
types rule), so both the sweep and the engine reconciler passed the object into the
String-typed run/schedule `error` fields; Mongoose would reject the cast, the per-row
catch would swallow it, and the run would keep its global capacity slot. The dep type
now reuses the canonical `AgentTriggerDeliveryFailure` and both call sites pass
`.message`. Re-typing immediately surfaced a stale test that had asserted a string.
P1 (pre-existing) — the base-config global stop is honored in `getLimits` via
`isRuntimeDisabled` rather than a literal `=== false`. The stop has two shapes, and
deepMerge turns base `{ use: false }` plus a principal override of `true` into
`{ use: true }`, so the literal check reported the feature enabled and Run Now
dispatched straight through fireSchedule, bypassing the operator's emergency stop.
Now the same predicate the engine gate already uses.
P2 — a Stop whose settlement DEFERRED past the poll budget had no convergence path
where no reconciler is armed. `acknowledgeScheduledStopPersistence` now optionally
re-drives the terminal outcome once the barrier clears; `recordRunOutcome` is
match-guarded and idempotent, so an owner that already settled makes it a no-op. The
abort route passes it for a running generation; a paused job still settles explicitly.
Tests: ambiguous dead letters refused under unsafe topology but settled when absence
is authoritative, definite rejections settled either way, the failure message carried
through, and the abort route's re-drive present for running / absent for paused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge terminal runs in clustered workers, retry suspension restore
Fourth Codex round on
|
||
|
|
27ed491a2a
|
🏷️ fix: Persist the Ephemeral Agent's Display Label as Sender (#14899)
* 🏷️ feat: Add getEphemeralSender and Cover the Ephemeral-Id Format * ♻️ refactor: Consolidate the Ephemeral Sender Chains * 🏷️ fix: Decode the Ephemeral Sender for Persisted Messages * 🏷️ fix: Mirror the Persisted Sender Chain in useGetSender * ✅ test: Widen the Custom-Endpoint Fixture Type * ✅ test: Expect the Spec Label in the Composer Placeholder * 🏷️ fix: Resolve the Sender from Exact Labels, Not the Lossy Id |
||
|
|
1b7e2a4e6a
|
⚡ perf: Optimize First Load of Large Conversations (#14901)
* ⚡ perf: Index the Conversation Fetch and Trim the Client Message Projection * ⚡ perf: Memoize the Message Tree per Cache Write * ⚡ perf: Serve Message Reads via the Trimmed Projection and an Ownership Probe * ⚡ perf: Defer Collapsed Disclosure Bodies Until First Expansion * ⚡ perf: Progressively Mount Long Threads from the Scroll Anchor * 🩹 fix: Address Codex Findings on Retention, Anchoring, and Cache Bounds * 🩹 fix: Poll the Oversized Export Precondition Through the Progressive Mount * 🩹 fix: Keep Video Results in the Client Message Projection |
||
|
|
e1ac7d2bda
|
⚓ ci: Settle the E2E Reply Before the Double-Click Quote (#14840)
* test(e2e): grant MULTI_CONVO.USE in the mock e2e config
`agent-skills-added.spec.ts` drives the composer's `+` command, which opens the
added-model popover. That path is gated on MULTI_CONVO.USE:
if (!hasMultiConvoAccess || !plusCommandEnabled || isAssistantsEndpoint(endpoint)) return;
The mock config never sets `interface.multiConvo`, so the permission falls
through to the seeded role default and `handlePlusCommand` returns before
opening the popover. The spec then fails on a popover that is absent from the
DOM entirely, which reads as a selector or timing problem rather than a missing
permission.
Set it explicitly, the same way `contextCost` is set just above for the usage
gauge — the mock config's job is to make each exercised feature's gate explicit
rather than inherit a default.
* test(e2e): wait for the reply to settle before the double-click quote
`quotes.spec.ts` › 'summons the popup from a native double-click word
selection' double-clicks a word as soon as `mockReply` becomes visible. But
`sendMessage` resolves on the stream *response*, not on the final render, so the
reply can still be re-rendering.
A streaming markdown re-render swaps out the text node the selection points at,
which collapses the selection — the same mechanism the sibling
`selectionchange` test documents deliberately. A double-click landing mid-stream
therefore loses its selection before the popup can be clicked, and because the
whole gesture is wrapped in `toPass`, every retry re-runs into the same
still-streaming reply rather than recovering from a one-off.
This is a different race from the one #14777 fixed. That one is the *selection*
still settling (touch long-press, native handle drags, block-granularity
gestures) and is handled inside QuoteButton. This one is the *reply* still
streaming, which no amount of component-side settling can absorb.
Observed on a downstream fork running this suite on slower hardware: the test
fails all three attempts, deterministically on the in-memory stream store while
the Redis lane passes the same shard — the in-memory store's final re-renders
land late enough to outlive the gesture. Four separate runs, same split.
Wait for the reply text to hold steady before selecting.
* ci(e2e): install ffmpeg so first-retry video actually records (#14841)
`playwright.config.mock.ts` sets `video: 'on-first-retry'`, but the runner only
installs `install-deps chrome`, which does not include ffmpeg. Without it the
first retry fails inside `browserContext.newPage` while setting up video
recording — before the test body runs.
The cost is the retry itself: a genuinely flaky test loses the attempt that
would have recovered it, and the reported failure is a video-setup error rather
than the original symptom.
Bounded and non-fatal on purpose. The CLI has been observed hanging after the
download completes on these runners, so the step is wrapped in `timeout` and
its failure is swallowed — if ffmpeg cannot be installed the job proceeds
exactly as it does today, and no lane is blocked on it.
Applied to both jobs that run Playwright (`e2e_shards` and
`mcp_tool_list_changed`), since both configure retries.
|
||
|
|
530a935a74
|
🎨 feat: Color the Context Gauge by Category and Collapse its Breakdown (#14855)
* 🎨 feat: Color the Context Gauge by Category and Collapse its Breakdown The context window bar becomes a stacked meter — one hue per category — and the breakdown collapses behind a disclosure so the gauge alone is the default view. The collapse choice persists per user. Adds a categorical series scale (`rgb-series-1`…`rgb-series-7`) to the versioned theme registry, so themes and `REACT_APP_THEME_SERIES_*` can retint it. Hues are anchored on LibreChat's own brand tokens; every step was computed rather than picked, by enumerating slot orderings and snapping each step until all gates passed in both modes: worst adjacent CVD ΔE 12.4 light / 13.0 dark (target 8) worst adjacent normal-vision 19.0 light / 19.0 dark (floor 15) contrast all 14 steps ≥ 3:1 on both the popover surface and the meter track Slot order is the colour-vision-deficiency safety mechanism, not cosmetics. Reserved status colors are never reused for series identity, and the circular composer gauge is deliberately untouched — it answers "how close am I to the limit", which stays a status question. - `SegmentedMeter` + `MeterSwatch` land beside `Progress` in `@librechat/client`, owning the 2px surface gaps, rounded ends, the min-width floor, and the hatch. The category-to-slot mapping stays feature-local: the palette is theme data, the mapping is not. - Every present category gets a 2px floor so a 251-token row cannot render as 0.09px; the shortfall comes out of free space, never another category. - Deferred tools keep their family's hue and add a 135° hatch, so a hue never means two things. Segments are reordered to put each deferred pair beside its parent, which is also the adjacency the palette was validated on. - Messages is drawn as a translucent fill with a solid edge: it is the only category the user grows, and the form difference doubles as secondary encoding. - A row carries a swatch if and only if it is a segment. The estimate path knows the total but not the composition, so it keeps a single unsegmented fill. - Usage totals gain a "Totals" heading, and row text lifts to primary ink on hover/focus. - The popover widens 256px → 288px to absorb the chevron and the legend swatches. Guardrails: the series scale is held to the 3:1 mark floor on both surfaces, the app CSS defaults are held in step with the runtime themes, and each slot is asserted to resolve to a Tailwind utility backed by its CSS variable. * 🐛 fix: Address Codex Review on the Segmented Context Gauge Three P2 findings, all confirmed. **Gaps inflated the fill.** Segment widths were percentages of the whole track while `gap-[2px]` was added on top, so the gaps ate into the free-space remainder instead of living inside the filled region. Measured on the real component: a window at 47.2% painted 55.6% full, and the bar read full at ~94%. Each segment now surrenders its share of the gap budget, so fills plus gaps span exactly the used fraction. Same case now paints 50.2%. The residual 3.0pp is the `SEGMENT_MIN` floor doing its job — five sub-pixel categories rounded up to 2px each. That overshoot is deliberate and bounded, it comes out of free space rather than a neighbouring category, and the doc comment now states the magnitude instead of leaving it implicit. **No reference-theme test.** The suite only exercised the bundled token tables, so it could not detect the shared component becoming coupled to LibreChat's values. Adds a deliberately different reference `ThemeDefinition` and asserts the registry accepts it, the values reach the applied CSS variables, and every rendered mark takes its colour from those variables — no literal colours in the tree. `SegmentedMeter.tsx` also joins the shared-primitive colour guardrail. **Series tokens missing from the public maps.** `IThemeVariables` and `IThemeColors` are exported for downstream consumers to type their CSS-variable and Tailwind maps, and would have rejected the new keys. Adds the series entries to both, plus a compile-time guard in the registry so a slot added to one map and missed in another fails the build. The guard deliberately lives in `registry.ts`, not the spec: `tsconfig.json` excludes `*.spec.ts`, so an assertion there is never checked by the build — verified by removing a key from each map in turn and confirming the error. * ✅ fix: Expand the Breakdown in the Context Gauge e2e Specs `e2e/specs/mock/usage.spec.ts` asserts on rows that now sit behind the disclosure, so four tests failed on the collapsed default. My miss — I updated the component spec and never grepped for e2e coverage. `openBreakdown` now expands the detail after opening, so every caller that reads a row keeps working; the helper is idempotent, since a reload restores an already-expanded preference. The one inline `gauge.click()` that duplicated the helper now uses it. Adds the case the regression should have been caught by, and which only e2e can reach: the popover opens to the gauge alone with no detail mounted, expanding reveals the labelled Totals section, and the choice survives a real reload through localStorage without a second click. `e2e/specs/real/usage.spec.ts` reads the totals the same way. It also hovered rather than clicked, which never opened the popover at all — hover surfaces only the compact snapshot tooltip, as the mock spec asserts. |
||
|
|
af7e890b14
|
🐛 fix: Give the Header's Sidebar Toggle Its Own Test Id (#14850)
The header now branches on CSS instead of `useMediaQuery`, so its mobile `OpenSidebar` stays mounted at every breakpoint. The sidebar rail already publishes `open-sidebar-button` for its own collapsed toggle, so both held the id at once and `getByTestId` resolved to two elements. Scope the header's copy to `header-open-sidebar-button` and assert the count in the spec that broke — the click there failed only once the header had mounted, so a count assertion pins the collision deterministically. |
||
|
|
5d3edeb383
|
🪄 feat: Smooth Activity Phase Transitions (#14832)
* feat: Animate activity phase transitions
* style: Match activity phase formatting
* 🪄 fix: Fold activity phase entrance in one direction, flush-left label
The phase header replaced <summary> with <button>, which brought the UA
`text-align: center` with it — the label span is `flex-1`, so the text
filled the row and centered inside it. Left-align it and drop the leading
glyph: the card's border and fill already carry the weight, and the child
tool groups keep their own icons.
The entrance also read as two movements. The card, header and inset all
hard-cut in at full size, displacing the transcript below by ~57px, then
folded back up past the header that had just pushed it down. The card now
mounts in the shape of what was already on screen — zero-height header,
transparent chrome, no inset — and grows the header as the panel collapses,
so the block's height only ever decreases. Chrome, padding and both heights
share one curve.
The collapse also waits for a painted start value; a single rAF can land
before paint, and a start value the compositor never saw snaps rather than
transitions.
- Restore the e2e parent-phase selectors, which still matched `summary`
- Memoize the hoisted `groupActivityPhases` pass and its phase-index set
- Finish the amber -> `text-text-warning` sweep in ToolCallGroup and Part
* 🩹 fix: Scope phase-entrance history and resolve media queries at mount
Addresses both Codex findings on #14832.
`MultiMessage` renders siblings without a key, so `ContentParts` survives a
sibling switch with its refs intact. The recorded phase-marker set outlived
the message it described, and any phase in the newly selected sibling whose
index was absent from the previous sibling's set was read as a live arrival —
already-loaded history mounted expanded and collapsed itself. Scope the set
to its messageId and treat a mismatch as a fresh mount.
`useMediaQuery` initialized to `false` and resolved only in a passive effect,
so the first render always reported "no match". Anything branching once at
mount — the frozen entrance flag here, and every other first-paint decision
across its call sites — never saw the correction, which is how a
`prefers-reduced-motion: reduce` user still got the fold. Read the query
synchronously in the state initializer and guard both paths for environments
without `matchMedia`.
* ♿ fix: Honor reduced motion on manual phase disclosure
The entrance already respected the preference, but manually opening or
closing a phase did not: `useExpandCollapse` writes its transition as an
inline style, which cannot carry a `prefers-reduced-motion` media query,
and there is no global reduced-motion reset in the stylesheet. Before this
PR the phase used `<details>`, which had no animation at all — so the swap
to an animated disclosure handed reduced-motion readers a 300ms fold they
did not have.
Resolve the preference in the hook and drop the transition outright. Every
expanding panel in the message content shares it, so tool calls, thinking
blocks, attachments and web-search sources are covered by the same change.
The chevron and the fold's own utility classes get `motion-reduce`
overrides, which the inline styles cannot express.
* 🩹 fix: Keep the collapse completion signal under reduced motion
`transition: none` emits no `transitionend`, and ToolCallGroup waits on
that event to drop `shouldRenderBody`. Removing the transition therefore
left every collapsed tool subtree mounted indefinitely — expensive and
stateful children retained for exactly the readers who asked for less
work, not more.
Shorten the duration to 0.01ms instead. It is imperceptible, still fires
the event, and keeps the hook the single place that knows about the
preference. Caught by Codex on
|
||
|
|
d4c64d485f
|
ci: gate the e2e activity-phase DOM assertions on the persisted phase (#14821)
`activity-phases` asserted the parent `summary` was visible immediately after `sendMessage` resolved. A parent phase only exists once the turn completes, the phase closes, and its summary round-trips to the phase-label model — so that assertion raced the entire pipeline and only survived on Playwright's retries. It shows as `1 flaky` on the memory lane of a green dev run, and fails all three attempts on slower hardware. Gate the DOM on the durable projection instead. The test already fetched /api/messages twice; the first fetch now also waits for the persisted phase part before any DOM assertion runs, so the client is only asked about a phase the server has already written. Also drops the duplicate fetch. The two poll blocks queried the same endpoint for the same message and both asserted `finalTextIndex === activity_end_index`; the removed copy left `liveAssistant`, `livePhase` and `liveFinalTextIndex` shadowing their durable equivalents. No coverage removed — every assertion is preserved, reordered to follow the dependency chain: persisted shape, then DOM, then label-model requests, then the reload round-trip. |
||
|
|
d920328bfa
|
💬 style: Unify Message Row Layout and Edit Surfaces (#14770)
* style: Unify message row layout and edit surfaces Route chat, share, and search messages through a shared MessageRow so user turns render as right-aligned bubbles and assistant turns keep a visible identity column. Replace per-part text editors with one edit surface that keeps tools, errors, and artifacts visible. Preserve non-text fields when saving content parts, copy the full serialized message, and hide hover actions that do not apply during streaming or errors. * style: Align edit footer and lighten editor field in dark mode Drop the divider above the user edit footer so both edit surfaces share the same footer treatment. Move the editor fields to surface-tertiary-alt. Light mode is unchanged at #fff, while dark mode lifts from #0d0d0d to #2f2f2f so the field sits above the #212121 panel instead of sinking into near-black. * style: Drop focus border and ring from message editors The editor fields changed border color and added a ring on focus. Keep the border static and rely on the app-level focus handling instead. * fix: Keep a triggered message action visible when the row is not hovered Hover actions fade out on non-last rows, and mobile.css only restored display and visibility for an active button, never opacity. Opening the fork popover therefore left it anchored to an invisible trigger once the pointer left the row. Skip the fade entirely while a button is active. Extract the recipe the three toolbars repeated so the rule has one home. Rework the streaming guard to the contract the toolbar now implements: edit and fork are omitted from a streaming response rather than rendered disabled, and the settled turn above keeps its own actions. It asserted the removed disabled-and-transparent behaviour and its opacity check only held because the growing response shifted the row out from under the pointer. * style: Trim message edit chrome and stabilize the status row The edit surface was a titled card sitting inside the conversation: a bordered panel with an "Edit message" heading wrapping bordered fields, which read as a settings dialog rather than an inline editor. Drop the card background, border and heading, and take the footer buttons down to the small size so the editor reads as a field in the message flow. The captured row goes from 253px to 187px. Move "Unsaved changes" into the footer and merge the rerun hint into the same slot. Both previously added their own row, so typing pushed the rest of the conversation down. The slot is clamped to two lines, which stays under the 36px button row, so the footer height holds at 36px regardless of which message is showing. * test: Cover message edit layout stability Add a mock e2e spec that measures the edit footer and section boxes and asserts they hold steady as the status text appears, for both the single-part user editor and a multi-part response. The multi-part case needs an assistant message with two editable parts, so add an E2E_THINK_REPLY marker to the fake model. Its think tags are parsed downstream by the agents stream pipeline, which yields a reasoning part followed by a text part. * fix: Read the fork popover open state from its store Fork mirrored the popover state into its own useState and reset it from an onClose prop. Ariakit 0.4 has no onClose, and React's DOM types accept the name on any element, so it type-checked, landed on a div and never fired. Closing by Escape or an outside click therefore left the button reading as active until the trigger was clicked again. Read the state from the store instead so every close path clears it. * fix: Keep the whole toolbar visible while an action is open Only the triggered button escaped the hover fade, so opening the editor or the fork popover left the row as a single floating button once the pointer moved away. Mark the active button and have every action in the toolbar key off it, so the group stays opaque for as long as a surface is open. The marker is a dedicated class rather than the existing `active`, which HoverButtons pins to the edit button of every assistant message and would hold those toolbars open permanently. The existing guard pressed Escape to close the editor while focus sat on the body, so the editor never closed and its assertion only held because the sibling faded regardless. Close the editor through its own control, and drop focus before measuring the fade now that Escape returns it to the trigger. * fix: Withhold copy while a response is still streaming Text-to-speech, fork and feedback were all withheld from a message that is still generating, but copy was rendered throughout, so the button offered to put half a sentence on the clipboard. Gate it on the same condition. That empties the toolbar for the duration, and SubRow collapses an empty row, so a streaming response now carries no actions at all until it settles. Both guards encoded the old contract: the unit test asserted copy was present and counted a single button, and the browser guard used copy as its proof that the toolbar had mounted. The settled turn above takes over that role. * fix: Move retry navigation to the outer edge of a user turn A user turn is right-aligned, but its sibling navigation rendered ahead of the actions, so the retry counter sat inboard of the icons instead of under the edge of the bubble it belongs to. Order it last on user turns. * fix: Ride the stream instead of chasing it Following a generating answer went through a helper throttled at 145ms, so the thread caught up in visible jerks rather than flowing. It now writes the scroll position directly on each frame, which is what an answer arriving a few pixels at a time actually needs, and glides only for the one long trip a turn makes, when sending has to travel from wherever the reader was down to the newest word. Whether to follow at all is now answered by where the reader is and which way they were going, rather than by the abort flag. `useMessageProcess` raises that flag on any wheel at all, downward ones included, through a throttle whose trailing call lands after the gesture has ended, so nothing timed to the gesture could outlive it. Scrolling down to the newest word could therefore never resume the ride, while the scroll-to-bottom button, which touches no wheel, always could. Arrival is judged on the scroll it produces rather than the wheel tick that started it, because wheel scrolling is animated and at tick time the thread is still far short of where the tick is taking it. Arriving also counts from further out than leaving does: while an answer streams the end recedes between the last tick and the frame that measures it, so judging arrival as tightly as departure leaves a reader unable to catch it at all. * fix: Reveal retry navigation on hover while an answer generates Copy, edit, fork and read-aloud are all withheld from a response that is still generating, which left the retry counter as the only thing rendering under a half-written answer. It now reveals on hover there, like the actions it sits with, and stays put on a settled turn. * fix: Keep a refused rerun from discarding the edit While a response is streaming, the edit action stays available on every earlier row, and those editors see a per-message submitting flag that is false, so Update and rerun is enabled. The send itself is still refused: ask() returns false for the duration of the active submission. Both editors ignored that and closed anyway, so the draft went with them and no rerun ever started. Both rerun paths now check the result and leave the editor untouched when the send is refused, so the work survives until the thread is free. * fix: Let an upward gesture beat the pending send glide Sending arms a smooth glide down to the newest word, and the landing re-pins the thread to the bottom. The landing was scheduled two ways, on scrollend and on a 700ms fallback, and neither was ever cancelled. A reader who changed their mind and headed up mid-flight was pinned again regardless, then dragged back by the next streaming resize. The fallback fires for the whole window, so this held even after the glide had visibly settled. The gesture now marks the glide interrupted, wherever it lets go of the bottom, and the landing stands down when it sees that. A glide the reader leaves alone still re-affirms the ride. * fix: Fade retry navigation on every streaming response format Every other action is withheld from the row that is still generating, so the retry counter is the only thing left under a half-written answer. The plain text row already faded it to hover-only there; the structured rows did not, and left it sitting on its own. Both structured paths now apply the same condition, and the class string the three of them share moves next to the hover action styles it belongs with. * i18n: Correct the copy the edit surface rewrite left behind The multi-part hint told the reader to save first and then rerun, but a save closes the editor and reopening seeds the drafts from what was just saved, so there is nothing left to rerun and the button stays disabled. Rerunning carries a single edited section by design, so the hint now states that limit rather than pointing at a step that is not there. Drop com_ui_save_submit as well: the per-part editor that used it is gone. * test: Make the message visual baselines opt-in The suite asserts sixteen screenshots and the repository tracks none, so Playwright's default treats every one as a miss and the mock e2e job fails on Linux. Baselines only compare cleanly against the machine that produced them, and nothing here can generate ones that match the runner image. The flows keep running and asserting their structure, which is where their value was; only the pixel comparison is now gated behind E2E_VISUAL_SNAPSHOTS. * style: Restore import order in the reworked message files The repository sorter and CI disagreed with what these files were left holding after the edit surface rework. No behavior change. * test: Follow the reworded rerun hint in the edit layout spec The multi-part hint was restated in the previous commit; this assertion still expected the old wording and would have failed the mock e2e suite. * fix: Leave the send glide alone while the answer streams in Every delta of an answer reruns the scroll effect, and the plain follow writes scrollTop outright, which cancels an animation on its first frame. So the glide a send starts was killed by the first token to arrive and the reader was snapped down instead of carried. The follow now stands down while a glide is travelling, which is what the hook already documented but only enforced on the resize path. * fix: Write a saved edit onto the thread as it stands An earlier turn stays editable while the newest answer streams, and the save captured the thread before the request but wrote it back after. Every delta that landed during the round trip was overwritten. Most of the time the next delta re-merged and the damage showed as a one-frame truncation, but a save that resolved after the stream's final write left the cache wrong for the rest of the session. The thread is now read once the request has resolved, which is what the content part editor already did. The editor actions in this file also wrap again rather than hold one unbreakable row, for the reason given in the following commit. * fix: Let the editor actions wrap on a narrow row At 320px an assistant turn gives the editor about 252px once page padding, the identity column and the row gap are taken out, and Cancel, Save and Update & rerun need more than that in English alone. The group was pinned with shrink-0, so it ran past the edge of the row instead of wrapping. A longer translated label makes it worse, and the user turn had no margin left either. Both editors wrap again, which is what the footer did before the status row was folded into it. * fix: Catch up to the new bottom when the glide lands Following stands down for the length of the glide, so an answer that arrives while it travels moves the bottom past the target the glide aimed at. A short response that finished before the glide reported landing left the thread a few lines short of its own end, with nothing left to correct it. Landing now closes whatever gap opened, unless the reader took over on the way. * test: Follow the renamed rerun button in the edit flow specs The button became 'Update & rerun' when the edit surfaces were unified, but two edit-flow specs still located 'Save & Submit' and would have waited for it until they timed out. A type comment named the old button too. * fix: Judge the first thread scroll against a real position The direction check seeded its last-position ref at 0, so the first scroll event on an opened thread, which arrives carrying a large positive scrollTop, read as a jump downward. Near the end that cleared the abort flag and re-pinned a reader to the stream they were scrolling away from. Take the first event as a baseline and judge direction from the next. * fix: Hold the content part editor to what it replaced EditContentParts took over from EditTextPart and left two of its behaviors behind. An emptied box now blocks Save and rerun instead of persisting a blank part. EditTextPart refused the same edit through its form's required rule and the sibling EditMessage still does, so both editors hold one line. The keyboard shortcuts reach the save paths directly, so they are guarded there too, and the footer says why the buttons are down. The editor also follows the chat direction again, taking dir and text alignment from the same setting EditMessage reads. * fix: Hold the footer height while a response streams Every action is withheld from the row that is still generating, and a lone sibling counter renders nothing, so the footer measured zero until the answer landed and then sprang to the height of the buttons. The transcript stepped upward under the reader at the moment a response completed. The placeholder that used to reserve this space went when the footer became unconditional, so hold the height on the row itself instead. * fix: Remember where the thread was put before judging a gesture Direction is judged against the last sample, and the thread is placed at its end without the reader touching it. With no record of where it was put, their first gesture was spent taking the baseline instead of being obeyed: a single PageUp cleared no flag of its own, so the next streamed resize rode the reader straight back to the end they were leaving. Every programmatic move now records the position it left the thread at, so the sentinel stands only until something has actually placed it. * fix: Spend the start of a turn only once it can be honored A reader who scrolls away during one answer leaves the abort flag raised, and nothing lowers it until the next connection opens, which is after this effect has already seen the send. Marking the turn as started on that first pass spent it against a closed gate: by the time the flag cleared there was no start left to honor, the reader was still detached, and the answer they had just asked for streamed on offscreen. Record the turn as started only on the pass that acts on it. * fix: Show the part edits that survived a refused save The editor saves every changed part through one button, but the endpoint takes a single part per call and nothing rolls a write back. A part the server refused therefore left the earlier ones stored while the editor reported that the message could not be saved, so cancelling from there walked away from edits that were already live. Record the writes that landed and reconcile the transcript with them whichever way the save ended. The refused parts are the only ones left holding a draft, so a retry no longer rewrites what already arrived. * fix: Stop a shared transcript from calling the sharer the reader The share row reused the chat view's user label, which reads "You". It is the screen-reader heading for the user turn, so anyone opening a share link heard every prompt the sharer wrote credited to themselves. Use the neutral "User" label on this surface. It keeps the localization the row gained, unlike the untranslated string it replaced. * fix: Let go of the stream when an interaction settles over several resizes Expanding a tool result mid-answer renders the container first and fills it once its contents arrive, so one gesture produces more than one resize. Only the first was credited to the interaction. The second read the reader as still riding the stream and put them back on the bottom they had just left. The suppressed resize now settles the ride as well as the near-bottom measure, using the position the interaction actually left the reader at, so an interaction that kept them on the end still streams. * fix: Edit inside a structured text part instead of flattening it A text content part holds either a string or a { value, annotations } object. The Assistants thread sync persists the structured form with its file citations intact, and the editor reads the part through the same union, so saving an edit wrote a bare string over the whole object and took every citation with it. The same object was handed to the tokenizer, which measures length, so a part that had been edited this way also stored a NaN token count. Write the edit into value, keep the rest of the part, and count the text itself. * fix: Keep a saved part's citations in the transcript it is written back to A text or think part holds either a bare string or a { value, annotations } object, and the editor already read both through getPartText. Writing the draft back into the local message cache put the string over the whole value, so a response carrying file citations lost them the moment it was edited and did not get them back until a refetch. Reading and writing now go through the same accessor, so an edit lands in the shape it was read from and the rest of the part survives. * fix: Let the message editor follow the chosen font size Editing a message dropped the draft to a fixed 14px regardless of the Font Size setting. On dev the textarea carried the markdown class, so it read --markdown-font-size like the rendered message does; restyling it into a bordered box replaced that with text-sm, and the new per-part editor was written the same way. Anyone on Extra Small, Large or Extra Large saw the text jump the moment they entered edit mode. Share the .message-content typography with the editors through a message-editor-text class so a draft is sized like the message it replaces and keeps tracking the setting. |
||
|
|
155f71f81a
|
📱 fix: Show Quote Popup for Block Selections and on Touch Devices (#14777)
* 📱 fix: Show Quote Popup for Block Selections and on Touch Devices The "Add to chat" popup never appeared for two whole classes of selection. Block-granularity gestures (triple-click, double-click then word-drag) park the selection's far boundary at the start of the next block. For a message's closing block that boundary sits outside `.message-render` — on the composer wrapper or the following message row — while selecting no text there, so the anchor/focus equality check suppressed the popup. Triple-clicking any earlier paragraph worked, which is what made this look like an edge case. The range is now clamped to the message before the check, and selections that really do carry visible text from another message are still refused. Touch platforms could not reach the feature at all. A long-press, and every drag of the native selection handles, emits no mouse event whatsoever — only `selectionchange` — while the popup was shown exclusively from mouseup, dblclick and keyup. Showing now also hangs off a settle-debounced `selectionchange`, gated so an in-progress mouse drag still cannot flicker it. Accepting was broken independently: the tap is also the gesture that dismisses the selection, unmounting the button before `click` could land, so touch commits on `pointerdown` instead. The desktop mousedown path is deliberately unchanged, since preventDefault on `pointerdown` can suppress the compatibility mousedown that click depends on. Two UX consequences of the same code: scrolling re-anchors the popup rather than dismissing it on the first event (the chat auto-scrolls constantly while streaming, and a mobile URL bar collapsing fires resize), and touch selections place the button below the text, clear of the OS Copy/Share callout, with a 44px tap target. Covered by six e2e tests — three desktop, three on an emulated Pixel 5 with a real touchscreen — each verified to fail against the pre-fix build. * 🩹 fix: Address Review Findings and Repair the Scroll Specs The two failing e2e shards were a defect in the specs, not the component. `scrollMessages` reached for `.scrollbar-gutter-stable` with a document-wide query, but the nav and side panels carry that class too, so it could grab a sidebar list that never scrolls — 0px moved, and only in CI, where the nav renders differently. The scroller is now reached from the message itself, the way `MessageNav` does it. The specs also centre the selection first and nudge by a quarter of the visible height, so the gesture cannot scroll the selection clean out of view and then blame the popup for going with it. Review findings, all in `QuoteButton`: Visibility was tested against the window, but the list scrolls inside a bounded container, so text can sit clipped under the header or the composer while its un-clipped rect is still inside the window — leaving the popup floating over unrelated UI. It is now clipped to the nearest scrollable ancestor. Touch committed on the press, so starting a scroll on the button, or touching it and thinking better of it, still added the quote. The excerpt is captured on the press and committed on the release, and only when that release lands on the button, restoring the cancellation every button is expected to have. Commit on press existed because the tap dismisses the selection before `click` fires; capturing the text up front keeps that safe, and an in-flight press is no longer allowed to unmount its own target. A visible popup also described the previous selection for up to the settle window, so a tap while dragging a native selection handle queued the stale excerpt. It is dropped as soon as a differing selection starts settling. Finally, `viaTouch` survived from the last press into keyboard-driven selections on hybrid devices, which could flip the popup into the touch layout; keydown clears it. The cancel path is covered by a new touch spec, verified to fail against a commit-on-press build. * 🧵 fix: Reconcile Cancelled Presses, Widen Clipping, Steady the Scroll Specs Second review round, with one finding taken on trust and flagged rather than claimed as proven. A cancelled touch press could leave the popup backed by a selection that no longer existed. A press deliberately keeps the button alive through a collapsing selection so the release has a target to be judged against, but a cancel then dropped the press without ever honouring the collapse it had masked, so a later tap could add a dead excerpt. Ending a press without committing now rechecks the live selection and dismisses if it went away. Visibility now intersects every clipping ancestor of the message rather than stopping at the nearest. This one is precautionary, not a proven fix: the review that prompted it describes scroll containers *inside* a message (a wide table, a code block) shadowing the outer chat scroller, but the walk starts from the message element, so those are descendants and were never in the chain. Behaviour is unchanged in the current layout — a spec covering a table-cell selection passes identically with and without it — and it is kept only because intersecting the whole chain stays correct if the list is ever nested inside a further-clipped panel. The comment says exactly this. The scroll specs were the real instability. They now move the selection between two positions that are both on screen instead of nudging by a pixel count: blind nudges kept pushing it under the composer, where the popup correctly hides, and the chat's own auto-scroll made the landing spot unpredictable. They also target the opening paragraph, since the closing one is the last content in the conversation and cannot be carried upward from a list already at maximum scroll. The reply fixture gained a table so a selection inside a nested scroll container is exercised, and a spec covers the cancelled press. 15/15 pass locally. * 🪟 fix: Judge Quote-Popup Visibility From the Selection, on Both Axes Third review round. All three findings held up, and each now has a spec that fails without its fix. Clipping is now measured from the selection rather than from the message, and on both axes. A wide table or a long code line scrolls inside its own container — and `overflow-x: auto` makes the computed `overflow-y` auto, so it clips vertically too — which means scrolling it sideways carries the selected text out of view while the message never moves. Walking up from the message could not see those containers at all, and a vertical-only test could not see that motion. This supersedes the previous round's precautionary widening, which was kept without evidence; the evidence is now a spec that scrolls a table past its own selection. Publishing a settled selection also checks visibility. Nothing is tracked during the 300ms settle interval, so a scroll inside that window never reached the re-anchoring path, and the reading was published off-screen and then clamped into view — stranding the popup over unrelated UI. The cancelled-press spec now reproduces the ordering it describes. Collapsing the selection and cancelling in one synchronous block let the asynchronous `selectionchange` arrive after the press had ended, which is the ordinary path and passes either way; it now waits for delivery in between, so the collapse lands while the press is still masking it. Two other specs needed the same scrutiny: `toBeHidden` is satisfied by an element that does not exist yet, so the settle spec sits out the interval before asserting, and it scrolls just past the container edge rather than to the end of the conversation, because a violent scroll re-renders the messages and drops the selection for unrelated reasons. The reply fixture's table is now wide enough to overflow sideways. 17/17 pass, and each new spec was re-run against a build with its own fix reverted to confirm it fails there. |
||
|
|
df6e15a0de
|
🔖 feat: Bound Parent Activity Phases With an Exclusive End Index (#14768)
* 🧭 fix: Finalize Parent Activity Phases at Run Completion * 🧭 fix: Preserve Activity Phase Boundaries * 🎨 fix: Format Activity Phase Boundary Check * 🧭 fix: Ignore Late Label Artifacts at Phase Completion * 🧭 fix: Preserve Logical Activity Phase Membership * 🩹 fix: Narrow Optional Activity Phase Marker * fix activity phase tail boundaries * fix activity phase test lint * fix straddling activity phase batches * preserve activity phase boundaries at scale * fix persisted activity phase final boundary * fix resumed activity phase edge cases * fix sparse activity phase grouping * fix sparse activity phase tail scan * fix resumed activity phase text fallback * fix sparse activity phase completion scans * avoid sparse activity phase runtime scans * stabilize sparse activity phase resumes * support activity phases on current ts target * preserve sparse phase reservations * finalize activity phase boundary handling * avoid sparse phase start scans * fix activity phase final text bounds * tighten activity phase summary boundaries * format activity phase boundary checks * leave final commentary outside activity phases * recognize lane-tagged final activity text * rebase retained activity boundaries on resume * bound activity phase collection work * correct resumed phase activity count * resolve late reasoning before phase completion * preserve lane-tagged final answers * assert durable activity phase bounds in e2e * preserve empty finalized activity phases * ignore empty reasoning at phase completion * format phase completion guard * fix(api): retain overflow reasoning anchors * perf(api): index overflow reasoning anchors * perf(api): skip empty reasoning index scans * fix(api): reconcile completion boundaries efficiently |
||
|
|
236ee6c1ab
|
🧭 fix: Re-Anchor Parent Activity Phase Bounds (#14741)
* test: cover parent activity phase finalization * test(e2e): stabilize parent phase coverage * fix(agents): reanchor parent activity phase bounds * fix(agents): preserve delayed tools in activity phases * test(agents): keep phase slice bounds typed * fix(agents): preserve sparse activity phase bounds * test(e2e): read structured phase replies |
||
|
|
e108955c20
|
🧷 ci: Enforce Durable Agent Finalization for E2E tests (#14740)
* test: enforce agent generation finalization * test(e2e): correlate canonical persisted turns |
||
|
|
7347cfc195
|
🍡 feat: Batched User Questions With A Single Bounded Answer Form (#14737)
* feat: support batched user questions * test: align batched question fixtures * fix: harden batched question lifecycle * test: submit batched HITL answers in e2e * fix: address batched question review findings * fix: preserve invoke return typing |
||
|
|
c93609cb82
|
📸 fix: Guard Screenshot Export Against Main-Thread Freezes (#14733)
* 📸 fix: Guard Screenshot Export Against Main-Thread Freezes * 🧪 test: Cover Conversation Export Flows End-to-End * 🧪 test: Stabilize Export Spec CSV and Toast Assertions |
||
|
|
8da51562f5
|
🧜 feat: Open Mermaid Diagrams as Artifacts with SVG/PNG Export (#14713)
* feat: open Mermaid diagrams in the artifact panel with SVG and PNG export
Mermaid diagrams previously rendered inline only, and the artifact panel
routed every artifact through Sandpack even when no bundler was needed.
- Route Mermaid artifacts to a direct renderer in ArtifactTabs, moving the
Sandpack path into a lazily loaded SandboxArtifactTabs so opening a
diagram no longer pulls in the bundler chrome or the startup config.
- Add an inline artifact card that opens the diagram in the panel instead
of rendering the same diagram twice.
- Add SVG and PNG export from both the inline diagram and the panel
header, with size-capped canvas scaling and background compositing.
- Lazy-load the artifact panel in Presentation and ShareArtifacts.
- Accessibility: label the panel as a dialog on mobile with a focus trap,
make the mobile resize handle keyboard operable, restore focus to the
opener on close, and honor prefers-reduced-motion.
- Fix the generated Sandpack wrapper to serialize diagram source instead
of interpolating it into a template literal.
- Cover the new paths with unit tests and a cross-browser Playwright spec.
* fix: keep Mermaid artifact identity and render state per diagram
Addresses three review findings on the Mermaid artifact panel.
Mermaid fences do not consume a code-block index, so every diagram in a
message received the same `mermaid-${blockIndex}` and therefore the same
Recoil artifact key: expanding one overwrote the other, and both cards
read as selected. Mermaid fences now carry their own index sequence,
seeded per markdown block the same way the code and artifact counters
are, so the id stays stable across streamed tokens.
The panel renderer is keyed by artifact id, so switching directly
between two diagrams can no longer carry the previous render, its
dimensions, or its export payload across the boundary while the new
source debounces. Editing an open diagram still does not remount.
The preview Refresh action drives the Sandpack client, which a Mermaid
preview never populates, so it only covered the panel with a spinner.
It is hidden for Mermaid, which offers its own retry on render failure.
Also drops com_ui_mermaid_export_preparing and com_ui_mermaid_source,
which no longer have call sites, fixing the unused-i18n-keys check.
* fix: bind Mermaid preview and export to the artifact on screen
Three further review findings, all on state outliving what it describes.
The editor reset in ArtifactTabs only lands after commit, so the render
that switched artifacts still passed the previous artifact's editor text
to the freshly keyed renderer, which mounted showing (and exporting) the
diagram just navigated away from. Editor text is now ignored until the
reset catches up. SandboxArtifactTabs carried the same pattern and gets
the same guard.
Switching to the code tab unmounts the preview, but the export payload
survived it, so the toolbar kept exporting a diagram that was no longer
on screen and no longer matched an edited source. The renderer now
withdraws its payload on unmount, and the export action is scoped to the
preview tab.
The diagram canvas mounts only once there is a diagram to show, so the
ResizeObserver ran against a null ref while the placeholder was up and
never saw the real element. Wide diagrams were fitted to the default
700px and clipped in narrower panels. Observation now re-runs when the
canvas appears.
* fix: scope Mermaid artifact ids to the content part
Each content part renders its own markdown tree, so the per-message
Mermaid counter restarts at zero in every part. Diagrams sitting either
side of a tool call therefore both resolved to
`mermaid-artifact-${messageId}-mermaid-0`: one registration overwrote
the other and both cards shared a selection state. The part index the
message context already carries now takes part in the scope.
* fix: keep the Mermaid export menu reachable in fullscreen
The artifact panel gained a fullscreen mode on dev, which re-roots the
panel into the fullscreen element and portals the copy and version
popovers there so they stay visible. The Mermaid export menu portals to
the body, so once these branches met it opened outside the fullscreen
element and rendered invisible. It now takes the same portal target.
* fix: heal Mermaid registrations and cap PNG canvases after rounding
Two findings from the latest review pass.
Closing the panel unmounts Artifacts, whose useArtifacts cleanup wipes
artifactsState while the inline cards stay on screen. The Mermaid card
never observed that, so reopening one card restored only itself and any
other expanded diagram vanished from the version navigator until it was
clicked again. It now subscribes to its own slice and re-registers when
the entry goes missing, matching the self-heal ToolArtifactCard already
documents. The write is a no-op when the entry matches, so it settles.
Rounding each PNG side independently could carry the product back over
the 16.7M pixel budget the scale was picked to satisfy: 3129x50000
resolved to 1025x16374, which is 16,783,350 pixels and enough for a
browser enforcing the area limit to reject toBlob outright. Rounding
down cannot exceed the budget, since the bounding scale is derived from
it.
|
||
|
|
152dcf4721
|
🔗 feat: Shared Conversation Badge and Stable Share Links (#14712)
* feat: improve shared conversation links * test: Cover Shared Link Lifecycle * test: Cover Shared File Snapshots * fix: address review findings on shared links Stop double-decoding the conversation search term. Express already decodes req.query, so the route's extra decodeURIComponent threw URIError on any term containing a bare percent sign and mangled percent-escape-looking text. The sidebar already sent the term raw, so this failed there too. Advance a share's stored target to its branch tail when an update omits one. Updating from the conversation list could not resolve the tail and reused the stored target verbatim, silently republishing the same snapshot instead of the turns added since. Require revalidation on shared files. Updates now keep the shareId, so the file URL no longer changes and a cached response could outlive a revoked share-files choice; an ETag over the pinned snapshot fields keeps unchanged files on 304. * fix: keep the shared badge across conversation cache replacements isShared is derived per list request and absent from single-conversation payloads, so rename, pin, and the SSE conversation updates dropped it when they swapped a server response into the sidebar cache, hiding the badge until an unrelated list refetch. Carry the cached value forward in updateConvoInAllQueries so every replacing caller is covered, while an explicit value still wins. * test: mock syncStaticTools in server boot specs initializeMCPs now calls syncStaticTools when no MCP servers are configured, but the server boot specs stub ~/server/services/Config without it. Post-listen initialization threw, hit process.exit(1), and took the jest worker down until it exceeded the retry limit. * fix: address codex findings on the shared DataTable and file ETag Keep the published DataTable export bound to the legacy component and ship the design-system table as VirtualizedDataTable, so external consumers of @librechat/client keep the props they compile against. Fold the snapshot's stored location into the shared-file ETag, so a re-published output that keeps its size and revision but moves its object no longer revalidates to a stale 304. Auto-fill the table when a first page is too short to overflow its container, since pagination is otherwise only reachable through the scroll handler. * fix: re-scope share grants before publishing and retry stalled auto-fill Move the shared-link ACL expiration write ahead of the content update. The shareId survives an update, so a failed ACL write after the write-through left the new messages and file snapshot readable at the same URL while the owner saw a 500. Retry a rejected auto-fill fetch up to three times: an unscrollable table has no scroll event to fall back on, and the sentinel alone would strand it on the first page. * fix: follow regenerated branches and pin forks to the payload they saw advanceTargetToBranchTail only walked descendants, so a target replaced by a regeneration (a sibling, not a child) left the update parked on the obsolete branch and published none of the turns that followed. A childless target now hops once to the newest sibling the conversation continued under. A shareId survives an update, so an owner republishing between a viewer's load and their Continue click would resolve targetMessageIndex against different messages. The fork request now carries the payload's updatedAt and is rejected with 409 when it no longer matches; the viewer gets the current version pulled in and can retry. * fix: keep table sorting and legacy backfills from breaking share flows Restore the union formatting a local lint-staged prettier collapsed in data-provider types, which broke the CI lint run. Header clicks now toggle direction instead of cycling through an unsorted state, which the controlled tables translated straight back into the default and made one direction unreachable. Re-arm the auto-fill guard on a sort change: a re-sorted first page arrives with the same row count, and the guard would otherwise suppress paging on a container that still cannot scroll. Lazy fileSnapshots backfills no longer touch updatedAt. That timestamp is the revision a viewer's fork is validated against, so a legacy share's first read would have made the Continue click that followed it fail with a 409. * fix: break pagination ties by id and reset share state per conversation Both list cursors marked a page boundary with values that repeat: conversations by (sort field, updatedAt) and shared links by the sort field alone. Imported chats share a title and a timestamp, so every row tied with the boundary was skipped. Both now carry the boundary row's _id and sort by it last, and the shared-links cursor is an opaque composite the route still validates before querying. The share dialog outlives a switch between conversations, so a link with files disabled left the next conversation's dialog showing the switch off and quietly published without files. The stored choice now falls back to the enabled default, and a stale link no longer sits in the copy field. * fix: keep titleless shared links in the paginated list A share has no title default, and BSON orders a missing title before every string, so encoding the boundary as an empty string skipped the remaining untitled links when sorting Name ascending and re-admitted all of them descending. The cursor now carries the boundary's null rather than flattening it, and because $lt and $gt are type-bracketed against a string, descending adds an explicit clause for the untitled tail that a string comparison can never reach. * style: sort share method imports * fix: fail closed on orphaned share targets and guard snapshot backfills getMessagesUpToTarget walked levels from the roots and returned everything it had accumulated when the target was never reached. An imported or partially deleted branch whose parent is missing therefore published the whole conversation instead of the selected branch; the walk now returns nothing unless it actually reaches the target. A lazy backfill wrote fileSnapshots unconditionally, so a viewer's first read of a legacy link could land after a republish and restore the snapshot it replaced, re-authorizing the stable URL of a file the owner had just removed. The write is now conditional on the link still having no snapshot, and the stored one wins any race. Regenerating a message above the shared tail leaves the whole stored branch childless, so the target walk now climbs to the closest ancestor the conversation continued under instead of stopping at the stored tail's own siblings. Changing the search or sort also returns the table's viewport to the top, since the query holds the previous rows while it refetches. * fix: page through titleless rows on both sides of the cursor The route validator still required a string primary, so the composite cursor the data layer issues for a titleless boundary came back as a 400 and the shared-links table stopped at that page. Conversations had the same type-bracketing gap the shared links just closed: a name-sorted page could not reach conversations with no title, since a comparison against a string never matches a missing field. The cursor now carries the null and the filter spells out the titleless clauses for both directions. * fix: keep the share badge read-only and refresh rows on cell changes ensureLinkPermissions re-granted the owner ACL entry on every call, so rendering the header's shared badge turned ordinary navigation into a permission write. It now checks for the grant first and only migrates a link that still lacks one. A fork carrying a positional target but no revision falls back to the whole share, since nothing proves which payload the index was counted against. The memoized table row compared row data and selection only, so a cell rendering external state (the archived list's pending Restore, for one) kept its stale rendering until the row object itself moved; rows now also compare a marker that moves with the column definitions. * fix: keep the shared badge honest when a delete fails or a link remains A failed delete left the conversation looking unshared: the optimistic snapshot covered only the shared-link queries, not the conversation caches the badge reads. The cleared conversations are now restored with the rest. A conversation can hold one link per target message, so clearing the badge on delete is a guess. The conversation list is invalidated once the mutation settles, letting the server decide from the links that are actually left. * fix: refetch every cached conversation page after deleting a link The invalidation was pinned to page zero, so a conversation cached further down the sidebar kept the badge the optimistic update had already cleared even when another targeted link survived. * fix: treat a failed page fetch as a failed auto-fill React Query resolves fetchNextPage with an error result instead of rejecting, so the rejection handler never ran: the guard stayed armed on the unchanged row count and an unscrollable table could never reach the next page. * refactor: move the share request helpers into the typed backend Cursor validation, page-size clamping and the shared-file cache validator were plain backend logic sitting in the legacy JS route. They now live in packages/api with their own tests, and the route keeps only the Express-side wiring: reading query params, mapping domain error codes to status codes, and writing the response. Also carries the requested file choice into the cache entry the create and update mutations synthesize, since the response never echoes it and the dialog reads a resolved entry with no choice as the enabled default. * fix: hold auto-fill while the replacement page is in flight A search or sort swap keeps the previous rows and hasNextPage on screen while the new first page loads, so the re-armed auto-fill asked for page two against a query that was still fetching its first. An infinite query runs one fetch at a time, so that request could cancel or interfere with the one already out. Both tables now pass their fetching state and the guard waits for it. * fix: stop advertising links a deployment no longer serves The sidebar badge rendered from the derived flag alone, so a deployment that turned shared links off still told owners a link was live while the public routes were unregistered. The share dialog called the link a snapshot, but the payload populates the referenced message documents on every request: an edit to an already-shared message is visible immediately, and Update only adds newly referenced ones. The copy now says that. Scroll pagination inspects a resolved error the way auto-fill already does, since React Query reports a failed page that way instead of rejecting. * a11y: gate the shared conversation label on the feature flag The icon stopped rendering when a deployment turns shared links off, but the row still announced the conversation as shared to screen readers. Both now read the same condition. * fix: accept long title cursors and stop badge work the feature disables The cursor cap was tight enough that a Name-sorted page ending on a long title produced a nextCursor the next request rejected, stranding the rest of the list. It now sits well clear of anything the server can issue. The conversation list skipped straight into the shared-link lookup even where ALLOW_SHARED_LINKS is off, paying a round trip on the sidebar's first page for a badge that is never rendered. A regeneration is newer than what it replaced, so only a newer sibling counts: an older one that still has follow-ups is the branch the target was regenerated away from, and resuming there published turns the target had excluded. * fix: hold scroll pagination while a replacement page loads Resetting the viewport to the top after a search or sort change fires a scroll event, and the retained previous rows still report another page, so the handler asked for page two of a query that was still loading page one. * fix: keep the legacy share migration ahead of the owner-grant shortcut A legacy row keeps its marker until every grant it needs exists, so an owner grant on its own is not proof the migration finished. Reading the marker first means a half-migrated public link still gets its public grant, while a fully migrated one keeps the read-only settled path the badge lookup depends on. |
||
|
|
92d4705f79
|
🧭 refactor: make the side panels behave the same way (#14695)
* style: unify chat input tool badge styling Every tool badge repeated max-w-fit and its own hand-written checked-state colour triplet. Move max-w-fit into CheckboxButton's base classes, where tailwind-merge still lets a consumer override it, and collect the accent colours into a single map so the palette lives in one place. Artifacts repeated the amber triplet a second time on its dropdown button; that now reads from the same map. * feat: add feedback when resetting model parameters The button did nothing visible on click, so with parameters already at their defaults it looked broken. Spin the icon a full turn on press and announce the change politely, matching the Agent Builder panel which already announced but had no visual counterpart. The animation replays on consecutive clicks via a reflow, and is gated behind motion-reduce. * fix: keep the prompt editor open when inserting a special variable Opening the variables menu moved focus out of the textarea, whose blur handler exits edit mode, so the prompt snapped back to its rendered preview as if it had been saved. Guard the blur against focus landing inside a menu, since Ariakit focuses the menu itself on open, and hand the menu a finalFocus target so focus returns to the textarea on close. Without the latter the editor stayed open but unfocused, which quietly broke click-away-to-exit. * feat: create prompts from a dialog instead of a dedicated page Prompts now open a dialog from the sidebar, matching how skills and MCP servers are created, and /prompts/new is gone. The dialog reuses the existing form rather than duplicating it, with a flag to drop the page-level chrome that has no place in a modal. Three things the modal exposed: - Radix locks pointer events on the body, so the portaled category and special-variable menus rendered but could not be clicked. They now render inline when hosted in a dialog, as SetKeyDialog already does. - The floating labels notch out the page surface, which left a visible chip against the dialog background in dark mode. The surface is now passed in rather than hardcoded. - Creating gave no indication anything was happening; the button now shows a spinner and blocks repeat submits. Create buttons for both prompts and skills use the submit variant, since both perform a write. * style: match prompt action button sizes The share button sat at 36px next to a 40px Use Prompt button in the preview. Drop the size override so it takes the icon variant's default, and bring its row-mates in the editor header along so that row stays uniform. * feat: load prompts by scrolling instead of paging The query was already cursor-based; the nav hook was slicing it back into one page at a time behind Prev/Next buttons. Flatten the loaded pages and let the existing scroll hook fetch as the list nears its end. useNavScrolling only fetched from a scroll event, so a first page that did not overflow its container produced no event and the rest of the list was unreachable. It now tops up until the list actually scrolls, which is why zooming in used to 'fix' it. * feat: pin panel admin settings and scroll only the panel content Each side panel scrolled as a whole, so its filter row and toggles slid away with the list and the scrollbar spanned the full height. Give every panel a fixed header, a scrolling content region, and a footer that holds the admin settings. The skills panel gains the standard filter input in place of its title and toggle-to-search icon; it also rendered admin settings twice, once from the filter row and once from the accordion. Memories drops its client-side paging, which only sliced already-loaded data, in favour of scrolling the full list. * fix: repair the skills create menu and icon-only dropdowns The create menu was built on Dropdown, which is a select rather than an action menu, and Dropdown applies its className to the popover as well as the trigger. Sizing the trigger therefore shrank the menu itself to 36px and clipped both entries. Rebuild it on DropdownPopup, which is what the rest of the app uses for action menus. Dropdown's icon-only trigger also kept its horizontal padding and laid the icon out in a full-width flex row, leaving too little room so the icon flex-shrank to roughly half its width. That affected every icon-only consumer, including the prompts category filter. * fix: correct the gap above the MCP server URL field The fieldset grouping the connection sections carried display: contents, which removes its box and with it the margin that space-y puts on it. The first section inside sat flush against the description while every other gap kept its 16px. * refactor: unpin a favorite in one click The row's overflow menu held a single Unpin entry, so opening it was pure overhead. Show the unpin button directly instead. Its hover surface matched the row's own hover colour exactly, so hovering changed nothing; it now uses a surface that differs in both themes, with a border carrying the contrast in light mode where the surfaces are close. Adds the tests for unpinning, which had none. * fix: stop prompt skeletons stacking on top of the loaded list The groups were rendered outside the loading branch, so a refetch with data already cached drew three skeletons above the existing rows instead of leaving the list alone. The three states are now mutually exclusive. * feat: add PanelContent to standardize side panel loading states Each panel decided for itself whether to draw a spinner, a skeleton, or nothing, and some replaced the whole panel rather than just the list. PanelContent owns the scroll region and the loading/empty/content decision so a panel cannot invent a fourth pattern. It takes isLoading rather than isFetching on purpose: a refetch that already has rows on screen should leave them alone. * feat: give the side panels row-shaped loading skeletons Each panel now loads with a skeleton built from the row it stands in for, rather than a spinner or nothing: the memory card's key and token pill, the MCP server's icon over name and description, the bookmark's icon and count, the prompt card's block. Memories previously replaced the entire panel while loading, so the filter you had just typed into disappeared. The skeleton is now confined to the content region and the header stays put. Loading also moves out of the list components, which had each grown their own copy of it, and into the shared PanelContent. * feat: show a loading state in the bookmarks panel Bookmarks had no loading state at all: it rendered straight into its empty state while fetching, so it flashed 'no bookmarks' before the list appeared. Thread isLoading through and give it the same header, scrolling content and skeleton as the other panels. * style: tighten the favorite row and unpin button Even padding on the row, the unpin button sitting a little closer to the edge, and no border until it is hovered. * feat: scroll the bookmarks list instead of paging it Bookmarks were already fetched in full, so the pager was slicing data that was sitting in memory. Render the whole list and let it scroll, the same as the other side panels. It also removes a latent drag bug: rows were reordered by their index in the unsliced array while the list rendered a page slice, so dragging on any page past the first moved the wrong row. * feat: load skills by scrolling instead of capping the list The skills panel fetched a single page of 50 and never asked for more, so a 51st skill was unreachable. Switch it to the cursor-paginated infinite query that already existed alongside it and wire the shared scroll hook, matching prompts and the other side panels. The list and its rows only ever read summary fields, so they now take TSkillSummary and the response no longer needs casting through unknown. * fix: stop mocking real modules as virtual in specs Seven specs mocked @librechat/client and librechat-data-provider with `virtual: true`, which is for modules that do not exist on disk. These do, so the flag keyed each mock to a path derived from the spec's own directory rather than the module's resolved id. The component under test resolves the real id, so whether it got the mock depended on the module id cache of whichever worker picked the file up. UploadSkillDialog was the one that bit: when the mock missed, the real Radix dialog rendered and portaled its content to the body, so every assertion reading from the render container failed with the input "not rendered" while it sat in a portal a few nodes away. * test: give the lazy bookmark chunk room to load Waiting for BookmarkNav means waiting for babel to transform its whole module graph on first require, which does not fit in waitFor's default second when the transform cache is cold or the machine is busy. The failure looked like a missed re-render but was just an import in flight. * build: recycle jest workers before the OS kills them Coverage maps accumulate for the life of a worker, so a full client run pushes workers past a gigabyte and the OS kills one, failing whichever suite it was holding at the time. Capping idle worker memory also cut the wall clock, since the run no longer swaps. * fix: give the dialog prompt labels a real backdrop Floating labels notch out the surface behind them so the input's border does not run through the text. The dialog variant asked for `bg-background`, which no longer maps to anything and computes to transparent in both themes, leaving the border visible through the label. `bg-surface-primary` is what OGDialogContent actually paints. * fix: resolve side panel review findings Send the removed prompt create page to a tombstone route so a stale /prompts/new cannot render a blank form or fetch the id "new". Drive the list footer spinner from isFetchingNextPage alone; the old showLoading flag was set on scroll and only cleared by a later scroll, so it stuck on after the last page. Retry the scroll auto-fill through a ResizeObserver: the fill bailed whenever the panel had no layout yet and nothing asked again once it got one. A collapsed sidebar keeps its panel mounted and laid out, so gate fetching on the sidebar being expanded rather than draining the catalog behind an invisible panel. Gate the MCP admin footer on the admin role, matching the memories, prompts and skills panels; the bordered bar rendered empty for everyone else. Replay the reset icon spin by remounting the icon. Toggling the class list lost the animation to the re-render that setConversation causes. Announce panel loading from a live region carrying its own text. The skeleton rows and the spinner are both aria-hidden, so labelling the region left nothing for a screen reader to read out. Cover the scroll hook, the panel content primitive and the prompt create dialog with unit tests, and point the prompts e2e spec at the dialog rather than the deleted page. * chore: remove unused translation keys com_ui_pagination and com_ui_select_or_create_prompt lost their last callers when the prompt list moved to infinite scroll and the empty prompt preview was dropped. Only the English file is touched; the other locales are generated externally. * Fix nav pagination retry loop * Fix prompt field IDs and skills pagination * Fix prompt dropdown ARIA IDs * test: stub syncStaticTools in the server bootstrap specs initializeMCPs now calls syncStaticTools from services/Config when no MCP servers are configured. Both bootstrap specs mock that module wholesale, so the call threw, the post-listen handler ran process.exit(1), and the Jest worker died four times over before the suite was reported as failing to run. |
||
|
|
1bccc2bc18
|
📡 fix: Refresh MCP Tools After List-Changed Notifications (#14686)
* fix(mcp): handle dynamic tool list changes Co-authored-by: Pascal Garber <pascal@artandcode.studio> * test(mcp): fix CI validation * fix(mcp): keep dynamic tool catalogs live * fix(mcp): harden dynamic catalog lifecycle * test(mcp): use typed startup connection * test(mcp): isolate dynamic e2e fixtures * fix(mcp): refresh tools after reconnect * fix(mcp): close dynamic catalog cache gaps * test(mcp): update OAuth connection mocks * fix(mcp): preserve app snapshot ownership * style(mcp): sort connection imports * fix(mcp): close review race conditions * fix(mcp): preserve cache ownership edges * fix(mcp): harden recovery lifecycle * fix(mcp): guard tool-less app refresh * fix(mcp): fence distributed cache races * fix(mcp): retire stale connection state * fix(mcp): keep tool snapshots authoritative * fix(mcp): fence stale app tool publications * style(mcp): sort repository test imports * test(mcp): mock empty startup publication * fix(mcp): preserve app publication generations * fix(mcp): harden publication recovery races * fix(mcp): address tool catalogs by runtime config * fix(mcp): load scoped catalogs for assistant writes * fix(mcp): harden catalog publication recovery * fix(mcp): serialize forced connection replacement * fix(mcp): serialize ordinary creation with replacements * fix(mcp): harden catalog fallback boundaries * fix(mcp): close lifecycle fencing gaps * fix(mcp): preserve catalog authority on failures * fix(mcp): compensate failed catalog mutations * fix(mcp): fence catalog refresh ordering * style(mcp): sort agent loader imports * fix(mcp): cancel stale connection creation * fix(mcp): fence catalog coordination * fix(mcp): close catalog race windows * fix(mcp): harden cross-pod catalog fencing * fix(mcp): close catalog lifecycle edges * style(mcp): sort assistant imports * fix(mcp): reject stale recovery authority * fix(mcp): restore static catalog on every startup * fix(mcp): order app catalog publications * style(mcp): sort catalog revision imports * fix(mcp): separate catalog allocation and commit fences --------- Co-authored-by: Pascal Garber <pascal@artandcode.studio> |
||
|
|
51ed1fab4b
|
🩹 fix: Keep Edit Action Fully Hidden While Streaming (#14687)
* 🩹 fix: Keep Edit Action Fully Hidden While Streaming #14677 stopped the row-hover reveal from un-hiding the edit button, but the pencil still shows as a dimmed ghost mid-generation. The shared Button primitive sets `disabled:opacity-50`, which compiles to `.disabled\:opacity-50:disabled` — specificity (0,2,0). The hidden state used a plain `opacity-0` at (0,1,0), so the disabled style won and painted the icon at half opacity. Verified in Chromium against a running instance: only two opacity rules match the button, and the computed value was 0.5. Switching the hidden state to `!opacity-0` (Tailwind emits `opacity: 0 !important`) drops it to 0 while the sibling actions still reveal at 1 on hover. The existing unit test could not catch this: jsdom applies no stylesheet, so asserting class names never exercised the cascade. It now asserts the important modifier specifically, with a comment explaining why a bare `opacity-0` is insufficient. * 🧪 test: Browser guard for the hidden edit action The Jest spec can only assert class names — jsdom applies no stylesheet, so it could not see `disabled:opacity-50` (0,2,0) outranking `opacity-0` (0,1,0) and repainting the hidden pencil at half opacity. That is exactly how the ghost survived #14677 with a green suite. Asserts computed opacity in a real browser mid-stream, and asserts the sibling Copy action is at opacity 1 in the same breath so a hover that silently failed to register cannot make the check pass for the wrong reason. Verified to fail on the pre-fix build with `Received: "0.5"`, and to pass 3/3 after. |
||
|
|
1367672942
|
🔁 fix: Re-Arm Soft Default When a Stored Agent No Longer Resolves (#14664)
* 🔁 fix: Re-Arm Soft Default When a Stored Agent No Longer Resolves * 🔁 fix: Scope Agent-List Gate to Storage-Derived Selections, Reuse Shared Agents Map * 🔁 fix: Trust Stored Agent Pick When the Catalog Request Fails * 🔁 fix: Mount Keyboard Shortcuts Inside the Agents-Map Provider * 🔁 fix: Always Gate 404 Fallback on Agent List, Skip Wait When Selector Disabled |
||
|
|
4f5c9fec4f
|
🎨 refactor: adopt the @librechat/client design system (semantic color tokens + component migration) (#13879)
* refactor: unify Tailwind color tokens into a single source
Both the client SPA and @librechat/client Tailwind configs now consume one
createTailwindColors() map, eliminating config drift. Fixes the package-side
build along the way: shadcn tokens are wrapped in hsl(), the broken opacity
helper is removed, and text-destructive/border-destructive/switch-unchecked
plus the gray/green palettes are included.
* refactor: replace hardcoded colors in sidebar conversation list with tokens
Migrate the Conversations sidebar section to semantic tokens: focus rings to
ring-text-primary (keeps >=3:1 contrast in both modes; the mid-gray ring would
fail WCAG 1.4.11 on dark), the active-conversation indicator and hover-fade
gradient to surface/text tokens, and the pagination controls. Removes every
dark: color twin; no behavior change.
* feat: add semantic status-color tokens; migrate MCP status badge
Add a status-color layer (status-{success|info|warning|error|neutral} plus
-subtle variants) to style.css and the unified createTailwindColors map, with a
blue palette for the info hue. Migrate MCPStatusBadge (badges + dots) and
MCPCardActions to the new tokens, removing all hardcoded status colors and
dark: twins. Status colors are now themeable like the rest of the system.
* refactor: migrate status badges to semantic status-color tokens
Migrate the genuine status badges to the status-* tokens: MCPConfigDialog
connection pills (info/warning/neutral/error/success + dot), MemoryUsageBadge
usage levels, and DialogImage quality badge (also gains dark-mode support it
previously lacked). Removes hardcoded colors and dark: twins.
* feat: add Alert component and migrate alert banners to it
Add a reusable Alert component (@librechat/client) with error/success/warning/
info/neutral variants backed by the status-color tokens, default per-variant
icons, and role=alert. Migrate the duplicated colored-div banners to it:
Auth ErrorMessage, RequestPasswordReset success, and the identical error boxes
in ToolSelectDialog, AssistantToolsDialog, and MCPToolSelectDialog.
* refactor: migrate remaining alert banners and error states to tokens
Migrate the last banners to the Alert component: ResetPassword success,
MessageContent connection error, and MemoryInfo storage-full errors. Tokenize
the Agents ErrorDisplay error state in place (icon badge, headings, message,
retry button) since it's a full error state, not a compact callout. Also
tokenize ResetPassword field-validation errors to text-text-destructive
(fixes the low-contrast dark:text-red-900).
* refactor: tokenize SidePanel Memories/Parameters/Bookmarks colors
Delete-confirm buttons to surface-destructive tokens (MemoryCardActions,
BookmarkCardActions), drop redundant text-white on submit Buttons (the variant
already sets it), legacy preset button green hover/focus to submit tokens, and
slider hover borders to border-light. Leaves DynamicCheckbox dark overrides for
a separate pass against the Checkbox component.
* refactor: tokenize Settings danger/destructive buttons
Map the DangerButton, the Data tab destructive actions (RevokeKeys, ClearChats,
DeleteCache), and the DeleteAccount button from bg-red-*/bg-destructive to the
surface-destructive tokens.
* refactor: tokenize Chat file-upload table and upload status colors
Tokenize TemplateTable th/td/border classes (surface-primary, border-light,
text-primary/secondary) and FileUpload status colors (text-text-secondary,
text-text-destructive, text-status-success) plus the import button hover.
* fix: explicit type annotations on Alert for isolatedDeclarations
@librechat/client builds with tsdown --isolatedDeclarations, which requires
exported consts to have explicit type annotations (TS9010). Annotate
alertVariants and Alert to match the Button.tsx pattern.
* refactor: add soft status-border token layer for Alert and lighten dark status foregrounds
* refactor: tokenize Chat menus, popovers, and message surfaces
* refactor: tokenize Chat message content, tool output, and file UI colors
* refactor: add semantic link color token and migrate hyperlinks to it
* refactor: tokenize Files and Auth surfaces, text, borders, and CTAs
* refactor: add accent-primary brand token; tokenize Nav/Input/Prompts/Endpoints colors
* refactor: tokenize Auth brand-green accents, Skills, Sharing, Plugins, MCP colors
* refactor: tokenize OAuth, Share, ui, Bookmarks, Tools, Messages, Web, SharePoint colors
* refactor: final solid-color cleanup (brand-green accents, neutral grays, error text)
* refactor: migrate status callout banners to status-subtle/border tokens
* refactor: tokenize token-usage gauge, mic, and oauth countdown status colors
* refactor: replace shadcn color vocabulary with semantic tokens
Remove the shadcn/ui color tokens (background, foreground, card, popover,
muted, accent, secondary, destructive, input) and migrate every usage to
LibreChat semantic surface/text/border tokens.
Add surface-inverted/text-inverted for the neutral inverted CTA and
surface-fixed/text-fixed for controls that must not flip with the theme
(favicon chips, QR container, carousel arrows). New tokens are defined once
in style.css (light + dark), createTailwindColors, the theme types,
applyTheme and the default/dark theme objects so they stay overridable at
runtime.
Collapse paired dark: color variants into the dark-aware tokens and tokenize
the remaining raw palette and white/black utilities, mapping status colors to
the status-* tokens and legacy ring-black/ring-white focus rings to
ring-text-primary.
Retain the background, primary and ring tokens, which are still referenced by
the SidePanel/Agents and SidePanel/Builder panels (excluded from this pass).
* refactor: tokenize remaining status, neutral and message-text colors
Map the leftover semantic colors to tokens: skill error/dirty states and the
selected-version/selected-skill highlights move to status-warning/status-success,
the global indicator to status-success, and the markdown message text to
text-text-primary. Drop the redundant dark: overrides on the dynamic checkbox,
which the Checkbox primitive already handles.
What remains is intentional and stays raw: categorical color sets (category
icons, principal avatars, per-tool toggle accents), brand marks, the
WCAG-tuned toast severities, code/diagram surfaces, scrims, and text-white on
submit/destructive action surfaces.
* refactor: remove unused CSS rules, dead comments, and duplicate keyframes
Drop ~829 lines of dead styles across style.css (2992->2355) and
mobile.css (323->131): unreferenced classes (legacy token utilities,
orphaned animations, form/prose/scrollbar leftovers), commented-out
blocks, and duplicate/orphaned keyframes. Library-injected (hljs, sandpack,
codemirror, markdown language) and dynamically-applied (scroll-animation,
icon sizes) classes were retained.
* fix: resolve ESLint and frontend test failures
- Format with prettier (Alert, MCPStatusBadge, ApiKeys, Memory, etc.) after
--no-verify commits skipped the hook
- Localize the 'Or' auth divider (com_auth_or) instead of a bare literal
- Drop dead InvocationModePicker imports in Skill forms; fix VerifyEmail
unused arg + useEffect deps
- Revert out-of-scope color edits in legacy Files/VectorStore views that
carried pre-existing untranslated-string lint debt
- Update Memory tests to assert status-* tokens (text-status-error,
bg-status-error-subtle) instead of the old hardcoded red classes
* refactor: migrate theme tokens to RGB channels for opacity support
Convert semantic + palette CSS variable values in style.css from hex to bare
'R G B' channel triplets, and emit Tailwind colors as
rgb(var(--token) / <alpha-value>) via createTailwindColors. This makes opacity
modifiers (bg-surface-primary/50, bg-border-medium/60, etc.) resolve correctly
and remain dark-aware, fixing ~26 existing usages that previously fell back to a
hardcoded light hex.
- Wrap direct var(--token) color usages in CSS rules as rgb(var(--token))
(style.css, Dropdown.css, Tooltip.css) and two inline component styles
- applyTheme writes bare triplets to match the new wrapping
- shadcn tokens (HSL) and the JS palette (hex) are unchanged
* fix: prettier formatting after dev rebase
* refactor(client): migrate low-risk primitives to @librechat/client
Swap raw <label>, <textarea>, and native title= tooltips for the
@librechat/client Label, Textarea, and TooltipAnchor components across
Agents, Endpoints settings, Export modal, Prompts, Sharing, and Memory
dialogs. Add localization keys (scroll, sibling navigation, none
selected, select var) for the remaining swap waves.
* refactor(client): migrate buttons, inputs and labels to @librechat/client
Swap raw <button>, <input> and <label> elements for the @librechat/client
Button, Input and Label components across Auth, Chat, Conversations,
Endpoints, Nav, Prompts, Skills, Tools and Web. Preserve bespoke geometry
and behavior via cn className merging, keep data-testid/aria wiring, and
localize previously hardcoded aria-labels. Skip swaps that would break
floating-label animations, tiny bespoke controls or inline-text links.
Add com_ui_reload_page key.
* refactor(client): migrate dialogs, toggles and remaining controls to @librechat/client
Swap behavioral controls for @librechat/client equivalents: HeadlessUI
and legacy dialogs to OGDialog, native checkbox/switch to Checkbox/Switch
(onCheckedChange), and remaining buttons/inputs across Chat, Skills,
Tools, Sharing, Memories and Settings. Convert applicable native title=
tooltips to TooltipAnchor and localize close/scroll aria-labels. Skip
swaps that would break floating-label animations or bespoke select
behavior. Update co-located test mocks to provide the newly-used Button
and cn dependencies.
* style(client): soften dropdown and settings search inputs
Remove the heavy focus ring on the settings search and the searchable
Dropdown's search input, replacing it with a subtle border-light. Make
the search field background inherit the dropdown surface so it matches in
both light and dark mode, and reduce the Dropdown trigger border from
medium to light.
* refactor(client): migrate Agent Builder and Tool Library to @librechat/client
Swap raw buttons, inputs, labels, textareas and native title tooltips for
the @librechat/client Button/Input/Label/Textarea/TooltipAnchor components
across the Agent Builder panel (SidePanel/Agents) and the Tools
marketplace. Remove heavy input focus rings in favor of subtle borders,
soften dropdown trigger borders, and convert stray shadcn/raw colors in
touched lines to semantic tokens. Localize the tool delete aria-label and
toast messages. Update co-located test mocks to provide the newly-used
Button component.
* fix(client): keep Input border static on pointer focus
The pointer-focus override in Field.css used border-color: var(--border-light),
which became an invalid value after the theme moved to RGB channel tokens and
was silently dropped, letting the border fall back to currentColor (text-primary)
on mouse focus. Wrap it in rgb() so mouse focus produces no border, ring, or
outline change; keyboard focus keeps its ring for accessibility.
* refactor(client): remove residual shadcn color tokens
The background/primary/primary-foreground/ring and unused chart-* tokens were
retained only for the then-unmigrated Agent Builder. With that panel migrated,
replace the last usages with LibreChat semantic tokens (ring-primary/ring-ring
-> ring-text-primary; bg-primary/text-primary-foreground -> bg-surface-inverted
/text-text-inverted; text-primary -> text-text-primary; bg-background ->
bg-surface-primary) and drop the token definitions from createTailwindColors,
applyTheme, the theme objects, types, and style.css.
* fix(client): address semantic theme review feedback
* fix(client): use boolean Monaco hover option
* fix(client): resolve CI validation failures
* test(client): update shared component mocks
* fix(client): expose status tokens to runtime themes and document channel format
Add the status, text-destructive and border-destructive families to IThemeRGB,
IThemeVariables, IThemeColors, mapTheme and the bundled light/dark themes so
ThemeProvider consumers can theme Alert and the status badges instead of falling
back to the stylesheet palette.
Update the theme README to document the channel-triplet contract that the RGB
migration introduced, since the previous examples used complete CSS colors that
now produce invalid declarations.
* test(e2e): use accessible message action locators
* fix(client): address theme env, dialog padding and locked button review feedback
Expose every IThemeRGB token through REACT_APP_THEME_* instead of the
hand-maintained subset that omitted the status, destructive, inverted and
fixed families.
Drop the padding OGDialogContent contributes to the Tool Library so the
header divider spans the panel again, and stop disabled:opacity-100 from
overriding the locked delete-account button's dimmed state.
* fix(client): read theme environment variables from the build-time env
getThemeFromEnv read process.env, which vite-plugin-node-polyfills replaces
with an empty shim in the browser, so every REACT_APP_THEME_* value was
dropped and the loader always returned undefined.
Read import.meta.env instead and register the REACT_APP_THEME_ prefix with
Vite so the values are inlined at build time. The env source is now a
parameter, which lets the tests cover the mapping without mutating globals.
* fix(client): replace Tailwind classes that no longer resolve
Several class names in the client and shared component package emit no CSS
rule at all: legacy token- names with no definition, Tailwind v1/v4 names,
and plain typos. They fail silently past typecheck and tests.
- text-md -> text-base (Tailwind has no md font size)
- text-grey-100, text-tertiary -> text-text-tertiary
- text-token-secondary -> text-text-secondary
- bg-token-surface-primary/tertiary, bg-token-main-surface-secondary and
border-token-border-hover -> their semantic tokens
- bg-surface, bg-surface-50 -> bg-surface-primary
- bg-surface-primary-hover -> bg-surface-hover
- outline-hidden -> outline-none where focus styling already exists
- drop focus:shadow-outline, border-d-0 and the malformed
ring-offset-ring-offset, which have no meaningful replacement
MemoryArtifacts keeps its default outline instead of gaining outline-none,
since that button has no other focus indicator. MentionItem drops its dead
background rather than adopting one, which would have matched its hover
colour and erased the hover affordance.
Localize the two literal strings the pre-commit lint flagged in the touched
files, reusing the existing com_ui_upload_image and com_ui_more_count keys.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
|
45cc53c40b
|
🛰️ chore: bump @librechat/agents to v3.4.0, cover streamed subagent results e2e (#14647)
* test: cover streamed subagent results end to end * test: assert real e2e conversation id * test: harden streamed subagent e2e * test: stop incompatible subagent fixtures * chore: update @librechat/agents to version 3.4.0 in package.json and package-lock.json |
||
|
|
56175af0b5
|
🎟️ fix: Reconcile MCP OAuth Readiness Across Pods (#14629)
* fix: stabilize MCP OAuth readiness across pods * fix: harden MCP readiness review findings * fix: resolve CI type check and terminal OAuth polling * fix: address MCP OAuth readiness review * fix: align MCP OAuth readiness state * test: stabilize MCP OAuth readiness assertion * fix: reject stale MCP OAuth callbacks * fix: close distributed MCP OAuth readiness gaps * style: sort Redis MCP test imports * fix: preserve MCP OAuth polling across rolling pods * fix: finalize distributed MCP OAuth readiness * fix: preserve runtime-detected MCP OAuth * fix: report runtime MCP OAuth readiness * fix: preserve live MCP OAuth classification * style: sort MCP connection imports |
||
|
|
489bc02d4a
|
🧭 fix: Fail Closed When Expected MCP Tools Are Unavailable (#14646)
* fix: fail closed when expected mcp tools are unavailable * test: strengthen MCP handoff coverage * fix: clarify unavailable MCP tool guidance * fix: preserve MCP discovery for empty catalogs |
||
|
|
f738810c11
|
🧬 fix: Resolve URL-Named Model Spec to Its Full Preset on Cold Load (#14610)
* 🧬 fix: Resolve URL-Named Model Spec to Its Full Preset on Cold Load * 🧬 fix: Preserve Hidden Spec Names for Server-Side Resolution |
||
|
|
cdb60e74c2
|
⌨️ fix: Honor defaultPrevented in Global Shortcut Dispatch (#14570)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* ⌨️ fix: Honor defaultPrevented in Global Shortcut Dispatch * ⌨️ fix: Order-Independent Shortcut Yield via Window Listener * 📝 fix: Align Remaining Shortcut Contract Docs with Window Listener * 🧪 test: e2e Yield Contract Coverage for Global Shortcut Dispatch * 🧪 fix: Match Real Generation POST Path in Shortcut e2e |
||
|
|
cdf437dc5b
|
🪺 fix: Keep Preempt-Abandoned Siblings Nested, Make Message Tree Order-Robust (#14587)
* 🪺 fix: Keep Preempt-Abandoned Siblings Nested, Make Message Tree Order-Robust * 🔗 fix: Sever Cycle Back-Edges So the Returned Message Tree Is Acyclic * 🧪 fix: Satisfy TFile in buildTree Spec fileMap Fixture * 🌲 fix: Assert Repaired Trees in convoStructure Specs, Uncharge Self-Parent Edges * 📌 feat: Identity-Stable Sibling Selection Across Background Tree Churn * 🎭 test: E2E Coverage for Thread Fold and Sibling Selection Invariants * 🔑 fix: Treat Newest-Sibling Re-Key as Hydration, Not a New Branch * 🧭 fix: Rebind Sibling Selection Per Parent, Detect Appends by Membership |
||
|
|
e7f1838515
|
⚡ feat: Reliable Interrupt & Steer Escalation and Recovery (#14558)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* feat: surface interrupt-steer escalation on waiting messages The interrupt & steer feature shipped reachable only through the composer chord, the send-button hovercard, and the composer button; a message already waiting (queued for after the run, or steered and parked at the next tool boundary) had no path to it. Both waiting surfaces now carry one: - Queued rows get an icon-only ZapOff escalation button beside the existing Steer primary. It routes through sendQueuedNow, which now takes a preempt option on its live-run path. The tooltip teaches the composer chord, derived through resolveComposerKeyDown so a rebound or yielded chord is never advertised. - In-flight steer bubbles get an "Interrupt now" overflow entry with the same race rules as Edit: reclaim first, and only a `reclaimed` outcome resubmits (via retrySteer with preempt, swapping the chip for an interrupting one). `applied` and run-ended-mid-reclaim outcomes stop at the existing informational toasts, so the words can never land twice. Not offered on a steer already preempting. - Every during-run overflow menu gains an "Always interrupt instead" toggle for steerInterruptsByDefault, next to the existing steer/queue default toggle. MenuEntry supports disabled for the new entries. Only one interrupt can be unresolved at a time: while one preempt is pending (or the run is paused on approval, where the server 409s), every escalation control disables instead of racing the same seal. Ten new tests across both surfaces; 381 green in the affected suites. * fix: lock escalation across its reclaim window, keep the paused control visible, label as steer Codex round 1, all three findings. P2, escalation race. The single-interrupt invariant had a window between clicking "Interrupt now" and the reclaim resolving, where no preempt chip existed for the chip-derived gate to see: two bubbles escalated back-to-back could both resubmit. A shared escalating flag (Jotai, per-conversation) now covers the window and disables every escalation control on both surfaces, and a fresh recheck before resubmitting catches an interrupt armed elsewhere meanwhile (composer chord, queued row); those words re-home to the queue with an informational toast instead of breaking the invariant. P2, unreachable paused state. canSteer is defined as hasRealConvoId && !pausedOnApproval, so gating the button on canSteer removed it exactly when it was meant to render disabled; the test only passed on an impossible stub combination. The render gate is now duringRunActive && (canSteer || pausedOnApproval), and the test uses the real invariant. P2, label semantics. "Interrupt & send now" borrowed the name of the hard-abort action; this one preserves the partial answer and steers. Renamed to "Interrupt & steer now" (com_ui_interrupt_steer_now). Both behavior fixes counterfactually verified; 384 tests green across the affected suites. * fix: disable bubble escalation while the run cannot accept a steer Codex round 2, one P2. Answer mode (ask_user_question) sets duringRunActive false while pausedOnApproval stays false, since that flag only detects approval-bearing tool calls. The bubble's escalation entry stayed enabled there, so clicking it cancelled a healthy waiting steer and the preempt resubmission bounced off RUN_PAUSED, degrading the words to the queue. The entry now also disables on !duringRunActive, matching the queued-row control's gate. Counterfactually verified: reverting the gate fails the new answer-mode test. * fix: recheck live run state after the reclaim, not just at the click Codex round 3, one P2, and it is the round-1 recheck principle applied one level deeper: the entry-time disable cannot see a run that pauses (tool approval, answer mode) while the reclaim round-trip is in flight, and the .then closure held the render's stale steering controls, so the resubmit would fire into a RUN_PAUSED rejection after the reclaim had already surrendered the steer's boundary slot. The escalation continuation now reads the LIVE controls through a latest-ref: if the run can no longer accept a steer, the words re-home to the queue with an informational toast instead of resubmitting, and the resubmit itself also goes through the live controls. Counterfactually verified: reading the stale closure instead of the ref fails the new mid-reclaim pause test. * refactor: make escalation one atomic server-side arm, in place Codex round 4: four P2s, every one an interleaving of the same window — escalation as reclaim-then-repost is a compound, non-atomic operation whose continuation must revalidate the world (FIFO position lost, ref assigned too late, no run fence, competing bubble actions). Rounds 1-3 patched that window with a lock and rechecks; round 4 shows the window itself is the defect, so this removes it instead of guarding it again. Escalation is now POST /chat/steer/arm: the server flips preempt on the EXISTING queued item in one atomic store op (new IJobStore.armSteer; a decode-patch-encode LSET Lua on Redis, an in-place mutation in memory), fenced to the validated generation and refused once the queue closes. The handler mirrors the steer POST's preempt contract exactly: durable flag gated on the owner's recorded capability, volatile requestPreempt fire-and-forget because the durable flag is the truth resume/handover re-arm from. By construction this resolves all four findings: FIFO survives (the item never moves; the whole queue still drains in instruction order at the seal), there is no continuation to hold stale controls, the store op is fenced to the original run, and a competing Edit/Queue/Cancel either beats the arm (armed:false, chip untouched) or operates on the armed item, whose cancel already disarms. The client escalation entry becomes one mutation: armed:true relabels the chip in place (same steerId, same position), PREEMPT_UNSUPPORTED and lost races toast honestly, and the round 1-3 machinery — the escalating lock atom, the latest-ref, the post-reclaim rechecks and their two toast strings — is deleted rather than extended. Verified: 7 new handler tests on the real in-memory manager (including FIFO preservation and the stale-generation fence), 2 Redis integration tests against real Redis (in-place arm keeps order and every field; missing/stale/closed all refuse), client suites 396 green. * fix: decide capability inside the atomic arm, neutralize the lost-race toast Codex round 5, both findings, both edges of the new arm design rather than its mechanism. P2, capability TOCTOU. A HITL resume on a rolling deploy rewrites preemptCapable for the SAME generation, so the handler's read could go stale between validation and the flag flip, arming a steer the live owner cannot seal. armSteer now returns armed | missing | incapable, with the owner's live capability part of the same atomic predicate as the generation fence (HGET preemptCapable inside the Lua; the flat job field, not a metadata blob — the in-memory store reads the same field). The handler's pre-check is deleted rather than kept alongside; the store predicate is the single source. New handler test rewrites the capability after queueing and expects PREEMPT_UNSUPPORTED with the item left unflagged; the Redis guards test now asserts the incapable refusal against real Redis. P2, ambiguous toast. armed:false covers injected, cancelled, re-homed, and run-over alike, so telling the user the message "already reached the agent" claimed one specific outcome. The lost-race branch now uses a neutral message (com_ui_steer_arm_lost_race) and defers to the events for what actually happened. * fix: flip the escalation lock synchronously before the arm request Codex round 6, one P2. Round 4 deleted the escalating flag along with the reclaim continuation it guarded, but that left the one-interrupt gate blind during the arm request's own round trip: the chip-derived check cannot see an arm until its response relabels the chip, so on a slow connection two bubbles could both arm before either response landed. Double-arm is harmless server-side now (the run seals once and drains the whole queue in order), but every escalation control advertises "one interrupt at a time" by disabling, and the controls must tell the truth. The per-conversation escalating flag returns as a pure UX gate: set synchronously at click, before the mutation, cleared on settlement, and folded into interruptPending on both surfaces. Unlike its round 1-3 ancestor there is no continuation behind it to guard and no recheck to pair with it. Counterfactually verified: without the synchronous set, the two-bubble race test arms twice. 207 tests green across the Chat Input suites. * test(e2e): cover escalation of waiting messages through the real seal Three mock-harness tests on E2E_SLOW_REPLY, a 160-chunk stream with no tool boundary, so an in-thread steer part can ONLY come from a genuine mid-stream seal — which makes each test a behavioral proof rather than a UI check: - Queued row escalation: the ZapOff button turns a waiting queued message into a preempt-armed steer (202 echoes preempt: true) that seals and injects, where the sibling steering.spec test proves the unescalated path waits for run end instead. - Bubble in-place arm: an ordinary steer (202 with no preempt echo) waits as a bubble, POST /chat/steer/arm answers armed: true, the bubble relabels in place (same single bubble, same text, escalation no longer offered on reopen), and the armed steer seals mid-stream. - Always-interrupt toggle: flipped from a waiting row's overflow menu, plain Enter now produces a preempt: true steer that seals in the SAME run, and the menu offers the way back. An afterEach clears the localStorage preference so a mid-test failure cannot leak preempt-by-default into the rest of the serial suite. All three verified locally through the full harness (real backend, mock LLM, seeded DB): 3 passed in 27s. * feat: dedicated escalation arrow + shortcut, menu split into actions and preferences The escalation was still half-hidden: the bubble only offered it inside the overflow menu, and the tooltip taught the composer chord, which does a different thing (interrupts with typed text, not this chip). Three changes make it a first-class command: - A shared EscalateNowButton (circular arrow, ghost-bordered like the composer's interrupt control) is always visible on BOTH surfaces: beside each queued row's Steer primary and on every waiting steer bubble next to its menu. It disappears once a steer is interrupting. - A dedicated registry shortcut, escalateSteer (Cmd/Ctrl+Shift+.), editing-allowed and rebindable like every other action. Deliberately NOT an Enter chord: the composer owns every Enter chord, and the yield design rests on no default binding using Enter besides submit. Its handler clicks the newest enabled arrow control (bubbles beat queued rows), so the shortcut can never diverge from the button, and the arrow's tooltip teaches THIS command via the registry display. - The overflow menus separate one-off actions from sticky behavior changes: Edit, Cancel, Queue, then a smaller "Preferences" section holding the queueing and always-interrupt toggles, each with the standard InfoHoverCard reusing the Settings panel's descriptions. "Interrupt & steer now" leaves the menu entirely. 386 client tests green, including a menu-structure test locking the order and the absence of the escalation entry; bubble escalation tests drive the visible arrow. The e2e spec's bubble test now clicks the arrow, and a fourth test drives the dedicated shortcut end to end through a real mid-stream seal. * style: bind the escalation arrow to its message (variant A anatomy) Two same-weight circles in a row read as one control group, leaving the arrow's ownership ambiguous, and a floating arrow stops meaning anything once several messages stack. The shared control now carries variant A's anatomy: a thin divider binds a small SOLID arrow (filled, inverted) to the message region on its left, and the menu ellipsis stays a bare glyph, so the two affordances can no longer blur together — and the divider+arrow pairing repeats cleanly per chip at N messages. * chore: drop the unused within import CI lint caught * fix: advertise the escalation shortcut only while the control is live Codex on the e2e head, one P2: the tooltip appended the chord hint even while the button was disabled, advertising a shortcut that does nothing during an approval pause. The flagged control (InterruptNowButton) was since replaced by the shared EscalateNowButton, which inherited the pattern; the successor now omits the chord whenever the control is disabled, matching the rule the during-run hovercard already follows. * fix: harden steer escalation lifecycle and recovery * test(e2e): disambiguate accessible steer preferences * test: align abort persistence coverage with prerequisites * chore(i18n): remove obsolete steer race message * chore: normalize imports across steering changes * test: exercise stream integration on Redis Cluster * test: scope HITL checkpoints to generation * test: fix cluster cleanup and locale policy * fix: keep escalation visible during ask pauses * fix: fence recovery downgrade and stale predecessors * fix: require generation owner abort acknowledgement * fix: validate delayed preempt arms * test: align final escalation fixtures * fix: preserve in-memory predecessor abort handoff * fix: restore controls for recovered queued messages * test: cover recovered queue controls * fix: close final steering review gaps |
||
|
|
60ca751a7f
|
🧠 fix: Preserve Deferred Tool Schemas Across HITL Resume (#14552)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🧠 fix: Preserve deferred tool schemas across HITL resume * 🧪 test: Harden deferred tool resume regression * 📦 chore: bump @librechat/agents to v3.3.10 |
||
|
|
52b2ebf948
|
🧪 test: Run mock E2E against Redis in shards (#14551)
* 🧪 test: Run mock E2E against Redis in shards * 🧪 test: Isolate local Redis E2E data |
||
|
|
3f02efdef9
|
⚡ feat: Interrupt & Steer (Initial UI) (#14528)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🛑 feat: Preemptive Steer — server half (Interrupt & Steer, PR 2 of 3)
Lets the steer route ask the generating replica to seal its live model
stream at the next provider-safe boundary instead of waiting for a tool
step. The run is never aborted, job status never changes, the partial
answer is kept, and generation resumes in the same assistant message
after the injected steer. Consumes the SDK seam in @librechat/agents
(danny-avila/agents#335, #346).
Transport: IEventTransport gains a fenced emitPreempt/onPreempt pair
beside abort. RedisEventTransport fans PREEMPT out on the SAME events
channel and subscription (no new connection, key, or subscribe call);
onPreempt returns a registration-scoped unsubscribe with the same
replacement-safe state-identity guard onAbort uses. InMemory implements
neither — single-process preempt lives entirely in the runtime set.
Runtime state: RuntimeJobState carries the per-generation request set,
createdAt-fenced and capped at STEER_QUEUE_MAX_DEPTH, plus a bounded
`cleared` tombstone so a late cross-replica arm cannot resurrect a
request whose steer already drained. registerPreemptSubscription
mirrors the abort registration's double fence (runtime identity +
generation createdAt); releaseAbortSubscription retires BOTH listeners
and the armed set, so every terminal path drops preempt state for free.
Public surface: requestPreempt (arm + fenced publish, never a rejection
surface, never touches job status), isPreemptRequested (O(1)
level-triggered poll), noteSteersRemoved (drain/cancel bookkeeping +
fenced clear), clearPreemptRequests (empty-boundary disarm).
One drain body, two boundaries: createSteerDrainHook (PostToolBatch)
and createSteerPreemptBoundaryHook (PreemptBoundary) share
drainAndBuildInjections, so the two injection sites cannot drift — the
SDK's provider-safety argument rests on identical HumanMessage shapes.
The shared body builds injections incrementally under a swallow-all
catch (a mid-loop throw still injects what was applied — those parts
are already persisted), clears preempt requests in finally, and
disarms the generation when a boundary drains nothing.
Request path: POST /chat/steer accepts preempt: true. The guard ladder
is unchanged in order and in every status code. A preempt request is
NEVER a rejection reason — without the capability the steer still
enqueues and the 202 echoes preempt: false. Armed strictly after a
successful enqueue; cancel disarms. The capability is read from the
OWNING replica's recorded `preemptCapable` rather than the route
replica's own SDK probe, so a rolling deploy cannot label a steer
"interrupting" that the older owner will only inject at a tool step.
Durable label: SteerQueueItem.preempt → TPendingSteer.preempt, so a
parked/claimed/replayed chip keeps its wording.
Run wiring: createRun registers the PreemptBoundary hook and threads
RunConfig.preemption, both gated on isSteerPreemptSupported() — a
separate probe from isSteeringSupported(), so the client affordance can
never arm against an SDK that only injects at tool boundaries.
buildSteerWiring builds both hooks from one shared closures object, so
preemption survives HITL pause/resume for free.
Honest finalization: an empty preempt boundary persists and emits with
unfinished: true — the same contract an abort gets — re-marked
explicitly because BaseClient has already saved the row as
unfinished: false by that point.
Not changed: no new job status, store method, Lua, SSE event type,
endpoint, or authorization surface. abortJob, completeJob,
transitionStatus, closeAndDrainSteers, getResumeState, emitChunk,
applySteerPart and the whole abort path are untouched.
Tests: 120 packages/api steering specs (preempt lifecycle, tombstone,
fences, caps, terminal release, both-boundary drain parity,
level-triggered poll, request/cancel arming, owner-capability
degradation) plus 5 in api for buildSteerWiring gating, and 2
Redis-gated cross-replica transport specs.
* 🔒 fix: Codex round 2 — evict tombstones, scope the empty-boundary disarm, honest resumes
All four server findings were fresh consequences of the round-1 fixes,
which is the review doing exactly what it should.
- Tombstone cap refused new entries instead of evicting. Every drained
or cancelled steer is tombstoned, not just preempting ones, so a
generation that processed 20 steers exhausted the set and the
late-arm race resurfaced silently. Now evicts oldest-first (Set
iteration is insertion-ordered), with the budget named
PREEMPT_TOMBSTONE_MAX rather than an inline expression.
- The empty-boundary disarm I added in round 1 wiped the generation's
ENTIRE armed set. A second steer can enqueue and arm between the
atomic drain returning empty and the disarm running — that arm is
backed by a live, uninjected queue item and must survive. The drain
now snapshots the armed ids BEFORE draining
(getArmedPreemptIds) and clearPreemptRequests takes an explicit id
list instead of clearing everything.
- HITL resume finalized with a hardcoded unfinished: false. The
boundary hook is re-registered on resume via buildSteerWiring, so a
resumed segment can end on an empty preempt boundary exactly like a
fresh one; finalizeResumedTurn now reads getPreemptStats() and the
halt reason, matching the normal request path.
- Ownership moves on resume, so the job's recorded preemptCapable must
describe the replica that will actually generate. Refreshed before
resumeCompletion; a job created on a capable replica that resumes on
an older one during a rolling deploy no longer acknowledges steers as
interrupting.
Tests: +3 (scoped disarm sparing a post-snapshot arm, oldest-first
tombstone eviction, id-list disarm). 122 packages/api steering specs
green.
* 🚨 fix: Codex round 3 — deserialize preemptCapable from Redis (feature was dead under Redis)
The P1 here is the most consequential defect in the whole feature, and
it was introduced by round 1's own capability fix.
- `RedisJobStore.serializeJob` writes booleans generically, so
`preemptCapable` reached Redis — but `deserializeJob` is an EXPLICIT
field map and had no line for it. Every `getJob()` therefore dropped
the flag, `job.metadata.preemptCapable` was always undefined, and
`handleSteerRequest` computed `preemptArmed: false` unconditionally.
Interrupt & steer would have silently degraded to ordinary
tool-boundary steering in EVERY Redis deployment — i.e. the feature
shipping as a no-op in production while passing every in-memory test.
Now deserialized, with a round-trip assertion in the metadata spec
that fails (`Received: undefined`) against the unfixed store.
- The resume capability refresh moved from just-before
`resumeCompletion` to immediately after `approvals.resolve` claims
the run. That call already flips the job back to `running`, so the
steer route accepts requests from that instant; leaving the refresh
135 lines later (across the whole client reconstruction) left a real
window where a steer read the PREVIOUS owner's capability. Not the
fully atomic transition Codex suggested — that reaches into the
approvals Lua — but it shrinks the window from seconds to one await,
which is proportionate for a label-accuracy issue.
Refuted: "avoid triggering preemption inside subagents". The premise —
that the run-wide poll can seal a subagent stream — does not hold
against the shipped SDK. Child graphs are constructed with
`subagentScope: true` (SubagentExecutor) and `preemption` is NOT
propagated into child inputs, while `canClaimPreemptSeal()` requires
`!subagentScope && preemption != null`. Both conditions fail
independently, so a subagent can never claim a seal and the boundary
cannot fire with `agentId` set. The `input.agentId != null` guard in
the hook is defensive depth, not the thing standing between us and the
described failure.
140 packages/api specs green.
* 🔁 fix: Codex round 4 — re-arm durable interrupt steers when resume moves owners
- An arm lives only in the owning replica's runtime plus a transient
pub/sub message, while the steer's `preempt` flag is durable on the
queue item. A HITL resume landing on a different replica therefore
started with an empty armed set and a poll stuck false, so an
interrupt the user had already been ACKed for silently waited for an
ordinary tool boundary. New `GenerationJobManager.rearmQueuedPreempts`
rebuilds the armed set by peeking the durable queue (fenced on the
generation) and re-arming every item flagged `preempt`; resume calls
it right after claiming. Safe by construction: every item peeked is
still queued, so no drained steer can be resurrected.
- Capability-refresh failure now logs at error rather than warn, but
deliberately does NOT fail the resume — see the reply on that thread.
Tests: +2 (rebuild from queue arms only the flagged item and reports
the count; a stale generation arms nothing). 124 packages/api steering
specs green.
* 📡 fix: Codex round 5 — acknowledge only what was actually armed
- A cross-replica arm was fire-and-forget: `emitPreempt` logged its own
publish failure and `requestPreempt` returned void, so the route
answered `preempt: true` even when the owner never armed a poll. The
steer still injected at the next tool boundary, but the chip claimed
an interrupt that could not happen — and unlike HITL resume, an
ordinary running generation had no durable reconciliation to recover
it.
`emitPreempt` now resolves to the subscriber count and rejects on
failure; `requestPreempt` is async and returns whether the arm truly
landed (owned locally, or delivered to at least one subscriber). The
202 reports THAT rather than what was asked for, so the chip relabels
to ordinary steering exactly as it does for a capability-degraded
deployment. Errors are swallowed into `false` — an unarmed interrupt
is a downgrade, never a failed steer.
- The owner capability is re-read immediately before enqueue rather
than reused from the top of the guard ladder. `checkAgentAccess` and
file resolution are awaits, so a request can span an entire HITL
pause/resume that moves ownership to a replica with different
capability and rewrites that very flag. Only paid for by requests
that actually asked to interrupt.
Tests: +3 (not-armed when the publish reaches nobody; armed when this
replica owns the generation; a throwing publish downgrades instead of
propagating). 127 packages/api steering specs green.
* 🎯 fix: Codex round 6 — real ownership, confirmed disarms, and a CI regression of my own
Three review findings plus three CI failures the round-5 commit caused.
Review:
- Ownership came from `runtimeState`, which a cross-replica `getJob`
populates with a FACADE runtime on any replica that merely read the
job. Matching `createdAt` therefore proved only "we looked at this
job", so a non-owner could arm nothing and report success. Ownership
now comes from `ownedJobs`, the actual owner map.
- `armPreemptIds` returns how many ids it accepted, and a local arm is
only reported as armed when one was. A tombstoned id (its steer
drained at an ordinary boundary mid-request) no longer answers
`preempt: true` for an interrupt that cannot happen.
- The cancel disarm is awaited. A dropped clear is worse than a dropped
arm: the owner keeps a level-triggered request for a steer that no
longer exists, seals its next chunk and truncates an unrelated
answer. The boundary drain's own call stays non-blocking — there the
owner is local, so the disarm is already effective and awaiting the
informational publish would only delay injection.
- Subscriber count is NOT read as proof of owner receipt: the count
includes this replica's own facade subscription. A successful publish
reports armed, a rejected one does not. Documented rather than
papered over — see the acknowledgement-semantics note on the PR.
CI regressions from round 5, all mine:
- `registerPreemptSubscription` was AWAITED at both runtime-init sites,
so job creation blocked on a second Redis channel subscription and
hung when that subscribe was slow. Abort is awaited because a missed
abort strands a run; a missed preempt only degrades that steer to the
next tool boundary, so it now registers without gating createJob.
- Two api specs mocked `@librechat/api` without the newly imported
`isSteerPreemptSupported`, so the call threw before createJob; and one
exact-match assertion needed the new `preemptCapable` metadata field.
- My own Redis integration spec asserted arm-before-clear ordering,
which two publishes carry no guarantee of — the receiving tombstone
exists precisely because of that. Now asserts delivery and payload
fidelity, order-independent.
158 packages/api specs, 27 api specs green.
* 🧭 fix: Codex round 7 — settle the acknowledgement semantics (Option A)
Round 7's second finding is the incoherence I flagged on the PR: the
route persisted `preempt: true` on the durable queue item while
returning `preempt: false` when delivery could not be confirmed. Those
two then disagreed, and `rearmQueuedPreempts` trusts the DURABLE one —
so a resumed owner would honour an interrupt the client had explicitly
been told degraded to ordinary steering.
Rather than patch the disagreement, this settles the meaning:
`preempt` in the 202 means "queued as an interrupt request", NOT "a
seal is guaranteed". It mirrors `SteerQueueItem.preempt` exactly, so
the response, the durable record, and the resume-time re-arm can never
disagree. The gates that ARE knowable stay — the owner's recorded
capability and a successful enqueue. Everything past that degrades to
the documented fallback of injecting at the next tool boundary.
A route cannot synchronously know whether another replica will seal:
proving it needs a correlated request/response over pub-sub, and even
that only proves the owner heard, not that it is still streaming when
the arm lands. Four rounds of tightening this boolean each surfaced a
narrower case; the sequence does not converge, so the invariant is now
"the flag describes the durable decision" and an unconfirmed arm logs a
warning instead of rewriting the answer.
Also from this round: a failed disarm publish is retried once and its
outcome reported. `handleSteerCancel` keeps `removed: true` — the steer
really did leave the queue, and saying otherwise would make the client
re-show a chip for a steer that can never arrive — and adds
`disarmed: false` so the residual risk is visible rather than swallowed.
Damage stays bounded regardless: the empty-boundary self-clear disarms
the generation after a single seal.
Tests: +1 pinning the response/durable-flag invariant. 159
packages/api specs green.
* 🧹 fix: Codex round 8 — remove the unverifiable disarm signal
Round 8 found the same over-promise on the disarm side that round 7
corrected on the arm side, so this applies the same answer rather than
patching around it.
The `disarmed: false` field added in round 7 was both unreliable and
unused: a resolved publish is not proof the owner heard it (the
delivery count includes this replica's own facade subscription), and it
was never threaded into `CancelSteerResponse` or read by any client. A
signal that claims a certainty the transport cannot provide is worse
than no signal — it invites callers to trust it.
Removed from the response. The retry stays, because it genuinely
reduces the failure rate, and `noteSteersRemoved` still returns whether
the publish succeeded FOR LOGGING, now documented explicitly as
"published without error", not "the owner disarmed".
Disarm is best effort with a bounded, self-healing failure: if the
clear is lost the owner seals once, the empty-boundary self-clear
disarms the generation, and the turn is persisted `unfinished: true`
rather than silently truncated. Tightening that further needs a
correlated request/response over pub-sub with a timeout — noted on the
PR as the deliberate boundary of this design rather than an oversight.
130 packages/api steering specs green.
* 🧽 fix: Codex round 9 — spend snapshot arms on nonempty drains too
The round-6 scoping fix only cleared the pre-drain snapshot when the
drain came back EMPTY. On a nonempty drain the `finally` cleared just
the drained ids, so a stale arm — typically a cancel whose
cross-replica clear was lost — survived the boundary. It would then
immediately seal the continuation meant to answer the steer that had
just been injected, and land on an empty boundary as
`preempt_incomplete`: the interrupt appears to work, and the answer to
it is truncated.
A boundary that runs has spent its seal, so everything armed at
snapshot time is spent whether or not it came back from the drain. The
`finally` now clears the union of the snapshot and the drained ids.
Arms that land AFTER the snapshot are still spared — their queue items
are live and uninjected, which is the property round 6 added.
Also fixes an api-workspace CI failure of mine: `resume.spec.js` stubs
`GenerationJobManager` wholesale, and the round-3/4 resume work added
two calls (`updateMetadata`, `rearmQueuedPreempts`) the stub did not
define, so 34 specs threw. Stub extended.
Tests: +2 (a nonempty drain clears a stale snapshot arm; a nonempty
drain spares an arm that landed mid-drain). Counterfactually verified —
the stale-arm spec fails against the unfixed drain. 132 packages/api
specs, 60 resume specs green.
* fix: never let a failed preempt subscription reject into the void
registerPreemptSubscription is called detached at both sites, so a
rejected Redis SUBSCRIBE became an unhandled rejection — process-fatal
under Node's default --unhandled-rejections=throw. The comment already
promised this path merely degrades steering; it now does.
Swallowed and logged inside the registration rather than at each call
site, so a future third caller cannot reintroduce the trap. Losing the
channel costs this generation's cross-replica preempts, not the server:
same-replica arming is runtime state and still works, and remote arms
fall back to the next tool boundary.
Verified counterfactually — the new spec surfaces SUBSCRIBE failed as an
unhandled rejection against the unfixed registration.
* docs: state the real blast radius of a failed preempt subscription
LibreChat's own entrypoints install a global unhandledRejection handler
that logs and keeps serving, so the escaping rejection this guards was
never fatal to this server — only to another consumer of @librechat/api
that installs no handler. The fix stands either way; the comment just
should not overstate what it prevents.
* test: cover the cross-replica preempt hop with two manager instances
Every other preempt test runs against a single manager, so the hop that
actually carries an interrupt in production had no coverage: the steer POST
lands on whichever replica the balancer picks, which is usually not the one
generating. Non-owner publishes, owner arms, owner's level-triggered poll
flips — none of that was exercised end to end.
Two GenerationJobManagerClass instances are a faithful replica pair here.
runtimeState and ownedJobs are private instance fields, there is no
module-level mutable state between them, and createStreamServices duplicates
a dedicated subscriber connection per call, so separate OS processes would
exercise the same objects over the same Redis.
Both assertions verified counterfactually against real Redis:
- Deleting the preemptCapable deserialization in RedisJobStore fails this
with 'Expected: true, Received: undefined' — the exact P1 that shipped past
every in-memory test and would have made the feature a silent no-op on
every Redis deployment.
- Dropping the non-owner arm publish fails it with 'Received: false'.
* test: remove the fixed sleeps and vacuity from the cross-replica preempt test
Codex round 11, both findings, both on the test I added last commit.
P2 — the 300ms waits were load-bearing. Redis pub/sub never replays and the
owner's SUBSCRIBE is detached, so on a slow CI worker the publish could land
before anyone was listening and the test would fail against correct code.
Now it republishes until the owner's state converges, which is safe because
arms and clears are idempotent set writes keyed by steerId. Side effect: the
tests got ~10x faster (85ms/57ms vs 929ms/606ms) since they finish on
delivery rather than on a timer.
P3 — afterEach destroyed only the transports, leaving each manager alive in
its own cleanup-interval closure, still working against a dead transport.
Now tracks the managers and awaits destroy(), which disposes the job store
and its timer too. Matches how the rest of this file cleans up.
Fixing the sleeps exposed a third problem codex did not flag: the stale-arm
test could pass vacuously, because an undelivered arm and a fenced one look
identical. It now brackets the stale publish between two control arms — the
first proves the owner is listening before the stale one is sent, the second
proves it has had its chance to arrive.
Verified counterfactually against real Redis, and stable over 5 runs:
- dropping the preemptCapable deserialization fails with 'Received: undefined'
- dropping the non-owner arm publish times out both tests
- removing the generation fence fails the stale test with
["control-before", "steer-stale", "control-after"] — which also confirms
the bracketing orders as intended rather than by luck
* fix: gate interrupt on the OWNER's capability alone, not the route's
Codex round 12. The comment above this gate already said 'the OWNER's
recorded capability, not this replica's probe' — and then the code ANDed in
isSteerPreemptSupported(), which is exactly this replica's probe. The
contradiction dates to the original commit; round 6 made the gate
owner-scoped and wrote that comment without removing the local conjunct.
The route never seals. It enqueues and publishes an arm, neither of which
touches the SDK, so during a rolling deploy a steer landing on an
un-upgraded replica silently lost its interrupt even though the owner could
seal. When the route IS the owner the probe is redundant anyway: the flag it
would consult is the one this process wrote at createJob.
The real degradation path is unchanged and still tested — an owner that
recorded no capability relabels to an ordinary steer. The test that pinned
the local probe asserted an impossible same-replica state (capable metadata
plus an incapable local SDK, when the metadata is written from that probe);
it now pins the mixed-SDK direction instead, and fails with
'Expected: true, Received: false' if the probe is put back.
* fix: reconcile arms at handover, and stop holding the 202 on a publish
Codex round 13, two of three findings.
P2 — rearmQueuedPreempts only ever ADDED. A replica that merely read the job
still installs a facade runtime and subscribes, so it can accept an arm and
then miss the best-effort clear that follows the drain. HITL resume promotes
that facade to owner, the union keeps the orphan, and the first resumed
stream seals on a steer no longer in the queue, drains nothing, and
truncates the resumed answer as preempt_incomplete. acquireResumedJobOwnership
only sets ownedJobs, so nothing else was clearing it. The durable queue is
the sole authority at a handover: arms it does not back are now disarmed and
tombstoned, so an in-flight publish cannot revive them either.
Worth recording that my own independent review raised this and my verifier
refuted it. Codex found it separately; two reviewers converging should have
outweighed one refutation.
P2 — the route awaited the arm publish before answering. The 202 reports
capability, not delivery, so the await could not change the response; it only
exposed the caller to Redis latency after the queue item was already durable.
A client that times out and retries mints a second steer while the first
stays queued, injecting the same instruction twice, whereas a lost publish
merely takes the tool-boundary fallback. Detached, with both outcomes logged.
All three tests verified counterfactually: union-only rearm fails the two new
handover specs, and re-awaiting the publish hangs the stalled-publish spec
until jest kills it.
* fix: snapshot arms before reading the queue at handover
Codex round 14 — a regression from my own round-13 fix, and a worse failure
than the one it corrected.
Round 13 read the durable queue first, then tombstoned any armed id the
snapshot did not back. But approvals.resolve reopens steering before
reconciliation runs, so another replica can commit a preempt steer and
publish its arm while the peek is in flight. That arm is then present locally
but absent from a snapshot taken before the steer existed, so a LIVE
interrupt the route already acknowledged got dropped — and tombstoned, which
blocks the re-arm, making it unrecoverable rather than merely late.
Fixed by inverting the two reads rather than by locking or paying a second
round trip. A steer is durably enqueued BEFORE its arm is published, so any
id in an arms-first snapshot was already queued when it was armed, and the
later peek must observe it unless it has since drained — which is exactly the
orphan this reconciliation exists to drop. Arms landing after the snapshot
are simply not candidates.
Also re-checks runtime identity across the await, since the generation can be
replaced while the queue read is in flight.
New spec injects a steer + arm during the peek and verifies it survives;
against the round-13 ordering it fails with Received array: [].
* fix: bound the cancel disarm wait and fence enqueue to its generation
Codex round 15.
P2 — the cancel awaited its disarm publish unbounded. ioredis queues
commands during an outage rather than rejecting, so that await could hang for
the length of the outage with the steer ALREADY durably cancelled; a client
that gives up then treats the cancel as failed and restores a chip for a
steer that can never produce an applied event. Every successful cancel
publishes, so ordinary steers were exposed too, not only preemptive ones.
Now bounded at 1s, with the publish continuing behind it — its retry and
logging are unchanged, it is just no longer in front of the response. This is
the sibling of round 13's arm-publish finding; I fixed one path and left this
one.
P3 — enqueue was not fenced to the generation the capability decision was
made against. The access checks, file resolution and owner re-read are all
awaits, so the run can be replaced before the enqueue: the item then lands on
the REPLACEMENT queue while the durable preempt flag and the arm still name
the previous epoch, the arm is fenced out at the owner, and the 202 promises
an interrupt that cannot happen. enqueueSteer now takes an expected
generation, mirroring drain/peek, and the Redis path enforces it inside
STEER_ENQUEUE_LUA so the check is atomic with the push rather than racing it.
All three new specs verified counterfactually, including the Lua guard
against real Redis (removing it returns 1 where -1 is required).
* ⚡ feat: Interrupt & Steer — client half (PR 3 of 3)
Makes preemptive steering reachable. Consumes the server contract from
PR 2 (POST /chat/steer `preempt`, echoed on the 202) and the SDK seam
in @librechat/agents 3.3.5.
Settings shape follows the agreed correction, NOT the earlier plan
draft: `steerInterruptsByDefault` is a boolean ORTHOGONAL to
`duringRunDefaultAction` — that enum still chooses steer-vs-queue, the
new boolean chooses how soon a steer lands. This deliberately avoids
widening the enum to three values, which would have silently broken two
hard-coded binary TOGGLES (`DuringRunAction.tsx`'s setter and
`SteerMenu`'s `useDefaultToggleEntry`, both `prev === 'steer' ? … : …`)
where a third value collapses to the wrong branch and one click erases
the setting.
- useSteering: `submitSteer` takes an opts bag and threads `preempt`
into the POST, the optimistic chip, and the failure chip. The ACK
relabels from the SERVER's echo, so a deployment that cannot seal
mid-stream downgrades the chip's wording instead of erroring — the
entire UX surface of capability degradation. New `interruptSteer`
reuses the whole chip lifecycle and degradation ladder, and falls
back to `interruptAndSend` when `!canSteer`, because steering needs a
server-side job and an always-visible button would otherwise be dead
for the whole first turn. `steerFromComposer` honours the new
preference.
- Composer: always-visible `InterruptSteerButton` with one fixed
meaning (stop now, keep what's written), disabled on a paused run to
pre-empt the server's 409, `type="button"` so it never steals the
form's Enter submit, RTL-correct margins. A fourth hovercard row on
the during-run send button, and ⌘/Ctrl+Shift+Enter routed AHEAD of
the bare ⌘/Ctrl+Enter branch that would otherwise swallow it.
- Chips: an in-flight preempt chip reads "Interrupting" with a ZapOff
glyph; `preempt` survives reconnect through `seedSteerChips`.
- `RunEnd.interruptArmed`, `drainAfterAbortByIndex`, `useQueueDrain`,
`stopGenerating` and `interruptAndSend` are untouched — the preempt
path deliberately shares none of the abort machinery.
Tests: 7 new specs (posts preempt, turn-1 fallback, empty-text refusal,
default route with and without the preference, server-echo relabel,
double-click). 66 useSteering specs green; tsc and lint clean.
Round-1 review fixes folded in:
- P1: interrupt & steer no longer hard-aborts a run paused on tool
approval. `canSteer` is false there, so the fallback was routing the
keyboard and hovercard paths into `interruptAndSend` — discarding the
partial answer, the exact opposite of what the action promises. The
fallback is now scoped to the missing-conversation case only, and a
paused run refuses outright (the standalone button was already
disabled; the guard now lives where all three paths reach it).
- The preference no longer leaks into the explicit Steer action.
`steerFromComposer` backs both the default Enter route AND the
explicit hovercard row / Ctrl+Enter alternate; applying
`steerInterruptsByDefault` inside it made ordinary Steer interrupt and
the two rows indistinguishable. It now takes an explicit argument that
only `submitDuringRun`'s default route sets.
- Retry preserves preemption: a failed interrupt-steer chip keeps
`preempt: true`, and `retrySteer` now forwards it rather than silently
resubmitting as an ordinary tool-boundary steer.
- ⌘/Ctrl+Shift+Enter defers to a rebound submit shortcut, mirroring the
bare ⌘/Ctrl+Enter branch — a user who bound submit to that chord keeps
getting submit.
Round-2 fix: the preempt label now survives the page-reload resume path
too. `seedSteerChips` (useResumableSSE) and `restoreSteerChips`
(useResumeOnLoad) are two independent TPendingSteer→PendingSteer
mappers with near-identical bodies; the first carried the flag and the
second silently dropped it, so an armed interrupt reverted to plain
"Steering" after a reload. Swept: those are the only two in production
code. The reclaim/convert paths deliberately omit it — a queued
follow-up starts its own turn, so there is nothing to interrupt.
* fix: yield the interrupt-steer chord only to a submit shortcut bound to it
The previous guard skipped the Ctrl/Cmd+Shift+Enter branch whenever ANY
submitMessage override existed. Rebinding submit to something unrelated
(Ctrl+J) or unbinding it entirely then fell through to the override
resolver, which returns 'none' for shifted Enter — silently removing the
shortcut the hovercard still advertises.
Compare the pressed chord against the configured one instead. The
adjacent bare Ctrl/Cmd+Enter branch keeps its any-override guard on
purpose: that chord IS the default submit chord, so once submit moves
the resolver should own it.
The predicate already existed inside resolveSubmitOverrideAction; pulled
it out as bindingsMatch so both sites compare chords the same way. That
call is behavior-preserving — eventBinding.key is 'Enter' by the early
return, and equal hashes imply equal keys, so the dropped explicit key
check was redundant.
* fix: disable the Interrupt & steer menu row while paused on approval
interruptSteer hard-refuses when the run is paused for tool approval, but
the hovercard row was never gated, so it rendered enabled and clicking it
did nothing at all — no chip, no queue entry, no toast — at exactly the
moment a user is trying to say "stop, don't run that command". The
standalone button already gates on pausedOnApproval; the row contradicted
it.
Gated on pausedOnApproval rather than !canSteer like the steer row above,
because canSteer is also false before a conversation exists, where
interruptSteer deliberately falls back to interruptAndSend and the row
must stay live for the whole first turn.
Tests pin both directions and were verified counterfactually: removing the
gate fails the paused case, and using !canSteer fails the first-turn case.
* test: render the during-run hovercard eagerly instead of driving Ariakit
The new spec passed locally and failed all four cases on CI's Ubuntu and
Windows shards: Ariakit's show path keys off pointer geometry, which jsdom
reports as zeros, so whether a synthetic mouseEnter opens the hovercard is
environment-dependent. Driving it was testing Ariakit's hover behavior, not
which rows this component disables.
Mocking the three Ariakit primitives renders the rows unconditionally and
drops the fake timers. Both counterfactuals still fail as they should:
removing the gate fails the paused case, !canSteer fails the first-turn case.
* test(e2e): cover interrupt & steer sealing mid-stream
The mock Playwright suite covered every sibling during-run action — steer at
a tool boundary, steer degrading to a queued follow-up, queue, and interrupt
& send — but not interrupt & steer, the one this stack adds.
Uses E2E_SLOW_REPLY, which streams pure text with no tools, so the scenario
is the same one where an ordinary steer provably degrades to a queued
follow-up turn. Injecting in-thread there is something only a mid-stream
seal can do, which makes the assertion discriminating rather than incidental:
the steer part lands in the response, the final chunk never arrives, the text
written before the seal survives, and no follow-up turn pair is created.
* test(e2e): assert the run resumes after the seal, not just that it sealed
The other four assertions are all satisfied by a seal that killed the run:
the steer part is persisted by applySteer during the drain, before the
continuation starts, so 'sealed and resumed' and 'sealed and died' were
indistinguishable — and resuming is the whole difference from interrupt &
send.
The continuation answers the injected steer, whose text carries no
fake-model marker, so getLatestUserText falls through to the default reply.
That string ('E2E mock reply') is distinct from the setup turn's
('E2E reply <label>'), so seeing it proves generation restarted rather than
matching text that was already on screen.
The test itself is confirmed working: it ran as 104/121 in the Playwright
job on
|
||
|
|
91adcf3f2c
|
🪶 fix: Yield Soft Default Model Spec to Agent Picks in Picker-Only Deployments (#14515)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🪶 fix: Yield Soft Default Model Spec to Agent Picks in Picker-Only Deployments The `hasEphemeralModelOptions` gate makes the soft default canonical whenever the selector offers no ephemeral endpoint → model options, so lingering endpoint/model residue never strands a new chat on an unselectable endpoint. That gate swept in agent and assistant selections too: under an agents-only allow-list (`addedEndpoints: [agents]`), every New Chat re-armed the soft spec and discarded the agent the user had just selected, with no way to make the choice stick. An agent pick is the one real selection a picker-only deployment offers, so it now yields like any other selection, while endpoint/model residue keeps falling to the soft default. - Add `hasSelectableEntitySelection`: the stored setup yields when it names a non-ephemeral agent_id (or an assistant_id) on an endpoint the allow-list and endpoints config still expose. Ephemeral ids, and picks whose endpoint has since left the allow-list, stay residue so a stale entity cannot strand a new chat. - Invert the three unit cases that asserted the soft default outranking a stored agent under an agents-only allow-list; add coverage for assistants, prioritized configs, ephemeral agent ids, endpoint/model residue, an endpoints config without agents, and the pre-load allow-list path (35 cases, was 29). - Add an e2e regression test: under an intercepted agents-only allow-list, a selected agent survives New Chat and a cold load, while a cleared instance still lands on the soft default. * 🧹 chore: Type the Intercepted Startup Config in the Soft Default E2E The agents-only allow-list interception cast the `/api/config` response to `{ modelSpecs?: Record<string, unknown> }`, discarding the startup-config schema at the exact point the test rewrites an API response — so a future config shape change would go unchecked here. Reuse `TStartupConfig` instead, and only rewrite `modelSpecs` when the response actually carries it rather than fabricating it. |
||
|
|
7b6900d556
|
🏷️ feat: Activity Groups With Fast-Model Headers (#14391)
* ✨ feat: Activity Groups with Fast-Model Labels Groups each contiguous block of reasoning + tool calls into a collapsible unit headed by a fast-model label (claude.ai-style hierarchy), off the critical path: a PostToolBatch hook claims a live content slot at the batch boundary (steering index-offset pattern), renders a deterministic counts phrase instantly, and swaps in the generated label ~1s later while the next model call streams. Labels are UI-only — stripped before the SDK formatter and skipped in the legacy formatter — and reach live clients via a dedicated on_activity_label SSE event (live/replay/pending paths). Grouping preserves legacy rendering byte-for-byte when no label part is present. Generation bridges to Run.generateActivityLabel() when the SDK ships it (session-grouped Langfuse tracing); falls back to a direct call today. Env-gated: ACTIVITY_LABELS_POC=true, ACTIVITY_LABEL_MODEL. * 🧷 fix: Address Codex and Copilot Review Findings for Activity Labels - Settle in-flight label fills (bounded 3s) before finalization on both the main and resume paths, so a label resolving during the final batch still reaches the durable log and saved message. - Overlay on_activity_label chunks in RedisJobStore content reconstruction (splice path last-wins per index; replay path chronological overwrite), matching steer handling. - Wire activity labels into the HITL resume createRun so post-resume batches keep claiming slots. - Guard against out-of-order publishes: fill() awaits the claim emit before emitting the resolved label, and the client applier ignores a stale pending placeholder once a resolved label is present. - Stamp the batch's groupId onto label parts so parallel-column runs place them inside their group instead of filtering them out. - Localize the counts fallback phrase (10 keys, singular/plural) through useLocalize across chat rendering and exports. - Type the hook with Providers/ClientOptions instead of stringly types; drop the unknown cast in the spec; add a dedicated rAF retry ref for label events with effect cleanup. * 🛡️ fix: Address Independent Review — Abort, Usage, Lane Context, Redis Test - Propagate the run abort signal into label generation (both wiring call sites; runtime combines host + dispatch signals with the timeout) so a user abort cancels in-flight label calls instead of paying to timeout. - Record label-call usage like titles: the SDK bridge aggregates via chainOptions callbacks, the fallback path via a per-generation callback factory; both feed recordCollectedUsage under context 'activity-label'. - Scope block-context capture: reasoning collection stops at the previous block's label part and filters by executingAgentId, so consecutive or parallel batches can no longer bleed another block's thinking into the payload; intent text still scans past labels (persists across batches). - Forward the effective charLimit to the SDK call so host and SDK prompts agree (SDK default aligned to 600 in agents#327). - Add a Redis integration test proving last-write-wins reconstruction of on_activity_label chunks per claimed index. - Rebased onto main: only the two activity commits replay (the nine steering commits belonged to the old base branch), zero conflicts, steering suites green. * 📐 refactor: Move Activity-Label Wiring to TypeScript, Address Codex Round 2 - [P1] Slot claiming, lane stamping, emit ordering, context capture, and settle tracking now live in packages/api (createActivityLabelWiring + captureActivityBlockContext); client.js is a thin closure wrapper. - Register the activity-label hook BEFORE the steer drain so a steer draining at the same batch boundary cannot flush the tool block and orphan the label outside its group. - Resolve request-based header placeholders in resolveActivityLabelLLM (titleConvo parity) so metadata-keyed proxies work on label calls. - Trim labels centrally before filling so whitespace-only output from either generation path keeps the deterministic counts fallback. * 🧭 fix: Codex Round 3 — Capture Order, Shared Strip, Token Estimator, Hide Filter - Capture block context BEFORE pushing the label part: the scan stops at ACTIVITY_LABEL parts, so post-push capture hit the just-inserted label and silently collected no reasoning excerpts (regression test added). - Share stripActivityLabelParts from packages/api and apply it in the Responses and OpenAI-compatible controllers, closing the replay leak for entry points still running SDKs without the formatter skip. - Exclude activity_label parts from the fallback response-token estimator (UI-only parts must not inflate no-usage provider billing). - Keep label parts explicitly under hide_sequential_outputs — they summarize exactly the outputs that mode hides. * 🔁 fix: Codex Round 4 — Resume Gap, Delta Flush, Agent-Scoped Intent, Token Counter - Synthesize on_activity_label events for labels claimed or filled in the snapshot→subscribe window (the publish is fire-and-forget, so Redis-mode reconnects missed them). Feature-gated so the default path adds no content re-read; the client applier already ignores duplicates. - Flush queued deltas before applying a label part, matching the pending- action and steer appliers — without it the handler read a stale message cache and syncStepMessage pushed a pre-delta copy back. - Skip another agent's tail text when resolving intent, so parallel runs cannot seed a label prompt with a sibling agent's narration. - Exclude activity_label parts from countFormattedMessageTokens (the agent-path counter), not just the legacy BaseClient one. * 🏗️ refactor: Codex Round 5 — Extract Label Host Logic, Report Usage, Icon Strip - Move provider/model resolution, usage-metadata mapping, and the settle loop into packages/api (activityLabels/host.ts); client.js keeps only thin delegations, per the repo's TypeScript-implementation convention. - Fold label usage into the response rollup with an 'activity-label' tag (subagent precedent) so metadata.usage and the live cost gauge account for it; tagged, so it stays out of PRIMARY usage/context pairing. - Narrow tool metadata once in ToolCallGroup so THINK parts in a labeled block no longer render phantom generic icons in the stacked strip. - Import the activity-label helpers by deep path in GenerationJobManager: the package barrel now reaches provider-config/cache modules that import back into the stream layer, and the cycle broke suite loading. Declined: resetting steerOffsetState before HITL resume — resume builds a FRESH AgentClient via initializeClient (initialize.js:978), so the offset is already zero; the seed wrapper alone accounts for pre-pause parts. * 🚦 fix: Codex Round 6 — Stream Label Usage, Close Late Fills - Emit an on_token_usage chunk for label calls (sink push alone left the live session gauge blind); retained in pendingSubagentEmits so job cleanup cannot race the persist, tagged 'activity-label' as before. - Close the label scope when settle times out: the wiring gates fill() on isClosed and the client fires a label-scoped AbortController, so a straggling generation can neither mutate a saved response nor emit into a job whose runtime is gone. The controller also chains to the run signal, so a user abort still cancels label work. * 🩹 fix: Repair CI — Package Typecheck and Module Mocks Local runs covered the client tsconfig and jest, but never packages/api's own tsconfig, so nine type errors in the extracted host module shipped. - Type host.ts against the real contracts: ServerRequest, EndpointDbMethods, AppConfig from @librechat/data-schemas, IUser for createSafeUser, and a MaybeAzureConfig view for the azure instance-name probe and configuration. - Widen resolveConfigHeaders' llmConfig to Partial<RunLLMConfig>: it only reads the three provider header carriers, so auxiliary generations with a bare ClientOptions can resolve headers without assembling a run config. Type-only widening; every existing caller still satisfies it. - Add stripActivityLabelParts to the @librechat/api mock in the OpenAI and Responses controller specs — those mocks enumerate exports, so a new import read as undefined and threw before the assertions ran. - Use the real activity-label helpers in the ToolCallGroup spec's ~/utils mock; stubbing them out would hide the header logic under test. * ⚙️ feat: Configure Activity Labels via librechat.yaml, Drop Env Vars Replaces the ACTIVITY_LABELS_POC / ACTIVITY_LABEL_MODEL env gate with per-endpoint settings, following the title options convention rather than a top-level block — each endpoint picks its own cheap label model. - Add activity, activityModel, activityEndpoint, activityPrompt, activityMaxPerRun, and activityCharLimit to the endpoint schema, and to the endpoints.all pick list (enumerated, so 'all:' would otherwise drop them silently). - resolveActivityConfig reads them with title-style precedence: endpoints.all > named endpoint > custom endpoint config. - Model precedence is now activityModel > titleModel > the agent's model. activityEndpoint runs labels on another endpoint's credentials, with titleConvo's fallback-on-unknown-name behavior. - Thread activityPrompt/MaxPerRun/CharLimit through the wiring into the hook and the SDK bridge; they were hardcoded defaults. - The resume gap-repair gate keyed on the env var; it now keys on the snapshot actually containing label parts, so deployments without the feature still perform no extra content read. - Document the fields in librechat.example.yaml; add host.spec.ts covering precedence, custom-endpoint fallback, and opt-out. * 📝 refactor: Rename Enable Flag to activityLabel, Document Schema Inheritance - Rename the boolean from `activity` to `activityLabel`, matching the titleConvo/titleModel shape: a verb-object toggle whose prefix matches its modifiers (activityModel, activityPrompt, ...). `activity: true` alone read ambiguously — it could mean tracking or logging activity. - Document the two endpoint-schema inheritance paths, which behave oppositely and are ~900 lines apart: * `endpoints.all` omits from baseEndpointSchema, so new options are inherited automatically — nothing to maintain. * `azureEndpointSchema` enumerates via .pick(), so a new option is silently unavailable on Azure endpoints until listed there. The activity block now carries a pointer to the Azure caveat. * 🔍 fix: Address Codex Findings on the Config Rework - Pass the matched custom-endpoint config into the label gate. Custom endpoints live in the `endpoints.custom` ARRAY, so without it every custom endpoint resolved as disabled — including the example this PR added to librechat.example.yaml. - Give label usage a unique `runId:seq`. Label usage is billed but never appended to `collectedUsage`, so its length was static: every label event reused the last primary usage's pair and collided with itself, and the client dedupes on exactly that. - Attach `cost` to label usage when `interface.contextCost` is on; aggregateEmittedUsage treats coverage as all-or-nothing, so an event without it suppressed the whole response's cost. - Honor `activityPrompt` on the direct fallback path, not just the SDK bridge — it previously always used the built-in instruction. - Seed the per-response label cap from labels already on the response so a HITL resume cannot mint a fresh quota after every approval. - Reconcile label gaps on resume via a durable per-job `activityLabels` flag instead of probing the snapshot: the FIRST label of a run can be claimed inside the snapshot->subscribe window, which the old signal missed. The flag is read from a job record already fetched there, so runs without the feature still add no content read. - Auto-collapse labeled single-tool groups; one-call batches are common in agent runs and rendering them expanded defeats the grouping. * 🎯 fix: Correct Label Usage Seq, Cross-Endpoint Pricing, Close Scopes - Give label usage a NEGATIVE seq namespace. The previous fix was wrong: seq is a position in `collectedUsage` (push, then emit with the new length), so sink-length + array-length still lands on a real position — primary emits 1, the label computes 2, the next primary also emits 2. Labels have no position at all (billed separately, never appended), so they now occupy a namespace positional sequences cannot reach. The client key is a string used for Set membership, so the sign is inert. - Price cross-endpoint labels with the LABEL endpoint's token config: resolveActivityLabelModel now returns the resolved endpointTokenConfig, and both the streamed cost and recordCollectedUsage use it instead of the agent endpoint's rates. - Make close state per-wiring rather than per-client. A HITL resume rebuilds the wiring, and resetting a shared flag re-opened closures from the pre-pause segment whose provider call ignored the abort; settle now closes every retained scope, past generations included. * 🎯 fix: Make the Activity Header Say Something the Cards Cannot The header read "ran 1 command" next to a card already labeled "Code" — it restated the UI beneath it instead of adding to it. Two causes, both about content rather than timing: - A deterministic tool-type tally was the primary display and also fed the prompt, so the best case was a tally and the worst case was a tally dressed as prose. Removed from the metadata, the prompt, the part type, and the client. - The instruction only ever reached the fallback path. The wiring passed a prompt only when was configured, so the preferred SDK path silently used the published package default. The wiring now always supplies one and the hook forwards it on both paths. The register is rewritten around what the cards cannot show: past-tense git-commit-subject, leading with the distinctive noun, outcome over attempt, tool names and counts and arguments explicitly forbidden. The batch entries are labeled as reference material so the model stops transcribing them. Claiming a slot no longer emits. The slot still reserves its index so streamed parts never collide, but with nothing to say there is nothing to render: until a description exists the block looks exactly as it does without the feature. * 🧹 fix: Drop the Localize Hook Left Unused by the Counts Removal * ✅ test: Add Activity-Label e2e Coverage with a Recording Label Server Activity labels are the one model call a mock run does not already fake: fake-model.js swaps the GRAPH model via overrideTestModel, while run.generateActivityLabel() calls the endpoint resolved client options over HTTP. The custom endpoints already point baseURL at 127.0.0.1:8889, so serving that port exercises the real path with no production seam. fake-label-server.js answers it in both JSON and SSE form, records each prompt, and can inject blank/error responses. Recording is what lets the spec assert the CONTRACT rather than the rendering: that this repo register and the tool OUTPUTS actually reach the model. That is the bug class that produced unusable labels before, and rendered text looks identical whether or not the instruction arrived. Labels get a dedicated endpoint (Mock Provider E). A labeled block auto-collapses even at one tool call, which hides the tool cards other specs assert on -- enabling this on a shared endpoint broke steering.spec.ts. Provider D is the unlabeled control. Request-count assertions are scoped to a per-test token: a 5xx label response is retried by the provider client, and a retry can land after the next test has reset the server. * 🩹 fix: Address Review Findings on Activity-Label Indexing and Pricing Replay index (P1). Reserving the slot only in server memory left no event for it, so a cross-instance replay rebuilt content as [tool, hole, later], compacted the hole away, and the fill for the reserved index landed on the following part and overwrote it. The claim now publishes the empty, pending part so the index is real for every consumer, and fill publishes even when generation returned nothing so the client cannot stay pending. It stays invisible: an empty label still DELIMITS its batch in groupSequentialToolCalls but is not attached as the header, so grouping does not re-shuffle when the text lands and the block renders exactly as it does with the feature off. Edited-response index (P1). Edit-and-resubmit replays the kept prefix and the server indexes only new content, so run steps offset by that prefix. Labels are claimed in the same space and now take the identical shift; without it a label could land inside the prefix and overwrite it. Redis flag. deserializeJob never read activityLabels back, so every Redis reload left it undefined and resume skipped label gap reconciliation. Executing agent. RunActivityLabelOptions.agentId selects the executing agent tracing metadata AND its tool-output redaction policy; omitting it let a handoff be redacted under the default agent configuration. Label pricing. An undefined endpointTokenConfig is meaningful for a built-in label endpoint (priced from the shared table), so the nullish fallback billed those labels at a custom primary rates. Inherit only when the label runs on the agent own endpoint. HITL usage sequence. runId is the response message id and the counter was instance-local, so a resume restarted at -1 and the client runId:seq deduper discarded the post-approval label usage. Seeded past the labels already on the response. Also distinguishes "cannot serve" (undefined) from "no label" (null) in the SDK bridge, so a missing run falls back to the direct call instead of filling the slot empty. Version gating already happens at wiring time via the sdkCapable prototype probe. * 🩹 fix: Keep Unfilled Activity Labels Invisible and Unmask Endpoint Settings Follow-up review round. Publishing the reservation on every batch made two latent rendering paths reachable on every run, and both are fixed here. Empty labels no longer change grouping. The previous pass still formed a tool-group for a textless label, which wrapped even a single tool call and pulled THINK parts inside it — and since a reservation is published the moment each batch ends, that applied during every generation and permanently after a blank or failed fill. An empty label now flushes the legacy way instead: it still delimits its batch, but the block re-splits exactly as it renders with the feature off. Parallel lanes no longer show a blank line. Lanes render raw parts, so an unfilled label had nothing to draw; empty ones are dropped. Making labels act as collapsible headers inside lanes is still a separate gap. Edited responses no longer offset on resume. The sync replaces initialResponse.content with the server's aggregatedContent, which already contains the kept prefix AND everything generated since — so its length is not the prefix length, and indices reconciled from that snapshot are already absolute. Offsetting again pushed the label past its slot onto a later part. The shift now applies only to a fresh edited submission. Activity settings resolve field by field. Selecting one config object whole meant any endpoints.all block — even one carrying nothing but headers — shadowed the named or custom endpoint and silently disabled activity labels everywhere. Global still wins per field. Adds groupToolCalls coverage for the invisible-while-empty contract, which is the part most likely to regress: it is normal state on every run, not an edge case. * 🔒 fix: Scope Detached Label Writes to Their Generation Epoch Epoch scoping (P1). Label generation is detached and can outlive the generation that started it. emitChunk only proves that SOME runtime is current, not that the caller belongs to it, so an aborted generation's fill(null) -- and its usage event -- could be attributed to whichever generation replaced it, landing an index from the abandoned response on top of the new one. Because an empty label renders nothing, that overwrote content silently. emitChunk now takes an optional jobCreatedAt and drops the event when the runtime epoch differs, mirroring the existing setGraph/setContentParts convention, and both label emitters pass it. An abort now CLOSES the label scope instead of only cancelling the call: the rejected generation still runs its catch and calls fill(null), which would otherwise emit into a stream the next generation may already own. Edited-response indexing (P1). The previous pass skipped the prefix offset on resume, which was the wrong half of the problem: a sync replaces initialResponse.content with the server's aggregatedContent, which is completion-local, so after a reconnect its length is not the kept-prefix length and the offset is wrong -- but it is wrong for run steps in exactly the same way. Tool cards and the label that heads them must share one index space; a label shifting differently from its tools lands on another part. The label path now uses the identical expression as useStepHandler, with no resume special-case. Correcting the post-resume prefix length belongs in calculateContentIndex, where it fixes both at once. titleModel masking. The activity settings were made per-field last pass, but the titleModel fallback a few lines below still selected an entire config object, so a partial endpoints.all (for example one carrying only headers) hid a named endpoint's titleModel and quietly fell the label back to the main agent model. Both now read through one shared per-field helper. Resume reconciliation no longer depends solely on markActivityLabels, which is best-effort yet had come to gate correctness: a lost flag write silently dropped a label. The snapshot is consulted as a fallback. The exported host type for generateLabel now admits undefined, which is the documented "cannot serve, fall back to the direct call" signal the hook keys on -- distinct from null, meaning it ran and produced nothing. * 🧷 fix: Keep Group Identity Stable and Memoize Label Endpoint Resolution Group remount. Tool-group identity was keyed on the first part in the block. An activity label absorbs the block's leading THINK part the moment its text lands, so the key flipped from tool:<id> to fallback:<scope>:<idx> mid-run, remounting the group and discarding whatever the user had expanded. The key now scans for the first tool call, which does not move when the block re-forms. Label endpoint resolution is memoized per response. It reads provider config and can hit the database for user keys, yet nothing it depends on changes between batches of one run — and it ran twice per batch, once for generation and once for usage accounting. The promise is cached rather than the value so concurrent batches share a single in-flight resolution, and a rejection is evicted so one transient credential failure cannot disable labels for the rest of the response. * 🎯 fix: Offset Edited Resubmissions by a Prefix Length That Survives Resume The server indexes only NEW content for an edited resubmission, so the client offsets incoming indices by the prefix it retained. That prefix was read as initialResponse.content.length, which is correct only until a resume: the sync replaces that array with the server's completion-local snapshot, whose length is unrelated to the prefix. After a reconnect every offset was therefore wrong -- run steps and activity labels alike -- and could write over content the edit kept. For a label the symptom is worse than a bad position: the fill misses its own reservation, so the pending placeholder is never resolved. The prefix length is now captured when the submission is built, while initialResponse.content still IS the retained prefix, and carried on the submission as editPrefixLength. calculateContentIndex takes that length instead of deriving it from an array that a resume may have replaced, so run steps and labels share one index space by construction rather than by both happening to read the same field. Note the prefix is the FULL original content with the edited part substituted in place (useChatFunctions clones latestMessage.content and mutates one entry) -- it is not a slice, so the length cannot be inferred from editedContent.index. Group identity no longer changes when a label fills. Tool-group keys were derived from the first part in the block; an activity label absorbs the leading THINK part when its text lands, flipping the key mid-run and remounting the group, which discarded the user's expansion state. The key now scans for the first tool call, which does not move. Label endpoint resolution is memoized per response. It reads provider config and can hit the database for user keys, yet ran twice per batch -- once to generate, once for usage accounting -- while nothing it depends on changes within a run. The promise is cached so concurrent batches share one in-flight resolution, and rejections are evicted so a transient credential failure cannot disable labels for the rest of the response. The resume gap passes for steers and activity labels now share a single lazy content read instead of each issuing its own. The label pass stays gated on the run flag with a snapshot fallback: reconciling unconditionally would also close the residual first-label window, but it would bill a read to every resume of every run, including deployments with the feature off -- which the steer pass deliberately avoids. That residual requires a lost flag write, which shares fate with the content writes the labels live in. * 💵 fix: Bill Cross-Endpoint Labels at Their Own Rates recordCollectedUsage never accepted an endpointTokenConfig, so the value the activity-label caller passed was dropped and the balance transaction was written at the primary agent's rates. Only the UI cost honored the label endpoint, so a custom primary pointing activityEndpoint at another endpoint showed one price and charged another. The parameter is now accepted, and an explicit config wins outright over per-agent resolution: that map is keyed by AGENT, so it cannot describe usage that ran on a different endpoint. Group identity is stable for id-less tool calls too. The previous pass anchored the key to the first tool call ID; where a supported tool call carries no id the fallback still used the block's first part index, which shifts when a filled label absorbs the leading THINK part. The fallback now anchors to the first TOOL entry's index, so only a block containing no tool call at all keys off parts[0]. markActivityLabels is retried rather than fire-and-forget. It gates resume gap reconciliation and is a SEPARATE write from the durable label append, so a single lost write silently drops a label the content itself recorded. The earlier "shared fate with content writes" reasoning was wrong. One retry at run setup costs nothing and removes the only realistic way the gate goes stale, without billing a content read to every resume. * 🧮 fix: Stop Offsetting Once SYNC Drops the Edited Prefix The edit offset was applied unconditionally, but whether it is correct depends on which branch SYNC took. SYNC either preserves the content already loaded for the response -- which still contains the retained prefix, so the offset is required -- or replaces it with the server's aggregatedContent, which is completion-local and indexed from zero, after which any offset writes past the end of a now shorter array. That is why the two previous attempts each fixed half of it: skipping the offset on resume was right for the replace branch, applying it unconditionally was right for the preserve branch, and neither holds on its own. The offset now tracks the actual state of the rendered content. For an activity label the replace branch was worse than a bad position: the fill landed past its own reservation, so the pending placeholder was never resolved and the block kept its generic header for the rest of the run. Applied to run steps as well, not just labels. useStepHandler reads the prefix from the same submission and had the same unconditional offset, so after a mid-session resume of an edited response tool cards were misplaced too. Normalizing at the dispatch boundary keeps both in ONE index space by construction: a label that shifted differently from the tools it heads would land on another part. Note the reload path was already coherent -- useResumeOnLoad rebuilds the submission without editedContent or editPrefixLength, giving no offset against server-supplied content -- so only the mid-session SYNC path was inconsistent. * 🧾 fix: Keep Label Accounting Out of the Primary Usage Slot Label usage no longer owns getStreamUsage(). recordCollectedUsage assigned its result to this.usage unconditionally, so when the primary provider reported no usage metadata but the label provider did, BaseClient took the label's output tokens as the assistant response's authoritative count. The later primary call returns early on an empty collectedUsage and never replaced it, so the wrong value stood, the text-based fallback was skipped, and the real generation went unbilled. Secondary usage is still billed but no longer writes that slot. Cross-endpoint pricing keys off an explicit discriminator rather than the presence of a value. A built-in label endpoint prices from the shared table, so an undefined endpointTokenConfig is its MEANINGFUL value -- reading that as "no override" fell back to the primary's custom rates and restored the exact mismatch the previous pass set out to fix. The caller already knows whether the label ran elsewhere and now says so. markActivityLabels rejects on failure instead of swallowing it. The flag gates resume gap reconciliation and the caller retries it, but the internal catch resolved successfully and made that retry unreachable -- so the two changes cancelled out and a transient write failure still left the flag absent. Late label accounting is suppressed with the same gate as the late fill. A straggler that outlived the settle timeout still ran its finally block, so it charged the balance and appended to usageEmitSink after the response had passed its usage flush and metadata snapshot: a cost the user pays but is never shown. The cleared-prefix state is scoped to one generation. It was set on a resume SYNC that replaced the response and then never reset, so a later edited resubmission in the same mounted hook dispatched run steps and labels with no offset against content that still held its retained prefix. Reconnects pass isResume and keep the state; a new generation clears it. * 🔑 fix: Key Prefix State to the Stream and Honor current_model for Labels The cleared-prefix reset keyed on isResume, which skips exactly the case it was added for: a submission whose POST succeeded server-side but lost its response is retried, comes back resumed: true, and subscribes in resume mode even though it is a NEW generation. A previous generation's cleared state then survived into it, and incoming run steps and labels applied no offset against content that still held its retained prefix. The state is now keyed to the stream id, which changes with the generation and stays put across reconnects of one. activityModel now honors current_model. The options are documented as title-shaped and the titleModel fallback already excludes the sentinel, but the higher-precedence activity override passed the literal string through to getOptions and the provider, so an endpoint following that convention failed every label instead of using the agent model. * 🎯 fix: Key Prefix State to the Generation and Resolve the Run Model The cleared-prefix state was keyed to the stream id, which never changes within a conversation: request.js sets streamId = conversationId, so once a reconnect cleared the state every later edited resubmission in that conversation dispatched run steps and labels with no offset and could overwrite the prefix it retained. It is now keyed to the response message id, the only per-generation identity available here -- minted per submission and carried through a resume unchanged. That is the third identity tried for this state. isResume missed the deduplicated-retry path (a lost response returns resumed: true for a new generation); the stream id is conversation-scoped. The response id is the boundary that actually matches a generation. current_model labels now resolve the model the run is really using. initializeAgent merges the request's endpointOption override into model_parameters and the run gives it precedence, so preferring the saved agent.model could send labels to a different, potentially unavailable or more expensive model than the conversation is on. * 🆔 fix: Key Prefix State to the Submission and Keep the Origin Title Model Editing an assistant response reuses that response's messageId as editedMessageId, and useChatFunctions carries it onto initialResponse.messageId -- so re-editing the same response produced two generations with the same key and the cleared-prefix state survived between them, leaving run steps and labels with no offset against content the edit retained. Keyed now to clientRequestId, the per-submission uuid, which is minted fresh per edit attempt and forwarded unchanged on retries. That is the fourth key this state has had, and each earlier one failed at a real boundary: isResume missed the deduplicated-retry path, the stream id is the conversation id, and the response message id is reused across edits of one response. clientRequestId is the identity that actually means "this submission". The titleModel fallback is read from the ORIGINATING endpoint again, matching how titleConvo captures its config before switching credentials. Reading it after an activityEndpoint switch meant an OpenAI endpoint configured with titleModel claude-haiku and activityEndpoint anthropic fell through to the OpenAI run model and sent that name to Anthropic, failing every label. The destination endpoint supplies credentials, not the model choice. * 🧷 fix: Close the Remaining Edit, Epoch, and Scope Gaps for Labels SYNC clears the edit prefix on the new-row branch too. When a resumed edited submission cannot match an existing assistant row, that branch builds the response straight from the server's completion-local aggregatedContent, so it holds no retained prefix -- but the reset lived only in the matched branch, leaving later steps and labels adding an offset to indices that were already absolute. Label usage is keyed per GENERATION. Editing one assistant response reuses its responseMessageId while each fresh generation restarts activityLabelUsageSeq, so a second edit re-emitted the same runId:seq and the client discarded the newer usage while its balance transaction was still written. The key now carries jobCreatedAt, the run's own epoch: stable across reconnects and HITL resumes, distinct between generations. The scope is revalidated at commit time. Checking once before the await let a scope that closed mid-flight still charge the balance after finalization, while the matching fill saw the closed scope and dropped the label -- billed but never surfaced, the exact outcome the guard exists to prevent. The titleModel fallback no longer reaches the destination endpoint. With activityEndpoint set and no titleModel on the originating endpoint, it picked up the destination's, so changing only the credential target silently changed the model and its cost. Precedence is activityModel, then the originating endpoint's titleModel, then the run model; the destination supplies credentials only. * ✂️ refactor: Confine the Edit-Prefix Offset to Activity Labels useStepHandler is now byte-identical to dev again. The resume-aware prefix offset was applied there too, which was more correct in principle -- the post-resume prefix length is genuinely wrong for run steps as well -- but it changed index math that EVERY run step flows through, for every user, including everyone who never enables activityLabel. That shared correction needed five revisions in two days (isResume, the stream id, the response message id, clientRequestId, and the SYNC new-row branch), each passing the full suite and each failing at a boundary only review found. Carrying it inside an opt-in feature put every user behind logic with that track record. It belongs in its own change, with tests that construct the edit-plus-resume states none of the current suites reach. The offset now applies only where the label handler places its part, so this PR cannot alter rendering for anyone with the feature off. The known consequence is recorded in the description: with activity labels ENABLED, an edited response that reconnects mid-generation can place its label and its tool cards in different index spaces. That is a bug for opt-in users rather than a regression for everyone, and it disappears once the shared fix lands. submission.editPrefixLength stays: the label path still needs a prefix length that survives a SYNC replacing initialResponse.content. * 🧾 fix: Commit Labels Before Billing and Keep Blank Slots Invisible Round-nine review (all P2, feature-scoped): - Billing ordering (client.js:409, runtime.ts): usage accounting ran BEFORE the slot commit on both generation paths, so the settlement deadline could expire during the balance write — charged, then the fill dropped as out-of-scope: billed, never shown. `slot.fill` now resolves a commit flag, generators register their accounting via `deferUsage`, and the hook runs it only after a committed fill. - Scope gates (client.js:757): the direct-fallback `collect` omitted `scopeOpen`; both paths now gate on the OWNING wiring's scope, so a pre-pause straggler cannot bill because the resumed generation's scope is still open. - Blank-label grouping (groupToolCalls.ts:81): a blank slot forced a flush, splitting adjacent single-call batches into standalone cards where the feature-off path merges them. Blank labels now only mark the claim boundary — structurally invisible, while a later filled label still cannot claim an earlier batch. - Stale fill indices (wiring.ts:301): the skill-card unshift and the hide-sequential filter reshape contentParts before the finalization settle, so an in-flight fill emitted its claim-time index against a shifted array. Both completion paths now settle label fills before any post-run content reshaping (the finally settle stays as the error-path net; the second call sees an empty pending list). - Bounded serialization (runtime.ts:238): `JSON.stringify` fully materialized unbounded tool results to keep 200/600 chars per entry. A budget-bounded serializer stops at the limit (which also bounds cyclic values) and preserves the exact truncate-with-ellipsis output. Tests: fill/bill ordering + suppression on dropped fills (runtime.spec), blank-slot merging and claim boundaries (groupToolCalls.test), bounded serialization equivalence and giant-output truncation (runtime.spec). * 🧮 fix: Keep Deferred Label Billing Inside the Settle Window Self-review follow-up to the billing reorder: deferring usage until after the commit moved it PAST the fill's resolution, so a settle keyed on fills alone could let finalization flush the usage sink and snapshot metadata while the label's billing was still in flight — the usage row would silently miss the message rollup even on the happy path. The hook now reports its whole detached task (generate → fill → deferred usage) via a `trackTask` option, wired to the same settle tracker as the fills, so finalization waits for billing exactly as it did when accounting preceded the fill. The task never rejects. Pinned in runtime.spec: the tracked task resolves only after usage collection. * 🧰 fix: Harden Label Resolution, Output Bounds, and Cache Billing Round-ten review (all P2, feature-scoped); the sixth finding is the documented edited+reconnect index-space limitation, answered on-thread as deliberately out of scope for this PR. - Rejected-LLM memoization (runtime.ts): the hook cached a rejected `resolveLLM()` promise permanently, failing every later batch and silently defeating the host resolver's own rejected-cache eviction. The memo now evicts on rejection so the next batch retries. - `current_model` precedence (host.ts): an explicit `activityModel: current_model` resolved to `undefined` and then lost to a configured `titleModel`. The sentinel now resolves straight to the run model; the title fallback applies only when `activityModel` is absent. - Output bounds (runtime.ts): label text was persisted verbatim; a model ignoring the 4–9-word instruction (or steered by injection in untrusted tool output) could emit thousands of tokens duplicated through SSE, the chunk log, persistence, and the UI. `normalizeLabelOutput` keeps the first non-empty line, collapses whitespace, and hard-caps at 200 chars on both generation paths. - Cache-token billing (host.ts, client.js): the usage mapper dropped cache fields, vanishing Anthropic cache tokens from billing and charging OpenAI cache reads at the full input rate. The mapper now normalizes Anthropic/OpenAI/LangChain cache shapes into `input_token_details`, and the emit + cost path carries them with the label endpoint's `provider` (additive-provider adjustment). - Usage-type union (runs.ts): `TTokenUsageEvent.usage_type` now includes the emitted `activity-label` literal; the lone consumer keys on `usage_type != null`, so this is type-level completion. Tests: sentinel/title/explicit model precedence and all three cache shapes (host.spec), transient-resolution retry and output normalization with truncation (runtime.spec), the new usage literal (runs.spec). * 🪗 fix: Let Settled Labels Collapse Void Tools and Keep the Tail Cursor Round-eleven review (all P2, client-side). Two fixed; the other two findings restate documented Known limitations (edited+reconnect run-step index space; parallel-lane collapsible headers), answered on-thread. - Void-tool auto-collapse (ToolCallGroup.tsx): `allCompleted` keyed solely on output truthiness, so a tool that legitimately returns an empty string kept its labeled group expanded forever. A settled, filled label is itself a completion proof — the PostToolBatch claim only happens after every output in the batch returned — so it now satisfies `allCompleted`; pending labels keep the group live. - Trailing-reservation cursor (ContentParts.tsx): a blank label reservation at the content tail renders nothing but still counted as the last part, stripping the streaming cursor and last-item affordances from the last VISIBLE part until the next delta. `lastContentIdx` now walks back past empty label slots. Tests: labeled void-tool group auto-collapses, pending-label group stays expanded (ToolCallGroup.test). * 💳 fix: Price Label Cache Correctly, Honor endpoints.agents, Cancel Every Retry Round-twelve review: four fixed here; the remaining P1 (move the client.js bridge into packages/api) is an architecture call answered on-thread for the maintainer. - Provider on billed entries (client.js, P1): round ten added cache details to label usage entries but not `provider`, and `splitUsage` treats an unknown provider as additive — re-adding cache_read and cache_creation on top of an input count that already contains them, double-charging Anthropic/OpenAI cached label calls while the streamed cost (which carried the provider) disagreed. Every mapped entry now carries the label endpoint's provider. - endpoints.agents honored (host.ts, client.js): `initializeAgent` rewrites `agent.endpoint` to the backing provider, so activity settings under the PUBLIC `agents` endpoint — valid config, inherited by `agentsEndpointSchema` — were silently ignored. Field resolution is now `all` > public endpoint > backing provider/custom, applied to both the enable gate and the model/titleModel resolution. - E2E_LABEL_PORT reaches the YAML (playwright.config.mock.ts): an overridden port moved the fake label server and its health check but not the generated config's hard-coded 8889 baseURLs, so readiness passed while every label request targeted the wrong port. The override is now substituted into the generated copy. - Every retry frame cancelled (useResumableSSE.ts): concurrent label retry chains (reservation + fill per slot) overwrote one rAF handle, so cleanup cancelled only the newest chain; the rest ran up to 120 frames past unmount and could apply a stale label to a replacement generation reusing the same response id. Outstanding frame ids now live in a Set that cleanup drains. Tests: public-endpoint gate/precedence/all-above-public (host.spec). * 🖱️ fix: Keep the Last-Part Cursor in Parallel Lanes Too Round-thirteen review (single P2): `ParallelContentRenderer` computed `lastContentIdx` from the unfiltered array, so a trailing blank label reservation — filtered out of every lane — left NO rendered part carrying the last-part cursor and running-subagent affordances until the label filled. The sequential renderer's walk-back is extracted into a shared `lastVisibleContentIdx` helper (utils/activityLabels) used by both `ContentParts` and `ParallelContentRenderer`, so the two index spaces cannot drift again. Behavior pinned in activityLabels.spec: trailing blank skipped, consecutive blanks skipped, filled label counts, label-free content unchanged. * 🧹 chore: Alias the Retry-Frame Set for the Effect Cleanup Lint Rule * 📏 fix: Let activityCharLimit Reach Tool Inputs Round-fifteen review: `activityCharLimit` is documented as the per-entry truncation for tool input AND output, but `buildPrompt` hard-coded inputs at 200 characters — so raising the setting could never surface a distinguishing path, query, or operation that appears past the first 200 characters of a long argument. Inputs now truncate at the configured limit alongside outputs; the 200-char constant remains only for the intent line (renamed INTENT_CHAR_LIMIT to match). Config fidelity pinned in runtime.spec: a 400-char argument survives a 450 limit and truncates under a 50 limit. The round's other finding is the fifth restatement of the documented edited+reconnect index-space limitation, answered on-thread with the prior four cross-references. * 🤝 fix: No Labels for Pure Handoff Batches Round-sixteen review: a PostToolBatch containing only `transfer_to_*` calls claimed a label slot, but transfer parts are never groupable — the client flushed the handoff card standalone and the label orphaned into a stray line after it, restating what the card already says. Two-sided fix: - Hook (runtime.ts): a batch whose every entry is a transfer call claims nothing — no slot, no model call, no `maxPerRun` consumption. Mixed batches still label (the header describes the real work). - Renderer (groupToolCalls.ts): an orphan label whose `tool_call_ids` are all transfer calls is dropped instead of rendered standalone, covering content persisted before the hook-side skip. The round's two P1s are repeats answered on-thread: the packages/api extraction (maintainer-decided follow-up, recorded in the description) and the sixth restatement of the edited+reconnect index limitation. Tests: transfer-only batch claims nothing, mixed batch still claims (runtime.spec); transfer-only orphan label dropped, real-batch orphan label still renders (groupToolCalls.test). * 🎛️ fix: Sanitize Label Client Options and Bound the Batch Prompt Round-seventeen review: two fixed; the other two findings repeat the maintainer-decided packages/api extraction (follow-up) and the edited+reconnect index limitation (seventh instance), answered on-thread. - Primary-option strip (host.ts): the label client copied the resolved `llmConfig` wholesale, so an endpoint whose defaults enable extended thinking or carry model-specific output caps forwarded them to the (often cheaper) label model — unsupported options failed every label, and supported thinking spent real tokens and the settlement window on a 4–9 word header. The copy now strips `omitTitleOptions` keys and the `modelKwargs` output caps exactly like the title path, restoring the Anthropic `clientOptions` carrier by reference so proxy `defaultHeaders` still reach label requests. - Batch prompt budget (runtime.ts): per-entry truncation left the batch dimension unbounded — hundreds of parallel calls could build a prompt past the fast model's window. The entries section now has a total budget (8k chars, scaling with `activityCharLimit` so a raised limit still fits several entries); entries past it are skipped without paying their serialization cost, and the list notes how many were omitted. The first entry always renders in full. Tests: option strip with header-carrier survival (host.spec); giant batch bounded with omission marker, small batch untouched (runtime.spec). * 🛡️ fix: Keep SSRF Guards on Label Calls, Skip Mixed Handoff Batches Round-eighteen review: four fixed; the fifth repeats the maintainer-decided packages/api extraction (eighth instance), answered on-thread. - SSRF-safe carrier (host.ts, P1): the sanitize step restored the Anthropic `clientOptions` carrier only when `defaultHeaders` existed, but for user-provided base URLs `getLLMConfig` stores the guarded Undici dispatcher and `redirect: 'error'` there — dropping it reopened DNS-rebinding/redirect paths on label calls to user-controlled URLs. The carrier (client CONSTRUCTION options, not generation params) is now restored whenever present, same reference. - Primary maxTokens (host.ts): top-level `maxTokens` is not in `omitTitleOptions` and survived the strip; the title path deletes it explicitly, and a cap sized for the primary model can be rejected by the substitute. Deleted on the copy. - Bounded keys (runtime.ts): the object branch materialized every key via `Object.keys` and quoted oversized keys in full before the budget check. Enumeration is now lazy (`for..in` + own-property guard) and keys slice to the budget before quoting, like string values. - Mixed handoff batches (runtime.ts, groupToolCalls.ts): the client flushes the block at the transfer card, so a mixed batch's label orphaned exactly like a pure one. The hook now skips ANY batch containing a transfer call, and the renderer drops orphan labels covering one (legacy content). Tests: carrier survival without headers by same reference, maxTokens strip (host.spec); mixed batch claims nothing (runtime.spec); mixed orphan dropped, real-batch orphan kept (groupToolCalls.test). * 🧢 fix: Cap Label Generation, Order the Flag Persist, Detach Settled Listeners Round-nineteen review: three fixed; the fourth is the ninth instance of the edited+reconnect index limitation, answered on-thread. - Generation cap (host.ts): stripping the primary output caps left label calls with NO cap at all — `normalizeLabelOutput` bounds what persists, not what the provider generates and bills, so a model ignoring the 4–9-word instruction (or steered by injected tool output) could emit its provider-default output per batch. The sanitize step now installs a 256-token label cap (per provider family: `maxOutputTokens` for Google-style wrappers, `maxTokens` otherwise), after the filter so the omit set cannot remove it. - Flag-persist ordering (client.js): the `markActivityLabels` write was fire-and-forget, so an immediate cross-replica reconnect could read the job between the write and the first claim, see neither flag nor snapshot label, and skip gap reconciliation. Label emission now awaits the (settled-on-failure) persist chain, making "a label event exists" imply "the flag is durable" — the race window is gone; only the documented double-write-failure residual remains. - Listener detach (client.js): each HITL approval cycle's wiring adds a `once` abort listener to the shared job signal that only an actual abort removes; settled segments now detach theirs in `settleActivityLabels`, so long multi-approval runs cannot accumulate dead closures toward the listener-limit warning. Tests: the primary cap is REPLACED by the 256-token label cap (host.spec). * 🎯 fix: Route the Label Cap Per Model Family Round-twenty review: the 256-token label cap set maxTokens unconditionally, but GPT-5+ rejects max_tokens (the OpenAI builder routes its cap into modelKwargs.max_completion_tokens / max_output_tokens) and o-series models reject it with no stable kwargs alternative — every label on those models would have failed. The cap now mirrors the builder: modelKwargs for GPT-5+ (responses-API aware), no cap for o-series (title parity; the 200-char persistence bound still applies), maxOutputTokens for Google, maxTokens otherwise. Pinned in host.spec for both reasoning families. The round's other finding is the tenth instance of the documented edited+reconnect index limitation, answered on-thread. * ⏱️ fix: Persist the Label Flag at Run Start, Not on the Emit Path Round-twenty-one review: two fixed; the other three repeat the maintainer-decided packages/api extraction, the edited+reconnect index limitation, and the parallel-lane header limitation — all answered on-thread with their standing decisions. - Flag ordering, corrected (client.js): sequencing label emission behind the flag persist (previous round) delayed the claim-time reservation while the shared index offset had ALREADY shifted subsequent SDK chunks — reopening the cross-instance hole-compaction overwrite the reservation emit exists to prevent. The reservation emits immediately again; instead, run start (processStream and resume alike) awaits the settled-on-failure persist chain, so the flag is durable before any batch can claim a label. Same guarantee, zero latency on the emit path. - Tail-label cursor (ContentParts.tsx): a filled label at the content tail is consumed into the group header rather than listed in `group.parts`, so the `isLast` check missed it and nothing held the streaming cursor until the next delta. The check now includes `labelPart.idx`. * 🔌 fix: Detach Label Abort Listeners Even Without Claims A segment with labels enabled can end without a single claim (text-only, or handoff batches, which skip labels); the early return in settleActivityLabels skipped the detach added for HITL listener accumulation. The detach now runs on both paths. * ⚖️ fix: Make the Commit Flag the Sole Billing Authority Round-twenty-three review: a committed fill racing a late scope close (user abort or settle timeout during the durable emit) stayed visible — the part is mutated and persisted before the close — yet the deferred accounting's scope gates then skipped the charge: a completed provider call escaping both the label charge and the primary abort accounting. The scope gates on the deferred-usage path are removed; the hook's commit flag is now the single billing authority in BOTH directions. A dropped fill never reaches the accounting callback (billed-never-shown stays impossible), and a committed fill bills regardless of when its scope closed (shown-never-billed now impossible too). The dead `scopeOpen` payload threading is removed with it; the `recordActivityLabelUsage` parameter survives, defaulting open, for callers that own no commit signal. The round's other finding is the twelfth instance of the documented edited+reconnect index limitation, answered on-thread. * 🧮 feat: Bill Labels by Estimate When Providers Omit Usage Maintainer decision: follow the title convention rather than leaving label calls unbilled when a provider returns no usage metadata. The hook now passes a LAZY estimate thunk with the deferred accounting on the success path — the EXACT prompt the direct path sent (or the locally built equivalent for the SDK path: same entries, context, instruction, truncation contract, and continuity headers) plus the final normalized label. `recordActivityLabelUsage` invokes it only when no collected entry carries a real token count, counts both texts with the shared o200k_base tokenizer, and feeds the synthesized entry through the SAME pipeline (provider-tagged, streamed event, cost, balance transaction). Real provider usage always wins when present. The failure path passes NO estimate: a throw before a response bills only real collected metadata, never a full phantom prompt. Tests: the estimate thunk carries the exact invoked prompt and final label; the failure path defers with no estimate (runtime.spec). * 💵 fix: Estimate From the Raw Completion, Not the Normalized Label The fallback estimate counted the normalized label (first line, 200-char cap) while the provider generated and would bill the raw output up to the 256-token generation cap — under-recording verbose replies. The estimate thunk now carries the raw pre-normalization text; the persisted label is unchanged. Pinned with a multi-line reply test. * 🧾 fix: Commit Label Text Only After the Durable Emit, Estimate the Real SDK Prompt Round review on the billing work: two fixed; the third is the fourteenth instance of the edited+reconnect index limitation, answered on-thread. - Copy-first fill (wiring.ts): the fill mutated the shared content part BEFORE its durable emit, so a failed emit left the label text on `contentParts` anyway — persistence could save and display a label no client ever received and billing (keyed on the commit flag) never charged. The new state is staged on a copy; the shared part mutates only after the emit succeeds, so content, delivery, and billing move together. - Real SDK prompt for estimates (client.js): the estimate thunk carried this module's locally built prompt, but the SDK path frames entries differently — the estimated input count was for a prompt never sent. Chain-start callbacks (handleLLMStart/handleChatModelStart) now capture the prompt the SDK actually rendered, and the deferred accounting substitutes it into the estimate when capture succeeded, falling back to the local approximation otherwise. |
||
|
|
aa357a8e17
|
🎛️ test: Guard Multi-Steer Injection Across Tool Boundaries (#14498)
* 🎛️ test: Guard Multi-Steer Injection Across Tool Boundaries * 🎛️ test: Enforce ACK Overlap and Ordered Steer Echo Assertions |
||
|
|
a53936d273
|
🧭 test: Cover Agent Handoffs End to End (#14428)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Helm Chart Tags / Ignore non-main push (push) Has been cancelled
Sync Helm Chart Tags / Sync chart tags (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled
* test: cover agent handoffs end to end * style: sort handoff imports * fix: normalize missing agent handoff edges * chore: update package dependencies and versions in package-lock.json and package.json * chore: bump agents SDK |
||
|
|
d8427ffc5e
|
🛂 test: Cover Tool Approval Workflows End to End (#14427)
* test: cover tool approval workflows end to end * fix: preserve tool approval state across resume * fix: preserve agent context in mock stream responses * fix: preserve nested approvals in collapsed groups |
||
|
|
f3159f9891
|
🧩 fix: Harden Agent Skill Lifecycles End to End (#14429)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Has been cancelled
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
* test: cover agent skill lifecycles end to end * style: sort agent skill imports |