mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-01 11:33:44 +00:00
552 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Danny Avila
|
fc2b8584c4
|
📇 feat: Surface Event Child Activity Through a Bounded Parent Index (#15142)
* feat: surface event-driven child activity * fix: keep child task aggregation documentdb-compatible * fix: address event activity review findings * test: provide markdown message context defaults * fix: report bounded child history truncation * fix: preserve current child activity state * fix: preserve durable event child activity * fix: handle missing task timestamps * fix: keep active event snapshots live * fix: preserve event activity across valid anchors * fix: close event child activity gaps * fix: preserve event activity across resume |
||
|
Danny Avila
|
77cb72e50c
|
🧮 perf: Enable Agent Context Count Reuse (#15130)
* perf: enable agent context count reuse * fix: declare token counter return type * fix: initialize cached token counters * style: sort token counter imports * fix: Keep cached token counts exact |
||
|
Danny Avila
|
dd146ff74d
|
🧾 fix: Report Complete Agents API Usage (#15127)
* fix: report complete agents api usage * fix: preserve invoked usage context * test: cover absent usage context * fix: type responses usage finalization * fix: preserve reasoning usage aliases * fix: declare reasoning usage alias |
||
|
Danny Avila
|
2018c70040
|
🧫 test: Lock Subagent File Context Propagation (#15126) | ||
|
Danny Avila
|
c2aa688d73
|
🖥️ feat: Stream PTC Inner Tool Calls as a CLI-Style Trace (#15115)
* 🖥️ feat: Stream PTC Inner Tool Calls as a CLI-Style Trace Programmatic tool calling runs a whole program inside the sandbox, and the tool calls that program makes open no run step of their own. The card showed one running spinner for the entire execution, with no sign of what the code was doing. Emit a new `on_ptc_tool_call` step event for each inner invocation — once on dispatch, once on settle — and render them under the code as a terminal-style trace: status glyph, tool identity, argument preview, duration, with a failure message printed under the call that produced it. The seam is the tool map the sandbox bridge resolves inner calls against. `instrumentPtcToolMap` proxies `invoke` on each entry and leaves every other property (name, schema, mcp) passing straight through, so nothing about execution changes and emission failures can never fail a tool call. Client state is a per-tool-call Recoil atom keyed like the sandbox-starting and subagent atoms — live for the session, cleared on conversation switch so a finished program's trace stays readable. * 🩹 fix: Address Codex Review on the PTC Tool Trace Five findings, all confirmed against the source before fixing. Scope the trace atoms to a message occurrence. The hook already documents that providers repeat a tool_call_id across turns and even within one message, and `call_id` restarts at :0 for every outer call — so two programs sharing `call_0` merged into one card. Key by (response message id, tool call id) via `ptcTraceKey`, mirroring `subagentProgressKey`; the event's `runId` already carries the message id and the card reads its own from MessageContext. Prune unsettled rows on resume. Inner calls are not content parts, so the resume snapshot cannot rebuild them, and `trackReplayEvent` only persists OAuth events — a call that settled during a disconnect left a spinner that never resolved. Settled rows are real history and stay. Make the argument preview budget-aware. Iterate keys rather than entries so the budget check can actually skip work, and clip against a bounded window so a multi-megabyte value is never collapsed in full to build a 40-character preview. Catch the resumable emission promise. The synchronous try/catch around the emitter cannot observe a rejected `emitChunk`, so a failing transport raised an unhandled rejection per event instead of dropping telemetry. Announce completion to assistive technology. The check glyph is decorative and a fast call renders no duration, so a settled row previously announced no outcome; each row now carries an sr-only status and the visible cell that duplicated it is hidden. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP * 🧹 fix: Repair CI Failures on the PTC Tool Trace Two failures on the previous head, both mine. `Tests: api (shard 2/3)` — 46 failures in `initialize.spec.js`, all `TypeError: createPtcProgressEmitter is not a function`. The suite mocks the callbacks module with an object literal, and wiring the new emitter into `initialize.js` without adding it there left the factory undefined at call time. Added it alongside `createAttachmentEmitter`, plus an assertion that it receives the same generation fence as every other resumable emitter — a stale epoch would leak one run's inner calls into the next. `Static checks` — import-order drift in `PtcToolTrace.tsx` and `handlers.ts`, repaired with `scripts/sort-imports.mts`. ESLint and Prettier both passed, so only the dedicated check caught it. `openai.js` and `responses.js` never take the emitter, so their specs were unaffected; verified the initialize mock now covers every name the module destructures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP * 🔐 fix: Address Second Codex Review on the PTC Tool Trace Three of five findings actioned; two answered on the thread. Respect tool-argument PII filtering (P1). Inner calls never reach `filteredToolArgumentsResult` — the sandbox bridge invokes them directly — so the trace was the one path putting their values on the wire in a deployment that had configured `filters.toolArguments.pii`. When any of the name / arguments / output fields are filtered, the emitter now omits both the argument preview and the failure message, which routinely quotes the argument that caused it. Name, status and duration still report. Drop the light/dark-specific background (P1). `dark:bg-transparent` stepped outside the semantic roles and would lose the intended separation under a custom theme. The pane now sets no background at all and inherits the card's surface, which resolves to the same color the override produced in both default themes and stays correct when a theme reassigns its roles. Bound the live trace (P2). A program looping over a large collection made every event copy an ever-growing array and rendered a row per call. The trace now keeps a rolling tail of 100 rows and counts what it evicted, surfaced as "+N earlier calls" so the cap is never silent. A settle whose row is gone — evicted, or pruned across a resume gap — no longer reappears out of order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP * ✅ test: Keep PTC Trace Tests Aligned With Caller-Capability Filtering Left out of the merge commit by a staging slip; without them `handlers.spec.ts` fails on the merged tree. `#15105` restricts the PTC tool map to tools whose `allowed_callers` admit code execution, so the existing trace test's registry entry — which declared none, defaulting to `direct` — was filtered out before the instrumentation could see it. Declare the fixture `code_execution`. Add a guard for the resolution itself: a `direct`-only tool must never appear in the instrumented map. Tracing wraps the eligible map, and this fails if a later change reorders that and lets the trace widen what the sandbox reaches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP * 🛡️ fix: Close Name Disclosure and Follow the PTC Trace Tail Two findings from the third Codex pass on ` |
||
|
Danny Avila
|
1de88e7e91
|
📨 feat: Continue Bound Child Agents from Events (#15112)
* feat: add authenticated agent event ingress * style: sort agent ingress imports * fix: harden agent event ingress * fix: bind event provenance to API keys * fix: inspect event input with legacy PII filters * fix: scope event status reads to source keys * fix: bind event status reads to remote sources * feat: add bound event-driven child turns * fix: harden event-bound child continuations * fix: satisfy event binding type contracts * fix: close event actor lifecycle races * fix: harden event actor dispatch continuity * fix: fence event actor resume lifecycle * fix: bind event actor state to lifecycle * fix: preserve cascade write outcomes * test: type cascade failure injection * style: sort cascade test imports * fix: harden event child lifecycle boundaries * fix: make event cleanup retryable * fix: annotate event retention clock * fix: reconcile partial cascade metadata * fix: recheck event binding expiry on resume * fix: fence event actors by retention deadline * fix: close event actor lifecycle races * fix: harden event child lease acquisition * fix: lazy-load event child lease adapter |
||
|
Danny Avila
|
8f9fae0a6e
|
🛂 fix: Preserve Legacy Assistant Attribution (#15118) | ||
|
Danny Avila
|
89494d45fd
|
🚦 fix: Restrict Programmatic Tool Execution Maps (#15105)
* fix: restrict programmatic tool execution maps * chore: bump `@librechat/agents` to v3.6.10 * fix: honor live programmatic caller projections * style: sort caller capability imports * test: expect caller projection loader argument * chore: bump agents sdk to v3.6.11 * refactor: use SDK caller projection type * style: sort agent handler imports |
||
|
Danny Avila
|
67b7b441b2
|
🛂 feat: Filter Model-Bound Content by Source (#14425)
* feat: introduce optional content protection seam * feat: enforce source-aware content filters * feat: complete source-aware content enforcement * test: activate skill file-text fail-close fixtures * fix: harden source-aware content filters * fix: harden model-bound content filtering * fix: preserve legacy filters and generated files * fix: inspect shared scalar metadata * test: align mocks with current dev dependencies * feat: add persisted content filter safeguards * feat: complete source-aware content filter enforcement * fix: move resume content preflight into TypeScript * fix: close content inspection edge cases * fix: harden content protection boundaries * fix: complete content protection safeguards * test: align persisted memory filter coverage * fix: reconcile content protection with current dev * fix: reconcile content protection with latest dev * fix: close content protection review gaps * fix: enforce source-aware provider boundaries * fix: preserve legacy PII preflight semantics * test: stabilize stored branch preflight fixture * fix: defer agent writes until protected model admission * perf: harden source-aware model-bound filtering * fix: canonicalize provider lineage before validation * fix: satisfy model-bound callback type checks * perf: Bound content protection filtering work * fix: Bound submission array traversal * fix: Stabilize bounded content snapshots * fix: Scope model-bound traversal overflows * fix: Preserve scoped content inspection * fix: Accumulate aggregate traversal scopes * fix: centralize content policy boundaries * test: align deferred tool policy context * test: align controller policy mocks * style: normalize content protection imports * fix: close content policy review gaps * fix: narrow active skill policy config * fix: address content protection review boundaries * fix: retain exact provenance overflow sentinel * fix: preserve literal and scoped provenance updates * fix: narrow persisted edit provenance * fix: isolate exact overflow attribution * fix: centralize stored prompt protection * fix: fail closed on incomplete transcript evidence * fix: align canonical transcript routing * refactor: centralize content policy preflights * fix: isolate upload policy error typing * style: sort policy preflight imports * refactor: centralize content policy boundaries |
||
|
Danny Avila
|
8ae94afa91
|
🪡 fix: Thread Parent Message ID Through MCP Request-Scoped Bodies (#15095)
* fix: Unify MCP request-scoped headers * fix: address request-scoped MCP review findings * test: preserve request scope on status errors * fix: treat authorized on-demand MCP servers as ready * refactor: separate MCP readiness from connection state * fix: preserve on-demand MCP readiness labels * test: satisfy OpenAI conversation ownership guard * fix: keep MCP action predicates boolean * fix: close deferred MCP request context gaps * fix: preserve on-demand MCP configuration actions * fix: fail closed on unavailable MCP parent context * test: complete MCP connecting-state mocks * fix: preserve missing MCP parent on continuations * fix: align native MCP request identities * fix: preserve edited MCP parent identity * test: use scoped Agent initializer fixture * test: expose MCP request body helper * fix: preserve MCP turn identity across resume * style: sort stream metadata imports * fix: carry normalized MCP identity to execution |
||
Dustin Healy
|
f02ce63d57
|
✂️ fix: Strip Redundant Server-Name Prefixes from MCP Tool Keys (#14732)
* ✂️ fix: Strip Redundant Server-Name Prefixes from MCP Tool Keys MCP servers that prefix every tool with their own name produce model-facing keys that embed the server twice once the _mcp_<server> suffix is appended, pushing long tool names past provider 64-character function-name limits. Tool keys now drop a leading <normalizedServerName>_ prefix (case-insensitive, skipped when a sibling tool already owns the stripped name). The original upstream name is recorded as serverToolName on the cached definition and is always what tool calls send to the server, and runtime lookups also try the stripped spelling of persisted pre-strip keys so existing agents keep resolving. * 🩹 fix: Keep Stripped MCP Tool Keys Provider-Safe and Collision-Free Assistant writers submit catalog entries verbatim, so the internal serverToolName mapping is now removed from provider-facing definitions before they reach create/update payloads. Prefix stripping is collision-guarded over the resulting name set rather than raw siblings only, which also covers case-variant prefixed pairs under the case-insensitive match. Assistant payload healing now rewrites a pre-strip persisted key to the stripped catalog key when that key actually exists in the loaded definitions, and legacy agent references keep their persisted spelling as the runtime instance name so per-tool options stay applied while the upstream call still uses the matched entry's raw name. * 🧷 fix: Harden Stripped MCP Tool Keys Against Heal, Collision, and Cache Edges The pre-strip heal now resolves the key boundary against both raw and normalized server spellings, mapping back to the raw name for the shadow and membership guards, so keys persisted after server-name normalization heal too. Collision detection iterates to a fixpoint so a fallback to a raw name cannot silently collide with another sibling's stripped result, and a stripped remainder equal to a synthetic marker (wildcard or server pin) is never produced. MCP catalog cache slices are versioned so replicas that predate serverToolName never read stripped entries during a rolling deploy; stale slices expire on their own. * 🔎 fix: Resolve Pre-Strip Keys in Event-Driven Definitions and Reinspect Persisted Catalogs The event-driven definitions loader now tries the stripped spelling of a persisted key when the exact lookup misses, keeping the persisted name so it matches the runtime instance, which stops legacy agents from failing initialization with expected tools unavailable. The registry storage schema version is bumped so followers rebuild persisted toolFunctions instead of republishing pre-strip definitions into the versioned catalog namespace. The assistants heal also fails closed when a normalized-suffix reference lands on a contested server-name slot, since rewriting persisted data must not bind an ambiguous reference to the tie-break winner. * 🛰️ fix: Reserve the Synthetic OAuth Name and Heal User-Owned Server Keys A stripped remainder equal to oauth would make the client stream handlers treat a real tool call as a synthetic authentication prompt, so it joins the reserved remainders alongside the wildcard and pin markers. The assistants heal now audits the FULL accessible server set on every run instead of operator config names only, since assistants reference user-owned servers whose catalogs the definitions loader already resolves; an unavailable audit still skips healing entirely. * 🧬 fix: Verify Upstream Identity for Legacy Keys and Reserve Sibling Raw Names Stripped results now reserve every sibling's raw name even when that sibling itself strips, so a stripped key can never shadow another tool's pre-rollout persisted references within the same snapshot. Every legacy fallback (runtime lookup, event-driven definitions, assistants heal) accepts a stripped-spelling match only when the entry's recorded serverToolName proves the same upstream tool, so a stale key for a removed tool degrades to unavailable instead of calling a different sibling. To keep that identity visible to the heal, assistant tool definitions retain serverToolName and the controllers sanitize entries through toProviderToolDefinition at the provider submission boundary instead. The agent editor migrates pre-strip persisted ids the same identity-verified way, with the upstream name exposed on the MCP tools payload. * 🧭 fix: Heal Wildcard Tool Options and Reserve the OAuth Namespace Wildcard-expanded catalogs rename stripped tools without any agent.tools entry to preserve the spelling, so buildToolClassification now aliases persisted pre-strip tool_options keys onto the current instance names in place, identity-gated on the definition's recorded upstream name and never overriding an explicit entry. Both loading modes flow through it: instances carry mcpServerToolName from createToolInstance and event-driven definitions thread serverToolName from the catalog. stripServerNamePrefix also reserves the entire oauth namespace rather than the exact name, since the client stream handlers classify every oauth-prefixed key as a synthetic authentication call. * 🛡️ fix: Derive the Full Reserved Namespace and Heal Approval Policies The reservation guard now covers every namespace consumers classify by prefix: the wildcard and server-pin markers alongside oauth, plus the server-scoped mcp_ pluginKey namespace that pre-strip keys could never enter. Stripping also never produces a key whose isActionTool classification differs from the raw key's, since a server whose normalized name contains _action_ would otherwise see a real MCP tool routed down the OpenAPI action path past MCP authorization. Admin toolApproval globs written against upstream tool naming keep applying: pattern lists are healed at run wiring with the current names of tools whose pre-strip spelling matches, list-level so deny, ask, and allow precedence is unchanged and a non-matching deny can no longer fail open. The MCP tools wire type also declares serverToolName end to end. * 🪪 fix: Alias Both Key Spellings for Approval Policies and Hook Matchers Identity aliases are now collected once at tool classification, in both directions: a stripped instance aliases its pre-strip spelling and a legacy-named instance aliases its current catalog spelling, with the current name recorded on legacy matches by the runtime lookup and the event-driven definitions loader alike. The aliases ride the agent config through both loading modes, so approval pattern healing applies to deny rules written against either spelling, closing the bypass where a rule targeting the current name missed an unedited agent's legacy instance. Programmatic approval hook matchers get the same treatment: each hook is additionally registered under an anchored exact-name pattern for tools whose other spelling its regex matches, keeping the admin's matcher semantics intact while argument, user, and tenant specific deny or ask decisions keep executing for renamed tools. * 🔁 fix: Alias Tool Options in Both Spelling Directions Options aliasing now consumes the same bidirectional alias pairs as policy healing and hook matchers, so options the editor migrated to the current catalog spelling still reach a legacy-named instance retained by an unedited agent.tools entry. The previous serverToolName-only derivation skipped exactly that case since the legacy key equals the instance name there. * 🤝 fix: Reserve the Agent Handoff Namespace Before Stripping The client renders any lc_transfer_to_ prefixed call as an agent handoff and the background and intent passes exclude such names, so a stripped remainder inside that namespace would misclassify a real upstream tool. It joins the mcp_ pluginKey namespace as a bare-prefix reservation, which pre-strip keys could never enter. * ⚡ fix: Reuse the Loader's Server Snapshot and Index the Editor Catalog getAssistantToolDefinitions now returns the accessible-server snapshot from the same merged registry read that resolved the catalogs, and the heal consumes it instead of repeating the app-config and registry round trips on the assistant write path; without a snapshot the heal still fetches and fails closed as before. The agent editor's id migration uses a memoized tool_id map, so the per-key form heal does constant-time lookups instead of scanning the catalog per option. --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
Yorgos K
|
9f8d71a3c5
|
🪢 fix: Preserve Response Identity and Branch During Resumable SSE Sync (#14788)
* fix(client): preserve resumable response identity Fixes #14787 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(client): align resumable sync regressions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(client): clarify resumable response ownership Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(client): preserve resumed regeneration ordering Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(client): cover missing resumed response row Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(client): preserve resume identity on page reload * fix(client): replace reassigned resume placeholder * fix(client): preserve content during response id handoff * fix(client): limit resume placeholder handoff * fix(client): preserve resume display metadata * fix(client): reconcile resume metadata in one pass * fix(client): reconcile preliminary resume user * fix(client): restore regenerated branch on early abort * test(client): cover external regeneration resume * fix(client): preserve regeneration history on errors * fix(client): replace reused regeneration error ids * fix(client): preserve exact-id regeneration rollback --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Danny Avila
|
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. |
||
|
Danny Avila
|
c7e355b219
|
🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup (#15051)
* 🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup Fixes #15042, fixes #15043. `resume.js` inferred a confirmed stop from the ABSENCE of `failureReason`, but `abortJob` had four `success: false` paths that returned no reason at all. Those settled the occurrence as `interrupted` and pruned the checkpoint on aborts that never landed — including one where a REPLACEMENT generation owned the conversation, which pruned the successor's checkpoint. Every `success: false` return now names itself (`job_not_found`, `already_settled` added alongside the existing `generation_replaced` / `job_still_active`), and a single canonical `isStopConfirmed` predicate decides whether durable state may be settled. `already_settled` confirms a stop — `awaitProviderDrain` has proven the provider segment can no longer persist — so a permanently terminal generation is not answered with a retry loop. Separately, a schedule engine that failed to arm advertised its permanent outage as a transient 503 with `Retry-After`, so a client obeying it would poll forever. Readiness is now tri-state (`starting` / `armed` / `unavailable`): the retry contract applies only while arming is genuinely pending, and a failed arm returns a terminal `SCHEDULES_UNAVAILABLE` with no `Retry-After` and an error-level log. * 🏷️ fix: Declare Schedule Write Gate Return Types `--isolatedDeclarations` requires an explicit return type on the exported factory and on the middleware it returns (TS9007). Adds a named `ScheduleWriteGate` type matching the existing `ShareMiddleware` shape. |
||
|
Danny Avila
|
c5276fc63d
|
⏱️ feat: Run Scheduled Chats Through Durable Agent Triggers (#14939)
* feat: Scheduled Chats — agent-centric scheduled runs creating real conversations
Squash of the full review-hardened branch (PR 14540, supersedes 14373) onto
latest dev, preserving the exact verified tree. History prior to this commit
lived on the pre-squash branch; every invariant below survived 25 Codex review
rounds plus two external audit rounds (R26) with regression tests that fail
without their fixes.
Feature:
- Schedules CRUD + side-panel UI (cadence dialog, run cards, Run Now), roles/
permissions (SCHEDULES:USE), interface.schedules availability, per-user limits
and capacity slots, timezone-aware cadence with DST-conservative floors and
misfire grace.
- Engine: single-process claim/fire loop with leases, loopback POST dispatch
(signed schedule-fire JWT claims, per-occurrence idempotency key inside the
route's clientRequestId charset), overlap/balance/capacity/duplicate skip
policies with auto-disable streaks (too_many_failures, insufficient_balance),
reconciliation from retained terminal-job evidence, erasure sweep.
- Scheduled runs create real conversations through the resumable agents chat
path: HITL pauses surface on the card (requires_action), resumes re-apply the
fire boundary's admission policy (revision fence, enabled, global kill switch,
SCHEDULES:USE, availability) before continuing a billed generation.
Correctness invariants (the audit surface):
- Settlement discipline: every persistence-producing write happens-before a
run's terminal outcome write; Stop/complete/pause race through single-winner
terminal CAS claims (dev's TerminalJobClaim substrate) with retained,
completedAt-less evidence for scheduled fires plus an owner-intended outcome
stamp (scheduleOutcome) the reconciler prefers over re-derived success —
round-tripped through the Redis hash mapper.
- Swallowed generation failures (client error content parts) classify to
error/skipped_balance instead of success on both initial and resumed paths;
stale stamps are refreshed evidence-first when persistence plus the Mongo
outcome write both fail.
- Abort honesty: delivery judged by generation ownership and the CAS's actual
from-status; republication escalates to the transport's acknowledged variant;
the Stop route settles only genuinely paused runs.
- Account deletion: one-way barrier (deletionRequestedAt) with auth-cache
tombstone-before-stamp, boundary rechecks across all auth strategies, quiesce
of scheduled + interactive work with durable per-stream abort fences
(positive-evidence acknowledgement only), owner-side finalization markers for
post-terminal billed writes, deferred-deletion sweep (explicitly ensured
partial index) that completes cascades autonomously. Remote OpenAI-compatible/
Responses requests are documented as outside the quiesce and tracked in issue
14594.
- Store compatibility: finalization markers optional on the legacy IJobStore
contract with coherent degradation (registration fails -> synchronous title
fallback; count reads 0).
* fix: fence terminal response persistence from deletion; retain stale-pause evidence
Two blockers from the third external review round.
Terminal persistence visible to account deletion:
- The finalization marker was registered only when post-terminal TITLE work was
possible, but every persistence-owning terminal CAS opens the same window: the
claim drops the job out of the active set BEFORE the response save (and the
background user-message/convo saves), so a deletion quiesce landing there saw
neither an active job nor a marker and could cascade while the admitted request
could still recreate messages. Both controllers now register the marker before
every persistence-owning claim — the fresh-turn path and the HITL resume — and
release it only after their pending saves (and any post-terminal title) have
landed, on success and failure paths alike. The TTL bounds a crash.
- settleAbortFence no longer clears a complete/error fence while
`terminalPersistencePending` is set: terminal at the CAS is not settled while
the owner is still persisting. The job facade now surfaces the flag.
- The marker trio is REQUIRED by the runtime store contract (assertJobStoreV2
refuses a store without it at configure time, keeping the failure loud and
deterministic) while remaining optional on the legacy public IJobStore type for
source compatibility. The silent degrade path from the previous round is gone —
it was not deletion-safe.
Stale-pause recovery retains scheduled evidence:
- Three crash/timeout recovery paths — ApprovalLifecycle.failStalePausePersistence
and the InMemory/Redis stale-pause cleanups — unconditionally stamped
`completedAt`, putting a scheduled fire's failed-pause error terminal on the
short completed TTL. A Mongo outage longer than that TTL erased the evidence and
the reconciler recovered the run as `interrupted` instead of `error`. All three
now follow the controller-observed path from the previous round: scheduled jobs
omit `completedAt` (retained-evidence TTL) and stamp the error outcome.
The PR description now explicitly narrows the deletion guarantee for the remote
OpenAI-compatible/Responses paths (tracked in issue 14594).
* fix: deletion-fence marker protocol — generation-scoped, atomic, fail-closed, all terminal paths
One consolidated pass over the finalization-marker mechanism, per the fourth
external review round. The invariant it establishes: NO persistence-owning
terminal CAS runs without a durable, generation-qualified marker covering the
window it opens, and every consumer treats a pending terminal as unsettled.
- Generation-scoped markers. Entries were keyed (userId, streamId), so a
Stop-superseded generation finishing late could clear the marker its
replacement registered on the same conversation. Marker fields are now
qualified by the generation's createdAt; clears must present the same
identity, and an unqualified legacy clear cannot drop a qualified entry.
- Atomic Redis registration. HSET-then-EXPIRE loses the fresh marker when the
user's existing hash expires between the two commands (or the process dies
there); registration is now a single Lua script carrying both.
- Fail closed everywhere. Registration failure (after one retry) now REFUSES
the terminal CAS instead of proceeding uncovered: the completion claim throws
into the error path, the error path skips completeJob and leaves the job
ACTIVE — deletion-visible by itself, recovered by the stale-running reaper —
and abortJob returns a new retryable `fence_unavailable` failure the Stop
route answers with 503 and the deletion quiesce treats as an unacknowledged
stop (fence kept). The previous round's log-and-proceed is gone.
- Every terminal path enrolled. abortJob now owns its window (register before
the abort CAS, clear in its finally — the Stop route's checkpoint prune and
partial save run inside beforePublish, between CAS and publication); the
interactive and background generation-error paths register before their
completeJob; the resume controller's error finalization registers before its
completeJob. A lost or thrown claim releases the marker after pending saves
flush instead of holding the user's deletion behind the TTL.
- Every pending terminal unsettled. settleAbortFence defers on
terminalPersistencePending for ALL statuses — including `aborted`, whose
route-side persistence the previous guard missed.
Also: a direct Redis regression for scheduled stale-pause retention (the P2
test gap), and the PR description no longer claims to carry every commit.
* fix: lease-token lifecycle fences — same-generation isolation, admission fence, undelivered-Stop retention, legacy abort enrollment
Fifth external review round; four P1s, handled as the requested consolidated
lifecycle-fence pass.
- Lease tokens. Marker fields were (streamId, createdAt), shared by every
contender on the same generation — completion and Stop, or two racing Stops —
so a losing contender's cleanup erased the winner's still-live marker. Every
registrant now carries a unique lease token in the field and may only ever
clear its own lease; unqualified legacy clears cannot touch qualified entries.
- Admission fence. Authentication can pass before the deletion barrier goes up,
and the durable createJob is several async steps later — a deletion quiesce in
that window saw neither an active job nor a marker and could cascade before
the admitted request created its job. The controller now registers an
admission lease and THEN rereads the deletion barrier: the ordering guarantees
either this request observes the barrier (403, lease released, slot/claim
cleanup) or the quiesce observes the lease and defers. Held until createJob is
durable; released on every refusal and initialization-error path. Fail closed
when the lease itself cannot be registered (503 retryable).
- Undelivered-Stop retention. abortJob released its lease in a finally even when
delivery AND publication had provably failed — the job reads terminal
(invisible to active-set scans), a user Stop writes no durable abort fence,
and the remote owner keeps generating and will persist its abort-catch writes
whenever the signal finally lands. The lease is now retained in exactly that
case, and each resignal attempt heartbeats a fresh lease so the fence outlives
the TTL for as long as delivery is still being driven. The abort-winning
turn's own loser-side pending saves are additionally fenced in the controller
catch (best-effort — those writes are already in flight).
- Legacy abort enrollment. abortMiddleware (assistants abort route fallback for
non-assistants endpoints) awaited abortJob and then spent usage and saved the
stopped response AFTER the abort's lease was released. Both writes now run
inside `beforePublish`, between the abort CAS and publication, covered by the
same lease as every other abort.
Barrier tests, each verified to fail without its fix: same-generation lease
isolation (store), racing two-Stop loser cleanup (manager, stale-read forced
CAS race), undelivered-Stop lease retention, resignal heartbeat, admission
refusal with lease-before-reread ordering plus release-on-durable-create, and
legacy-abort persistence inside beforePublish.
* fix: heartbeat-backed owner-lifecycle leases close the settlement handoff races
Sixth external review round: the remaining P1 interleavings were one structural
problem — lease handoffs that were not atomic — resolved as the requested
consolidated lifecycle-lease pass.
- Quiesce reads leases BEFORE the active-job scan. The admission-lease -> durable
-job handoff is only atomic against a reader in the OPPOSITE order of the
writer: writers hold the lease strictly until the job is active-set visible,
so leases-first shows every interleaving either the lease or the job.
Jobs-first allowed a request to create its job after the scan and release its
lease before the count — hiding both, cascading, and letting the new
generation persist into a deleted account.
- The abort acknowledgement is fenced by the owner-lifecycle lease. Redis ACKed
the moment the owner's AbortController tripped; the stopping side released its
lease on that ACK while the owner's asynchronous abort-catch persistence was
still ahead. The transport now awaits a manager-installed pre-ACK hook that
registers a DETERMINISTIC owner lease (exactly one owner exists per
generation, and determinism is what lets the signal-time registrant and the
owner's catch-side release agree across processes) before the acknowledgement
is persisted or published; an owned same-replica abort bridges to the same
lease before tripping its local controller. Both generation-owner catches
(fresh turn, resume) release it once their writes land.
- A failed replacement handoff no longer orphans the predecessor. The atomic
replacement removes it from active storage, and an unconfirmed handoff
terminalizes the replacement too — leaving nothing a quiesce could discover
while the predecessor's provider may still be generating. Its owner lease is
now retained at the point the receipt fails delivery; the owner replica renews
it through the pre-ACK fence when the signal finally lands.
- Leases HEARTBEAT while held. The five-minute store TTL only bounds a crashed
holder; live persistence — a stalled save, a long deferred title — must never
outlive its own fence. holdUserFinalization registers and renews every minute
until released; the controllers' completion/error/admission leases all hold.
The undelivered-Stop retention moved to a deterministic `stop` lease that
every resignal attempt renews and the first successful one clears (no more
opaque leases accumulating to TTL), and a THROWN abort transition releases the
contender lease instead of leaking it.
- The user-document abort-fence mutations now invalidate the auth user-doc
cache, matching every other user-doc write.
Barrier tests, each verified to fail without its fix: quiesce lease-scan
ordering (plus the observed-lease defer), pre-ACK fence ordering at the
transport, owned-abort owner-lease bridging, replacement-handoff predecessor
retention, held-lease heartbeat past the TTL, and failed-then-successful
resignal reaping the retained stop lease.
* fix: one manager-owned owner-lease span across every abort delivery path
Seventh external review round; four lifecycle-fence gaps, closed by making the
owner-lifecycle lease a single manager-owned, heartbeat-held span.
- Fail-closed acknowledgements. The pre-ACK hook registered a one-shot lease and
the transport ACKed even when it failed; a same-replica owned abort likewise
swallowed registration failure. The hook now acquires a HELD owner lease
(heartbeat until the owner's catch releases it via releaseOwnerLease) and a
rejection SUPPRESSES the acknowledgement — the stopping side stays retryable
behind its retention lease, and every resignal re-drives the handler. The hook
also stopped gating on `job.createdAt === generationId`: during a replacement
handoff the store holds the replacement while the abort targets the
predecessor, and that gate silently skipped exactly the generation being
acknowledged (the store job is owner identity, never a generation gate). A
local owned abort acquires the same held lease before tripping its provider;
post-CAS the trip cannot be withheld, so acquisition failure downgrades
delivery and the retention handoff keeps the user fenced.
- Committed-but-lost-reply disambiguation. A thrown abort transition released
the contender lease as if nothing had happened, but a Lua CAS can commit and
lose its reply — an aborted job invisible to active-set scans whose provider
was never signalled, with no fence left. The throw path now re-reads the exact
generation: only a job still live under the caller's identity proves no
commit; committed or ambiguous outcomes hand the fence to the deterministic
stop lease (kept on the contender lease if even that fails) before rethrowing.
- Replacement handoff covered end to end. A LOCAL replacement abort acquires the
predecessor's held owner lease before the trip (failure reports the receipt
undelivered, engaging retention). Failed-handoff retention is no longer a
swallowed one-shot: it heartbeats with the durable acknowledgement proof as
its renewal predicate — acquisition failures keep retrying for as long as the
fence is needed, and the retainer stands down (without clearing the shared
field) once the remote owner ACKs and thereby holds its own lease.
- A LOCAL resignal delivery hands off to the owner lease BEFORE clearing the
retained stop lease, and keeps the stop lease when that handoff fails.
Barrier tests: hook rejection suppressing the ACK, commit-then-lost-reply
retention with its provably-uncommitted counterpart, local-resignal owner
handoff ordering, pre-ACK owner lease held past the store TTL until release
(and provably stopped after), and failed-handoff retention retrying on its
heartbeat — verified fail-before/pass-after by stashing the fixes.
* fix: finish scheduled chat lifecycle hardening
* fix: close scheduled chat review follow-ups
* fix: generation-fence abort recovery evidence
* test: wait for settled approval tool output
* test: preserve scheduled init reconciliation option
* refactor: rebuild scheduled chats on durable agent triggers
* test: reset MCP cache mock between cases
* test: isolate scheduler startup in server specs
* fix: harden scheduled run lifecycle
* fix: normalize schedule capacity conflicts
* test: type schedule collision fixture
* fix: re-fence scheduled resume and expiry
* fix: fence scheduled resume handoffs
* fix: release superseded manual schedule leases
* fix: release failed run-now claims
* fix: release superseded engine claims
* fix: repair schedule dialog interaction and rework its form
The agent picker was unusable: ControlCombobox portals its popover to the
body by default, which lands it outside the dialog's Radix focus trap. Clicks
passed through it, it could not be tabbed into, and the trap fighting Ariakit
for focus locked the page up on selection. The prop is documented for exactly
this case — pass `portal={false}` and give the dialog `overflow-visible`, as
ProjectButton already does. The time and day dropdowns defaulted the same way.
Alongside that:
- Extract the agent builder's instructions editor (special-variable menu plus
expand-to-fullscreen) into a controlled `VariableEditor` and use it for the
schedule prompt. Insertions now route through `onChange`, so react-hook-form
sees them — the schedule PATCH is built from `dirtyFields`, and a `setValue`
that skipped dirty tracking would have dropped an inserted variable silently.
- Replace the hand-rolled frequency buttons with the shared `Radio`. They marked
the selection with `bg-surface-hover` on an outline button whose hover is the
same token, so the selected option was indistinguishable from a hovered one;
`Radio` is also a real radiogroup rather than four `aria-pressed` toggles.
- Wrap the fields in a real `<form>` and associate the footer button by id, so
Enter submits. Group the frequency, day and time controls in fieldsets.
- Add placeholders for name and prompt, match the textarea fill to the other
fields, and label the hourly case as minutes past the hour.
- Widen the dialog to `md:max-w-3xl` and pair name with agent so the form fits
without scrolling on desktop.
- Move scheduled chats below skills and above prompts in the side nav.
The new dialog spec fails when the portal fix is reverted.
* test: cover scheduled and subagent deletion drains
* refactor: own the form-control appearance in the client primitives
Addresses the codex finding on ScheduleDialog: a feature-local `FIELD_CLASS`
restated the `Input` primitive's border, radius, height and background so it
could be pasted onto the schedule dropdowns, leaving those controls with no
connection to the primitive they were imitating.
Move that appearance into `packages/client/src/components/Field.ts` as the
single source `Input` and `Textarea` now compose, and give `Dropdown` and
`ControlCombobox` a `variant="field"` that applies it. The schedule dialog
passes the variant and carries no class strings of its own.
This also repairs a break the dev merge would otherwise have introduced: the
newer `Dropdown` splits `className` (wrapper) from `triggerClassName`, so the
old pasted classes would have landed on the wrapper and left the triggers
unstyled.
The semantic-token guard now watches the shared module and asserts each
primitive still composes it, which covers more than the two files it read
before.
* fix: keep an explicit schedules disable from becoming an opt-in
`use` is two things at once for a dual-purpose runtime interface field: a
permission bit, which DB overrides strip, and the runtime disable signal that
`getLimits` reads. Stripping it from `{ use: false, maxPerUser: 2 }` leaves an
object, and `getLimits` treats any object without `use: false` as enabled — so
an admin override written to stop scheduled billing for a role or user started
it instead.
Collapse an explicit disable to the boolean form before the strip, on both
paths that accept it: the `interface.schedules` field patch, which admitted the
object wholesale because bare runtime paths deliberately bypass the permission
gate, and the overrides merge, which reached the composite-field branch and
kept `maxPerUser`. Objects that only narrow limits are untouched, so a
principal can still be given a smaller cap.
Both regressions fail without the normalizer.
* fix(schedules): Wave A — null-balance CAS, atomic paused-card clear, clustered erasure sweep
Slice 1 (thread r3804518381): route the existing-null balance initialization
through a { user, tokenCredits: null } compare-and-set instead of a blind $set,
so a concurrent initializer/charge landing between the preflight read and the
write is never handed back its spent starting balance. On a CAS miss the
preflight re-reads the winner. The absent-record $setOnInsert path and the
credited-record refill-config sync are unchanged. Adds the initializeNullBalance
adapter (no upsert) and regression coverage for winner/miss/sync cases.
Slice 2 (thread r3804518388): updateScheduleById now drops a `requires_action`
lastRun projection atomically with the configRevision bump. Any pause present at
edit time was projected under the pre-edit revision and can never be replaced by
its own revision-fenced terminal outcome — a disabling edit would strand the
card on "Needs approval" forever. Implemented as classic-operator CAS branches
(DocumentDB rules out a conditional pipeline $unset), fenced on the card STILL
being the pause so a terminal outcome or newer occurrence that races in is
preserved. Terminal history survives untouched.
Slice 4 (thread r3803826204): expose initializeScheduleErasureSweep from the
schedule runtime facade and start it in every clustered (experimental) worker
after Mongo is up. It re-drives eraseScheduleIfDrained for soft-deleted rows so
a hidden prompt cannot outlive its drain when the delete/erase-on-settle
attempts miss. It arms nothing else and never infers owner death from a
process-local missing job (isTopologySafeToArm gates that).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.1 — reversible account-deletion schedule suspension
Thread r3804518383. Account-deletion quiesce marked every schedule `deleting`,
disabled it, and cleared nextRunAt destructively. When a later cascade step (or
the drain itself) failed, the controller cancelled the user-deletion fence —
restoring the user — but the schedules stayed `deleting` and were erased by the
sweep, silently losing all of a live user's scheduled prompts.
Replace the destructive marking with a REVERSIBLE, token-fenced suspension:
- suspendUserSchedulesForDeletion(userId, token) snapshots each schedule's prior
enabled/nextRunAt under a per-attempt token, then fences firing (disable, clear
nextRunAt, rotate claimToken). It never sets `deleting`, so a suspended row is
not erasure-eligible. Snapshotting reads then bulkWrites (a classic update
cannot copy field values under DocumentDB), fenced per row so an already-
suspended/soft-deleted/edited row is left alone; idempotent per token.
- restoreUserSchedulesFromDeletion(userId, token) reverses it, re-enabling and
re-arming only rows still carrying the exact attempt token and not independently
deleted — so an owner-deleted or newer-attempt-suspended schedule is never
resurrected.
- deleteUserController generates the attempt token, passes it to quiesce, and on
any failure that cancels the user-deletion fence restores the suspended rows. A
successful deletion hard-deletes them (and their snapshots) via the existing
cascade and never restores.
Adds a `deletionSuspension` embedded field (excluded from the wire schedule),
data-method regression tests (suspend/restore/fence/idempotency/no-resurrect),
and controller tests (restore on drain-false and post-quiesce cascade failure,
no restore on success).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave B.2 — wire the interactive Stop persistence protocol
Thread r3804255932. The durable Stop primitives (requestRunAbort 'stop',
getScheduleRunAbortState, markRunAbortPersisted) already existed but production
only used requestRunAbort(..., 'deletion'). An interactive Stop flipped the job
to `aborted` and then persisted its partial message + checkpoint inside
`beforePublish`, without ever stamping the schedule Stop or acknowledging it —
so reconciliation, the generation owner, or a concurrent schedule/account
deletion could terminalize the run and release its capacity (and erase data)
mid-write.
Wire the request -> persist -> acknowledge -> settle barrier through the
schedule runtime (the route never touches raw Mongo):
- Expose beginScheduledStop / acknowledgeScheduledStopPersistence on the service.
- The abort route stamps the Stop BEFORE signalling abortJob (a serialized
'in_progress' loser returns 409 STOP_IN_PROGRESS without a second abort),
acknowledges only after beforePublish persistence succeeds, and on a
persistence failure leaves the barrier unresolved so the run stays preserved
(client retries; stale-owner timeout is the bounded recovery). A failed abort
releases the stamp it placed so a replacement/retry is never blocked through
its predecessor.
- recordScheduleOutcome (the owner settlement path) now waits, bounded, for the
Stop acknowledgement before terminalizing; a resolved/non-stop/stale marker
proceeds immediately. The paused Stop settles only after its own ack.
Adds service-layer barrier tests and route-level ordering/persistence-failure/
in-progress tests; the data-layer serialization is already covered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): Wave C.1 — reconcile durable trigger delivery with the reservation
Threads r3803826192 (manual limiter) and r3804255924 (PII/moderation), plus the
independently-found long-Retry-After race. fireSchedule reserves a `started` run
and a global capacity slot BEFORE the durable trigger delivery reaches the chat
route, where an interactive limiter (manual Run Now), PII, or moderation can
reject it before any generation job exists — dead-lettering the delivery while
the run sat `started` until the 30-minute orphan sweep mislabeled it interrupted.
And a valid delivery deferred by Retry-After (up to 24h) could be orphan-settled
and have its capacity released, then fire anyway.
Translate durable delivery state into the schedule outcome:
- Store the deterministic trigger deliveryKey on the ScheduleRun reservation
(computed from the envelope BEFORE enqueue, so an ambiguous commit still has it).
- Add a getTriggerDelivery engine dep (wired to the merged trigger service's
getDelivery) that reads the durable delivery by key.
- Schedule reconciliation, for a jobless `started` run: staging/pending/leased →
admission is live, never orphan; dead → record `error` from the durable
lastError and release capacity promptly (no 30-minute wait), through the
ordinary outcome/auto-disable path; succeeded or no record → the existing
legacy orphan policy (interrupted only past the cutoff); a delivery lookup
failure defers rather than orphaning a possibly-live delivery. Limiter/PII/
moderation middleware writes no schedule state.
Adds reconcile state-mapping tests (dead/pending/leased/staging/succeeded/none/
lookup-failure) and a fire test that the reservation's deliveryKey equals the
enqueued delivery's idempotency key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): Wave C.2 — durable retry/ack for terminal host lifecycle actions
Thread r3804518375. Approval expiry won the `requires_action → aborted` CAS and
then invoked the host hook best-effort: `runApprovalExpiredHandler` swallowed a
failure, and because later sweeps enumerate only `requires_action` jobs, the now-
aborted job was never offered again. In the clustered entrypoint (no schedule
reconciler) the ScheduleRun stayed `requires_action` and its retained job
persisted indefinitely.
Make the host lifecycle work durable rather than schedule-specific:
- Add a generic `terminalHostActionPending` marker, set ATOMICALLY in the same
terminal transition (ApprovalLifecycle.expireWithIdentity), only when a host
adapter is installed.
- Retain and index such jobs: both stores keep them out of terminal reaping and
expose getTerminalHostActionJobs(); Redis adds a set + extended (24h-bounded)
TTL, in-memory a bounded 24h retention so a permanently-failing hook cannot leak.
- The manager clears the marker only after the adapter acknowledges success,
fenced by generation identity (clearTerminalHostAction), so a replacement
generation can neither clear nor execute its predecessor's action.
- cleanup()/expireStaleApprovals() enumerates unacknowledged terminal host actions
across restarts and replicas and retries the idempotent hook; the relay only
re-invokes while the marker is unacknowledged, so a successful ack prevents
duplicate work. Store-won expiry marks it too, so a loser-replica relay still
crosses the hook.
- Terminal SSE notification continues regardless of host-hook outcome.
Covers in-memory behavior (retry after failure, restart/other-replica retry, ack
prevents duplicates, identity fence, terminal notification on failure, no marker
accumulation for non-scheduled jobs) and updates the Redis cluster-membership
contract test for the new index.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* style(schedules): satisfy import sorting in fire.ts and fire.spec.ts
CI "Static checks" failed on IMPORT_SORT for the two files Wave C.1 added imports
to (the AgentTriggerEnvelope type import and getAgentTriggerIdempotencyKey).
Applied scripts/sort-imports.mts to exactly those files — imports-only reordering,
no behavior change. Other files reported by a repo-wide check are pre-existing on
dev and deliberately left untouched so this PR is not widened.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): repair the deletion CLI and order restore before the fence release
Addresses three findings from the fresh Codex review.
P1 — config/delete-user.js called methods.disableUserSchedulesForDeletion, which
Wave B removed in favor of suspendUserSchedulesForDeletion. The file is
@ts-nocheck and its spec mocked the removed name, so neither typecheck nor tests
caught it; the real CLI would throw a TypeError before deleting anything and then
only unwind the fence. The CLI now uses the tokenized protocol: it mints a
suspension token, suspends with it, and restores that exact attempt's rows in its
finally block when the deletion does not commit. Its spec mocks the real methods,
so the breakage can no longer hide.
P2 — both the HTTP controller and the CLI released the user-deletion fence BEFORE
restoring schedules. That fence is what refuses new schedule writes/claims, so the
gap let an owner PATCH edit a still-suspended row and have its enabled/next-run
state overwritten by the older snapshot, and let a second deletion attempt
re-suspend under a new token — making the first restore a no-op and stranding the
disabled snapshot permanently. Restore now runs first, while writes are still
fenced.
Hardening for the same defect class across a crash: suspendUserSchedulesForDeletion
now ADOPTS an existing suspension's snapshot when re-suspending a row abandoned by
an earlier attempt, instead of re-capturing the row's current (already-suspended)
state. Without this, an attempt that died before restoring would have its
successor snapshot "disabled, no next run" and permanently strand the schedule.
Tests: CLI restore-before-fence ordering and no-restore-on-success; the same
ordering assertion on both controller post-quiesce failure paths; a data-method
regression that a second attempt adopts the abandoned snapshot and restores the
original enabled/next-run state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge dead deliveries in topology-safe maintenance
Codex finding: the `dead` delivery mapping added in Wave C.1 lives only inside
startScheduleEngine's reconciler, but the clustered entrypoint arms no engine — it
runs erasure-only maintenance. A delivery queued before a restart into clustered
mode and then rejected before generation creation (interactive limiter, PII,
moderation) dead-letters while its ScheduleRun stays `started`, holding a global
capacity slot indefinitely for an ordinary non-deleting schedule.
Add a dead-delivery convergence pass to the erasure sweep, so every topology that
runs schedule maintenance settles it. The pass is POSITIVE-EVIDENCE-ONLY and is
therefore safe where absence-based reconciliation is not: a `dead` delivery is
durable shared state proving no generation owns the reservation. It settles only
when the job is confirmed absent or identity-mismatched (an identity-matched job
still owns the run), defers on an unknown job lookup, on an in-flight abort, and
on an in-flight resume hand-off, ignores legacy reservations with no deliveryKey,
and applies a short grace so an accepted delivery still creating its generation is
never settled mid-handoff. Auto-disable policy is deliberately left to the armed
engine; this path records the failure and frees the slot.
Deliberately does NOT touch api/server/experimental.js — the clustered entrypoint
already starts this sweep, so the convergence arrives through the existing
initializer and the shared-file footprint stays as-is.
Tests: settles a dead delivery as error under an explicitly UNSAFE topology,
leaves live deliveries alone, never settles under an identity-matched running
generation (delivery is not even consulted), defers an in-flight abort, and
ignores a reservation with no deliveryKey.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(stream): refresh host-action retention on each retry attempt
Codex finding: unacknowledged terminal host-action evidence was capped at the
24h pause TTL, so a host dependency (Mongo) unreachable for longer than that let
the Redis key — and its pending marker — expire with no generation-fenced
acknowledgement, stranding the ScheduleRun where no reconciler is armed.
Measure retention from the LAST retry rather than from the terminal transition:
enumerating a pending host action IS the retry attempt, so both stores refresh
its retention as they hand it to the hook (Redis re-EXPIREs the job key; the
in-memory store stamps terminalHostActionRefreshedAt and bounds from it). Evidence
therefore survives as long as some replica is still actively retrying, while a
deployment that stops sweeping entirely still lets it age out — so this does not
reintroduce the unbounded leak the cap existed to prevent.
Test: after a failed hook, a later cleanup pass keeps the marker pending and moves
its retention basis forward.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): defer settlement when the Stop barrier times out
Codex finding: waitForStopPersistence returned after its 5s poll budget even when
the Stop was still fresh and unacknowledged, and recordScheduleOutcome then went
straight on to terminalize the run — releasing its capacity, deletion, and erasure
barriers while beforePublish may still have been writing. Slow checkpoint cleanup
is indistinguishable from a dead route on that signal, so the timeout was being
treated as if the barrier had been satisfied.
The poll budget now means "undecided", not "clear". On timeout with a fresh,
unacknowledged Stop the barrier DEFERS: recordScheduleOutcome returns false
without recording, leaving the run active/preserved. Settlement then happens
either when the route acknowledges, or once the existing stale-owner cutoff
(ABORT_OWNER_PRESUMED_ALIVE_MS) authorizes a later attempt — which the loop
already treats as clear-to-settle. Callers with durable retry (the approval-expiry
host action, reconciliation) re-drive it, so a deferral converges rather than
stranding the run.
Test: a fresh Stop that never acknowledges within the budget reports not-settled
and records no outcome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): require definite delivery failure, extract its message, converge deferred Stops
Third Codex round. Three findings in code from this closeout, plus one pre-existing
P1 that is a one-line operator-safety fix.
P1 — dead-delivery settlement demanded too little evidence. `dead` does not prove a
request was rejected: the trigger host marks response timeouts and invalid success
responses `certainty: 'ambiguous'`, and the engine dead-letters those once retries
are exhausted. The erasure sweep treated every dead letter as positive evidence, so
an ambiguous one sitting over a generation a peer had accepted could terminalize the
run and release its capacity mid-flight. It now settles only on a DEFINITE rejection,
unless job absence is deployment-authoritative (safe topology), where the
confirmed-absent job is itself the evidence.
P1 — `lastError` is an `AgentTriggerDeliveryFailure` object, not a string. A
duplicated local interface declared it `string` (against CLAUDE.md's no-duplicate-
types rule), so both the sweep and the engine reconciler passed the object into the
String-typed run/schedule `error` fields; Mongoose would reject the cast, the per-row
catch would swallow it, and the run would keep its global capacity slot. The dep type
now reuses the canonical `AgentTriggerDeliveryFailure` and both call sites pass
`.message`. Re-typing immediately surfaced a stale test that had asserted a string.
P1 (pre-existing) — the base-config global stop is honored in `getLimits` via
`isRuntimeDisabled` rather than a literal `=== false`. The stop has two shapes, and
deepMerge turns base `{ use: false }` plus a principal override of `true` into
`{ use: true }`, so the literal check reported the feature enabled and Run Now
dispatched straight through fireSchedule, bypassing the operator's emergency stop.
Now the same predicate the engine gate already uses.
P2 — a Stop whose settlement DEFERRED past the poll budget had no convergence path
where no reconciler is armed. `acknowledgeScheduledStopPersistence` now optionally
re-drives the terminal outcome once the barrier clears; `recordRunOutcome` is
match-guarded and idempotent, so an owner that already settled makes it a no-op. The
abort route passes it for a running generation; a paused job still settles explicitly.
Tests: ambiguous dead letters refused under unsafe topology but settled when absence
is authoritative, definite rejections settled either way, the failure message carried
through, and the abort route's re-drive present for running / absent for paused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
* fix(schedules): converge terminal runs in clustered workers, retry suspension restore
Fourth Codex round on
|
||
|
Danny Avila
|
5e3c680761
|
🪃 feat: Wake Parent Agents on Child Completion (#14975)
* feat: wake parent agents on child completion * wip: harden child completion wakeup lifecycle * fix: close the completion-wakeup static failures Type the durable-claim store fixture, the continue-envelope test helper, and the terminal message's task metadata so the wakeup suites compile against the shapes they actually exercise. Replace `Array.prototype.at`, which the package target library does not provide. Capture the prepared child thread in a non-optional local before the provider callback closes over it, and narrow the trigger envelope itself on `mode === 'continue'` rather than a separately copied mode, so reading the continue target is sound. Lift the parent-message fallback out of a nested ternary into a named resolver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * test: cover the active-predecessor admission fence The Redis job-creation call gained a thirteenth scalar argument, so the spec helper reconstructed the HSET pairs one slot early and rebuilt an invalid job hash; three creation tests failed on that alone. Give the fence itself direct coverage in both store adapters, which it had none of despite deciding whether an automatic continuation may replace a live parent turn. Each proves a running and a requires_action predecessor are refused with the state a controller needs for a finite 409, that an absent or settled predecessor is admitted, and that an ordinary user turn without the policy still replaces its predecessor. The Redis case also asserts a refused continuation leaves the parent's durable job and chunks untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1cCMDrTWaRNkmtKjpWELZ * fix: harden completion wakeup rollout and claims * fix: close completion wakeup race windows * test: keep the child store fixture exact * fix: close final subagent wakeup gaps * fix: preserve ambiguous completion claims * fix: release pre-admission wakeup claims * fix: stabilize subagent completion recovery --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Danny Avila
|
e4d6bb71f9
|
📁 feat: Surface Stateful Workspace Downloads (#14984)
* feat: surface stateful workspace downloads * fix: sort workspace change imports * fix: reuse workspace button primitives * fix: hide collapsed workspace actions |
||
|
Dustin Healy
|
a33b128c47
|
🪪 fix: Preserve Stored Access Token Expiry Over ID Token Exp (#14982)
* 🪪 fix: Preserve Stored Access Token Expiry Over ID Token Exp extractOpenIDTokenInfo let the ID token exp claim overwrite the token set's stored expires_at. The ID token is minted at login and never refreshed, so once a session outlives the ID token TTL, isOpenIDTokenValid reports the access token as expired even when expires_at is hours in the future, and OpenID placeholder substitution silently stops: MCP headers configured with {{LIBRECHAT_OPENID_ACCESS_TOKEN}} ship the literal placeholder string as the bearer credential and the receiving server rejects every connection with an unparseable JWT until the user fully logs out and back in. The ID token exp now only fills a missing expiresAt instead of overriding a stored one. Identity claim enrichment from the ID token is unchanged, and the exp fallback for token sets without expires_at is preserved. * 🪪 fix: Validate ID Token Expiry Before ID Token Placeholder Substitution The precedence fix made isOpenIDTokenValid track only the access token expiry, so an MCP header using {{LIBRECHAT_OPENID_ID_TOKEN}} could substitute an ID token that had already expired. The ID token exp is now preserved separately as idTokenExpiresAt and checked at the ID token substitution site, so an expired ID token substitutes empty rather than a stale credential while access token substitution is unaffected. * 🪪 fix: Address OpenID Expiry Review Round Fix expires_at at the source in the OpenID JWT strategy. The stored value described the incoming bearer's exp even when access_token came from the session or a cookie, so it could describe a different credential entirely. A new decodeJwtExpiry helper reads the exp of the token actually stored, and payload.exp is kept only when the raw bearer is the resolved access token. Opaque session or cookie tokens now store no expiry rather than a wrong one. Apply a 30 second clock skew buffer in isOpenIDTokenValid and isIdTokenCurrent via a new exported OPENID_EXPIRY_BUFFER_SECONDS, mirroring OPENID_REUSE_EXPIRY_BUFFER_SECONDS in AuthController. Tokens that would expire in transit are treated as already expired. Make isIdTokenCurrent fail closed when idTokenExpiresAt is absent. exp is REQUIRED in an ID token, so a missing value means the token is malformed or the claims parse threw. The check uses == null so an exp of 0 counts as present and therefore expired. Read the ID token exp with a numeric type check so an exp of 0 records idTokenExpiresAt and fails closed downstream while a non-numeric exp is ignored, and compare the stored expiry with != null so a gap filled expiry of 0 reads as expired instead of as no expiry at all. Raise an actionable re authentication error for the ID token placeholder instead of substituting an empty string. An empty substitution produced a malformed Authorization header and a 400 downstream rather than a clean signal that the user must re authenticate. Raise the same re authentication error from processSingleValue when a user has an OpenID identity, the stored token set is no longer valid, and the value still contains a credential bearing OpenID placeholder, so the expired access token case that motivated this PR signals re auth instead of silently shipping or stripping the placeholder. Only the access token, ID token, and generic token names raise: identity metadata resolves from the user document and an expiry hint never needed a token, so those keep their existing literal then strip behaviour. Unknown placeholder names also stay literal and diagnosable, matching the existing resolvable placeholder policy. Add the comments the review asked for on the exp fallback heuristic, the EXPIRES_AT placeholder semantics, why stale ID token claims stay usable for identity fields, and the advisory nature of the freshness check. * 🪪 fix: Honour Opaque Access Tokens And Type The OpenID Re-Auth Error Drop the ID token exp fallback in extractOpenIDTokenInfo. Storing the access token expiry honestly means an opaque access token now records no expiry, and the fallback then handed the ID token exp authority over a credential it does not describe. A deployment issuing opaque access tokens alongside a short lived ID token saw isOpenIDTokenValid go false and the credential guard reject a perfectly good access token, which worked before this branch. An unknown access token expiry is now treated as no expiry, and the ID token exp only ever gates ID token substitution through idTokenExpiresAt. Give the re-authentication signal a type. OpenIDReauthRequiredError is raised at both the ID token placeholder and the credential placeholder guard, ErrorController maps it to a 401 carrying the actionable message, and the class exposes statusCode so the agent generation path answers 401 instead of a bare 500 for the same condition. Omit rather than blank a header whose credential placeholder is still unresolved on a final resolution pass, since an empty bearer credential is malformed under RFC 6750 while an absent header lets the upstream answer its own challenge. Identity placeholders keep stripping to an empty string. Move the resolvable placeholder docblock onto the pattern it describes, resolve an EXPIRES_AT of 0 as the string 0 for consistency with the neighbouring null checks, and let AuthController consume the exported OPENID_EXPIRY_BUFFER_SECONDS so the 30 second skew allowance has a single definition. |
||
Ravi Kumar L
|
da0491d5db
|
💻 fix(agents): require Code Interpreter for programmatic MCP tools (#14977)
* fix(agents): require code interpreter for programmatic MCP tools * test(data-provider): fix tool options fixture type * fix(agents): address programmatic tool review feedback * fix(agents): avoid no-op update on version revert |
||
|
Danny Avila
|
547bd8c4bf
|
🧵 feat: Persist View-Only Subagent Threads (#14957) | ||
|
Danny Avila
|
7d62be2ad3
|
🕸️ feat: Run Saved Agent Teams as Subagents (#14944)
* feat: Add graph subagent integration * style: Sort response usage test imports * fix: Preserve lazy graph runtime context * fix: Use isolated graph input helper * test: Align graph integration fixtures * fix: Preserve lazy graph runtime capabilities * fix: Bound lazy graph metadata preload * fix: Harden lazy graph resolution lifecycle * fix: Coalesce lazy graph member resolution * fix: Snapshot initialized graph members only * fix: Preserve lazy agent runtime context * fix: Preserve batched lazy context preparation * fix: Preserve graph member capability bounds * fix: reconcile graph subagents with execution profiles * style: align graph subagent types with formatter |
||
|
Danny Avila
|
aa35cd42b1
|
📬 feat: Add Durable Agent Trigger Delivery (#14925)
* feat: wire trusted agent trigger dispatch * feat: add durable agent trigger delivery * fix: annotate trigger envelope byte limit * test: isolate trigger startup in server specs * fix: fence trigger delivery during account deletion * test: isolate trigger service in user controller specs * fix: close trigger deletion admission race * fix: harden account deletion fences * fix: close durable trigger review gaps * fix: require offline stale-fence recovery * fix: type trigger lane sequence ids * fix: fence admin user deletion triggers * fix: make trigger deletion recovery durable * fix: harden offline user deletion * fix: serialize trigger lane publication * style: sort trigger delivery imports * fix: recover orphaned trigger publications * fix: preserve trigger recovery ordering * fix: fence trigger publication during purge * fix: defer remote trigger deletion fences * fix: close durable delivery cleanup races * fix: drain CLI generation owners before deletion |
||
|
Danny Avila
|
f8f118ef29
|
🛰️ feat: Execute Generic Agent Trigger Deliveries (#14921)
* feat: add generic agent trigger dispatch seam * refactor: harden trigger dispatch contract * fix: annotate envelope depth alias * style: sort trigger envelope imports * fix: reject unknown trigger dispatch modes * fix: reject unknown trigger envelope versions * refactor: validate complete trigger envelopes * feat: add agent trigger execution host * fix: enforce trigger delivery contracts * fix: harden trigger admission path * fix: finish trigger cancellation handling * fix: retry strict steer rollout gaps * fix: retry paused trigger steers * fix: parallelize trigger admission setup |
||
|
Danny Avila
|
57ea1137f6
|
🛡️ feat: Let Admins Restrict Stateful Workspace Scopes (#14910)
* feat: let admins restrict stateful workspace scopes * fix: enforce stateful scope policy across agent paths * fix: close stateful scope policy activation gaps |
||
|
Danny Avila
|
7d850c308a
|
🧠 feat: Add Live Reasoning Labels (#14893)
* feat: add live reasoning labels * fix: Stabilize reasoning label checks * fix: Address reasoning label review findings * chore: Bump Agents SDK for reasoning labels * fix: Reset reused reasoning step evidence * fix: Reconcile cleared reasoning labels * fix: Fence reasoning label resets * fix: Reset reasoning ownership before gap labels * fix: Preserve THINK type through label reset * test: Expect run-global reasoning revision |
||
|
Danny Avila
|
fb8ae881cf
|
⏱️ feat: Show Run-Step Durations On Tool Cards (#14892)
* ⏱️ feat: Show Run-Step Durations On Tool Cards Surfaces how long each tool call took, derived from the `closed_at` / `created_at` pair already carried by `on_run_step_closed` — the same event #14871 and #14873 use for the terminal status. No new event, no new SDK surface. The duration is stamped onto the content part at the same three sites as `runStepStatus`, so it survives a reload and a resumable reconnect rather than living only on the live React message: - `callbacks.js`, on the aggregated part before the event is forwarded - `RedisJobStore`, in the host-authored replay reconstruction branch - `useStepHandler`, on the live message Rendering lands in the shared `ProgressText`, which nine tool cards already use, rather than in each card: one place decides whether a duration is shown and how it reads, and the cards only forward the number. That keeps this from adding a tenth independent state derivation to a component family whose label/announcement/progress split is already the subject of AI-1810. The value is deliberately absent rather than zero whenever it would be a guess — no `created_at`, non-finite input, or a negative elapsed time from two clocks that disagree, which is now reachable because a step can be opened in one process and closed in another after a checkpoint resume. Sub-second durations are suppressed as noise, and it renders only on a settled, non-error card, where the slot is not already carrying the cancelled icon or the error suffix. For assistive technology the compact form (`3.5s`) is hidden and paired with a spoken equivalent ("took 3.5 seconds"), both inside the button, so the accessible name carries the duration without an `aria-live` region re-announcing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🎨 style: Sort Imports In Touched Files The import-sort gate runs against the files a PR changes, so pre-existing drift in `ProgressText.tsx` and `RedisJobStore.ts` surfaced on this branch. Both were already unsorted on `dev`; this is the sorter's output, with no semantic change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🐛 fix: Accept Partial Timestamps In Run-Step Duration Helper `getReportableRunStepDurationMs` declared its parameter as `Pick<RunStepClosedEvent, 'created_at' | 'closed_at'>`, where `closed_at` is required. That contradicted the function's own purpose: every guard inside it exists precisely to handle stamps that may be missing. The Redis replay branch reconstructs closures from persisted JSON and holds nothing stronger than "might be a number", so it failed to typecheck against the narrower signature. Widened to an exported `RunStepTimestamps` shape with both stamps optional, rather than asserting at the call site — an assertion would move the decision about what is trustworthy somewhere it cannot be enforced, which is the thing the helper exists to centralize. Callers holding a fully-typed event still pass, since a required field satisfies an optional one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🐛 fix: Suppress Duration When Failure Arrives As errorSuffix Alone At every call site `error` carries cancellation while failure travels through `errorSuffix` with `error` false, so gating the duration on `!error` alone rendered "· 3.5s" beside "· failed" — and announced it. The gate now checks both terminal-failure channels. The original test pinned only the `error: true` path, which is why this survived; the failed-via-suffix path is now pinned separately, both the visible and the announced half. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🧩 refactor: Persist Raw Run-Step Durations, Threshold At Render Only The three stamp sites filtered through the 1-second reportability threshold before persisting, baking a presentation rule into stored data: a 900ms step stored nothing, making "fast" indistinguishable from "not derivable" and unrecoverable if the display rule ever changes. Stamp sites now persist the raw `getRunStepDurationMs` value — absent only when genuinely not derivable — and the renderer alone decides what is worth showing, which `ProgressText` already did. Rendering is unchanged. `getReportableRunStepDurationMs` is removed; it existed only to serve the write-time filter, and a test now pins that sub-threshold durations survive to storage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🐛 fix: Suppress Duration On Backgrounded Bash And Code Cards A backgrounded call's run step closes when dispatch returns the handle, so the stamped duration is the dispatch time. Rendering it beside "Running/Finished in background" misstated a detached task's runtime as seconds — and violated the "settled card only" rule, since the card is still tracking the detached run. Scope is exactly the two cards that parse background handles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🌍 fix: Format The Sub-10s Decimal For The Active Locale The fractional seconds value was interpolated as a raw JS number, which hardcodes the en-US decimal point into every language — "1.4s" where the locale writes "1,4 s" — and translators cannot fix a number formatted in code. The value is now formatted via Intl.NumberFormat with i18n.language, following MessageTimestamp's pattern of threading the language into the util; plural-key selection stays on the numeric value. A malformed language tag falls back to the plain number. Also documents the two accepted limits of the derivation, so they read as decisions rather than oversights: positive clock skew is undetectable from a single stamp pair, and the value is wall-clock elapsed, so a step held open across a suspension (checkpoint resume, HITL approval wait) includes that time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🐛 fix: Persist A Durable `backgrounded` Marker Through Harvest; Localize Minute Digits Codex round 3, both findings confirmed. **Background origin survived only as transient state.** The dispatch handle in `tool_call.output` and the live status-marker attachment are both gone once the harvester patches the settled task's stdout over the handle — so the round-2 suppression (`backgroundHandle == null`) came back on after harvest or reload, showing dispatch time as the task's runtime. Following the same rule as e4bd15d (persist facts, decide at render): the harvest patch now stamps `backgrounded: true` onto the tool call in the same atomic write that erases the handle — on the heal path too, which re-applies over full-row saves that reverted the part. The cards gate on handle-or-marker; the dispatch duration itself stays stored. **Minute-branch digits bypassed locale formatting.** The seconds branch went through Intl.NumberFormat while minutes interpolated raw numbers, so Arabic/Persian locales flipped to ASCII digits above one minute. All interpolated values now flow through the (renamed) formatDurationValue; an ar-EG test pins the localized digits. data-schemas cannot be installed in this environment (same npm ci 403 as packages/api), so message.ts/harvest.ts are syntax-checked with resolution off and otherwise verified by review; CI runs their real typecheck and suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🧪 test: Assert The `markBackgrounded` Stamp In Harvest Expectations The successful-harvest test's exact `toHaveBeenCalledWith` object did not include the newly forwarded `markBackgrounded`, so the API suite would fail on it. All three harvest-call expectations now assert `markBackgrounded: true` — the exact-object one of necessity, the two `objectContaining` ones deliberately, since the durable stamp (on the best-effort file-failure path and the reapply heal alike) is now part of the behavior under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🎨 style: Wrap Harvest Spec Expectation Per Prettier Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Danny Avila
|
06bf324cf0
|
🛤️ feat: Per-Agent Code Execution Routing With Stateful Session Scopes (#14848)
* feat: route code execution per agent profile * chore: sort execution profile imports * test: preserve stateful environment literal types * fix: isolate stateful code environments by user * fix: preserve per-agent code routing end to end * fix: route code priming by execution profile * fix: isolate code profile lifecycle state * fix: preserve mixed-profile code resources * fix: complete stateful skill routing |
||
|
Danny Avila
|
eb3b353712
|
📡 fix: Publish App-Level MCP Tool Catalogs Without a Reserved Revision (#14858)
* 📡 fix: Publish App-Level MCP Tool Catalogs Without a Reserved Revision Shared MCP servers advertised no tools to agents, so every turn failed with "configured to use MCP tools, but none are available" (#14857). `replaceAppServerTools` returned false whenever a publication carried no `publicationRevision`, but only `refreshChangedTools` reserves one. Every other app-level publisher — the first-connect snapshot, reinitialization, on-demand catalog reads, the retained-catalog restore — was silently dropped. The agent path fails closed on that drop: the skipped write returns null, so reinitialize yields no tools and the turn 503s. Startup hid it. `connectAppServers()` defers the initial refresh and calls `refreshToolList()` itself, which does reserve, so a boot that reaches its MCP servers looks healthy. Only a lazily created app connection — the server not yet up when LibreChat boots, a dropped connection, a cold cache — takes the unreserved path. `ConnectionsRepository` now reserves before its own `tools/list`, matching the list_changed path; a failed reservation publishes unordered rather than failing the connection. Publishers with no pre-fetch reservation point have already fetched by the time they reach the cache, so they take the next revision at write time instead of being discarded. `mergeAppTools` still publishes at revision 0 and stays deferential to a live catalog. * 📡 fix: Bind App Catalog Ordering to the Fetch That Produced It Addresses review feedback on the previous commit: allocating a revision at publish time lets a slow `tools/list` of an old catalog outrank a newer one that reserved after it started, and it would let the retained-catalog restore — which republishes deliberately pre-mutation data — outrank a live catalog. Ordering now travels with the data. `fetchToolsSnapshot` reserves before its first page and returns the ticket on the snapshot, so every app-level publisher reads the revision belonging to the read it is publishing rather than one allocated at an unrelated moment. `fetchOrderedToolsSnapshot` carries the refresh's revision when it defers to one, since that is whose catalog it returns. With the reservation at the single point where app-level tools are read, no publisher can forget it, so `replaceAppServerTools` goes back to refusing an unordered write: a publication that lost its ticket fetched at an unknown time and cannot be ordered. A failed reservation is reported as `orderingUnavailable` rather than swallowed, which keeps the list_changed path retrying instead of publishing a catalog that would be silently dropped, and leaves inspection unaffected by a transient cache outage. `MCPServerInspector.getToolFunctions` becomes `getToolCatalog` and returns the revision with the tools, so there is no variant that quietly discards ordering. * 📡 fix: Retry an Empty App Catalog That Could Not Reserve Ordering Review follow-up. The no-tools-capability branch destructured the reservation result and dropped `orderingUnavailable`, publishing without a revision when the revision store was transiently unavailable. That write is rejected in silence, and unlike the snapshot branch this one returned without reaching `refreshToolList()`, so whatever the server last advertised stayed in place until the connection was recreated or the cache expired. Both branches now route an unreservable catalog through the same retry path. * 📡 fix: Serve Tools Whose Shared Catalog Write Could Not Be Ordered Review follow-up. Only the shared catalog write needs ordering; the tools themselves were just read from the server and are correct to serve. Discarding them because the write could not be ordered is what turns a cache failure into a server that appears to have no tools at all, which is the reported symptom. `updateMCPServerTools` now returns the tools it built when the publication has no reserved revision, instead of null. A superseded write still discards — there another replica holds something newer. Reinitialization also asks the connection to republish under backoff when its snapshot could not reserve ordering, so the shared catalog does not stay cold until something else triggers a refresh. * 📡 fix: Surface a Discarded App Catalog Instead of Debug-Logging It #14857 went a release without a diagnostic because the only trace of a dropped app-level catalog was a debug line no deployment runs. Operators saw agents fail every turn with nothing in the logs to explain it, and the reporter had to read the source to find the cause. A publication discarded because it cannot be addressed or ordered means this server's tools are unavailable to every agent that selected them, and serving an unpublished catalog means every request re-fetches it. Both are warnings now. A superseded write stays at debug: concurrent replicas produce it routinely and the winner already holds newer tools. Tests pin the level, so a later refactor cannot quietly make the failure silent again. * 🧪 test: Pin the Reinitialize Path's Catalog Ordering Reinitialization is the path an agent falls back to when the shared catalog is cold, so it is where #14857 surfaced as "configured to use MCP tools, but none are available". Nothing pinned that it forwards the ordering its snapshot was fetched with, nor that it asks the connection to republish a catalog it could not order. Both assertions fail against the pre-fix source. |
||
Anubhav Anand
|
a2ad0aa0c8
|
🤐 feat: Allow Promptless Sends When Files Are Attached (#13717)
* ✨ feat: Allow sending file attachments without a text message When an agent asks the user to upload a document, the user could attach the file but still had to type a placeholder message ("OK", "Here is the file") before the send button enabled and the submit guard let the message through. Attachments now count as submittable content: - New isSubmittableMessage(text, fileCount) util: non-whitespace text OR at least one attached file. - ask() in useChatFunctions uses it instead of bailing on empty text, so an empty draft with attached files submits. - SendButton receives the attached file count and enables accordingly. - ChatForm only marks the text field as required when no files are attached, so react-hook-form validation no longer blocks handleSubmit. Submitting an empty draft with no attachments is still rejected at all three layers. Fixes #13646 * Address review: support replayed file-only turns + drop empty vision text - ask(): count replayed attachments (overrideFiles) in the submittable check and skip it entirely for regenerate, so a file-only message can be regenerated or saved-and-resubmitted instead of being rejected as empty. - formatVisionMessage(): omit the text content part when the message text is empty. Anthropic rejects empty text content blocks with HTTP 400, and an empty block adds nothing for other providers; image-only sends now format cleanly. Added formatMessages tests for with-text and image-only (Anthropic + other) cases. * Address review: keep attachment-only turns valid for providers, answer mode, and titles - formatMessage: substitute minimal text when a user turn carries files but no inline content, so Anthropic does not reject an empty user message for RAG or code-environment attachments. - assistants chatV1: send the same stand-in for attachment-only Threads messages, which reject an empty body. The persisted message keeps empty text. - ChatForm: attachments no longer make an empty draft submittable in answer mode, where submitText consumes the click without answering or sending. - agents request: seed title generation from attachment filenames when the turn has no text, so immediate-mode titles are not invented from an empty string. - useChatFunctions.regenerate.spec: mock the utils barrel over the real module so new exports resolve. * Cover the agents path for attachment-only turns AgentClient formats its payload with the SDK's formatMessage, not the local one, so the earlier guard missed the endpoint the feature actually targets: an attachment-only turn still reached Anthropic as an empty user message. Apply the same stand-in after the file-context and quote merges, so a turn that already gained inline content is untouched. * Carry filenames on freshly attached files The fresh-file submission mapping copied only file_id, filepath, type, and dimensions, so the attachment-only title fallback read an undefined filename and produced nothing. Include filename, and cover it with a test that submits an empty draft with one attachment. * Address review: cover assistants v2, fresh agent attachments, editor, and title fallback - agents client: the current turn has no files during buildMessages, so read the resolved attachments from message_file_map instead. The previous guard only ever fired for persisted historical turns. - assistants chatV2: the default assistants endpoint routes here, so it needs the same stand-in body chatV1 got. - assistants title: fall back to filenames, then the response, and keep the default title rather than saving an empty one. - EditMessage: retained attachments make an empty edit submittable, matching the composer, so the overrideFiles replay path is reachable from the UI. --------- Co-authored-by: Marco Beretta <81851188+berry-13@users.noreply.github.com> |
||
|
Danny Avila
|
88747f0ad8
|
🩺 fix: Render Stopped Run Steps From Explicit Status (#14871)
* 🩺 fix: Render Stopped Run Steps From Explicit Status Tool calls decided "still running" vs "stopped" with a whole-message heuristic: const cancelled = !isSubmitting && progress < 1 && !hasError; That inference cannot tell which step actually stopped. An aborted step keeps spinning while `isSubmitting` is still true, and when submitting ends, every unfinished part flips to "Cancelled" at once regardless of which one died. `@librechat/agents` v3.4.6+ emits `on_run_step_closed`, a terminal per-step signal carrying `status` and timestamps — including for steps swept at end-of-run because the caller aborted. The pinned 3.5.1 already ships it; nothing consumed it. - `StepEvents.ON_RUN_STEP_CLOSED` plus `RunStepClosedEvent` / `RunStepStatus` types mirroring the SDK payload. - `PartMetadata.runStepStatus` — a dedicated field, since `status` is already claimed by activity-label and question-form parts. - Server handler forwards the event without the visibility gating the other step handlers apply: a step whose open reached the client must get its close, or the client is left inferring again. - `useStepHandler` writes the terminal status onto the tool call part. - Both decision points (`ToolCall`, the shared `useToolCallState`) prefer explicit status, keeping the heuristic as fallback for messages saved before this and endpoints that do not emit the event. Threaded through the five cards sharing `useToolCallState`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🩹 fix: Address Codex Review On Run Step Closure Rendering - Persist the terminal status server-side. The handler emitted the closure without folding it into `contentParts`, so the status existed only on the live React message: a reload or resumable reconnect dropped it and fell back to the very heuristic this fixes. Now stamped onto the aggregated tool-call part (via `stepMap`, falling back to the event's own index) before forwarding. - Honor terminal status independently of output parsing. Gating on `hasError` meant a `failed` step with unparseable output rendered as "cancelled", while a `failed`/`cancelled` step whose output did parse as an error was not terminal at all and shimmered indefinitely when no completion event arrived. A closed step now forces progress complete and reports `failed` as an error state on its own authority. - Pass the status to the second `BashCall` branch, which rendered the same updated component without it. - Reuse `Agents.RunStepClosedStatus` in `PartMetadata` instead of redeclaring the union, so a future SDK status cannot diverge between the event and the persisted part. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * ♿ fix: Replay Closed Status On Redis Resume, Announce Failures - Apply closure events during Redis reconstruction. The stamp added in the previous commit mutates only the originating process's in-memory `contentParts`; a resumable reconnect landing on another replica rebuilds from `RedisJobStore.getContentParts`, whose allowlist omits `on_run_step_closed`. The status was therefore absent from the sync snapshot and, being snapshot-covered, never redelivered as pending — so multi-replica resume fell back to the whole-message heuristic. Handled as a host-authored event alongside `on_steer_applied` and `on_activity_label`, since the SDK aggregator has no notion of it. - Announce terminal failures in the live region. Forcing terminal progress for a closed step meant a `failed` tool reached the `aria-live` region through `getFinishedText()`, which only special- cased cancellation and otherwise announced "completed function" — telling screen-reader users the opposite of what the card showed. A regression introduced by the previous commit; error states now announce failure before any completion string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 * 🎯 fix: Resolve Closed Steps By ID, Never By Index The steer and HITL offset wrappers clone and shift only `ON_RUN_STEP` and `ON_AGENT_UPDATE`; every other event passes through untouched. A stored `on_run_step_closed` therefore carries the SDK's unshifted index, while the part it belongs to was rebuilt at the shifted one. Any run containing a steer insertion or HITL resume would stamp the status onto an earlier tool card, or none — leaving the real card on the fallback heuristic while mislabeling a different one. - Redis reconstruction builds a step ID -> index map from the replayed `on_run_step` payloads (which carry the shifted index) and resolves closures against it, mirroring what the live callback does via `stepMap`. - The live handler drops its `?? data.index` fallback for the same reason. Skipping is the safe failure: a missing status degrades to the old heuristic, whereas a misplaced one actively mislabels the wrong card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014vLhxCFMYkCaTsoFTiAjJ5 --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Ravi Kumar L
|
bc6392d05b
|
🪢 fix(langfuse): mark provider-backed agent traces (#14833)
* fix(langfuse): mark provider-backed agent traces * fix(langfuse): mark stored response traces * test(langfuse): isolate provider marker setup |
||
|
Danny Avila
|
abc669ab58
|
🩹 fix: Restore the @librechat/api Build and Remove Legacy Code (#14808)
* 🧹 chore: Remove Dead Legacy Agent Controller `_LegacyAgentController` has been unreachable since the resumable path became the only route: it is unreferenced, unexported, and untested. It had also drifted out of compilability against the live file — line 2009 called `attachConversationCreatedAt(req, { userId, conversationId, isNewConvo })` against the 3-argument signature declared at line 97, which would await `undefined` and then throw dereferencing `resolved.createdAt`. Keeping it was not free. It carried a third independent copy of the response message-id wiring (`getReqData`, `onStart`, four `updateMetadata` calls), so every change to how a generation identifies its response row had a dead third site to keep in step, and no test to say whether it had been kept in step. Removing the block leaves `createCloseHandler` and the `sendEvent`, `clientRegistry`, `requestDataMap` and `handleAbortError` imports with no remaining callers, so those go too. `AgentController` was a three-line passthrough to `ResumableAgentController`; the real controller is now exported directly, which also matches the `[ResumableAgentController]` prefix every log line in the file already uses. `server/routes/agents/chat.js` binds the export to its own local name and passes the same five arguments, so the route is unchanged. No behavior change: 379 lines removed, 2 added. * 🩹 fix: Remove Duplicated Anchor Block Breaking the `@librechat/api` Build `dev` does not build. `packages/api/src/agents/activityPhases/runtime.ts` carries two byte-identical 98-line copies of the same block (former lines 516-613 and 614-711), so rolldown fails to parse it: [PARSE_ERROR] Identifier `AnchorFields` has already been declared The duplicated block is the anchor-construction work from #14805: `AnchorFields`, `laterDefinedIndex`, `foldedAgentIds`, `boundedAnchor` and `mergeAnchors`. #14807 was squashed from a branch that predated #14805 and re-included that commit, so both copies landed. Only the `type` produced an error — the four function declarations simply redeclare. This removes the first copy. The two blocks were verified byte-identical before the cut, and the resulting file has no duplicate top-level declarations, is missing nothing that #14805 introduced, and retains everything new to #14807 (`ResolvedPosition`, `resolvePosition`). Verified: `tsdown` builds, `tsc --noEmit` clean, `config/circular-deps.mjs` green across all five graphs (it was reporting `✗ @librechat/api` purely because the build it shells out to was failing), and the 68 tests in `activityPhases/runtime.spec.ts` pass. Carried here rather than in a separate PR because this PR's checks cannot go green until it lands: the failed `packages/api` build cascades into e2e, MCP list_changed, bombadil and the Docker image jobs. |
||
|
Danny Avila
|
df6e15a0de
|
🔖 feat: Bound Parent Activity Phases With an Exclusive End Index (#14768)
* 🧭 fix: Finalize Parent Activity Phases at Run Completion * 🧭 fix: Preserve Activity Phase Boundaries * 🎨 fix: Format Activity Phase Boundary Check * 🧭 fix: Ignore Late Label Artifacts at Phase Completion * 🧭 fix: Preserve Logical Activity Phase Membership * 🩹 fix: Narrow Optional Activity Phase Marker * fix activity phase tail boundaries * fix activity phase test lint * fix straddling activity phase batches * preserve activity phase boundaries at scale * fix persisted activity phase final boundary * fix resumed activity phase edge cases * fix sparse activity phase grouping * fix sparse activity phase tail scan * fix resumed activity phase text fallback * fix sparse activity phase completion scans * avoid sparse activity phase runtime scans * stabilize sparse activity phase resumes * support activity phases on current ts target * preserve sparse phase reservations * finalize activity phase boundary handling * avoid sparse phase start scans * fix activity phase final text bounds * tighten activity phase summary boundaries * format activity phase boundary checks * leave final commentary outside activity phases * recognize lane-tagged final activity text * rebase retained activity boundaries on resume * bound activity phase collection work * correct resumed phase activity count * resolve late reasoning before phase completion * preserve lane-tagged final answers * assert durable activity phase bounds in e2e * preserve empty finalized activity phases * ignore empty reasoning at phase completion * format phase completion guard * fix(api): retain overflow reasoning anchors * perf(api): index overflow reasoning anchors * perf(api): skip empty reasoning index scans * fix(api): reconcile completion boundaries efficiently |
||
|
Danny Avila
|
1a3e2aebcb
|
🛰️ fix: Attach Request-Scoped MCP Servers (#14780)
* fix: attach request-scoped MCP servers * fix: satisfy MCP static checks * fix: format MCP runtime hint |
||
James Todaro
|
e696b07619
|
🧾 fix: Honor Disabled Transactions on the Token-Count Fallback Path (#14774)
`AgentClient.recordTokenUsage` had no `transactions` parameter, so the setting never reached `createTransaction`, whose guard reads it from the object it is handed. `transactions?.enabled === false` saw `undefined` and the write went ahead. This path is reached only from `BaseClient`'s fallback branch, when the provider returns no usable stream usage, so the bulk path masked it wherever usage is reported. Where it is not, the setting had no effect at all. |
||
|
Danny Avila
|
dccef82254
|
🪶 chore: Aggregate Empty MCP Tool Logs (#14767)
* fix: aggregate empty MCP tool logs * fix: retain server names in MCP tool logs |
||
|
Danny Avila
|
ee8c0abe2d
|
🪝 feat: Execute Agent Plugin Command Hooks (#14755)
* 🪝 feat: Execute Agent Plugin Command Hooks Implement the missing PluginHookExecutor boundary so deployment plugins' ai.librechat/hooks/hooks.json documents execute instead of loading inert: - Command executor runs handlers as child processes outside the API process: Claude-shaped JSON payload on stdin, exit 0 + JSON stdout as sanitized hook output, exit 2 blocks with stderr as the reason, minimal allowlisted environment plus PLUGIN_ROOT/PLUGIN_DATA, abort-signal kill - Plugin loading carries the parsed hooks document on the contribution and threads hookCapabilities from startup, gated on the operator opt-in DEPLOYMENT_PLUGIN_HOOKS (off by default: parsed-but-inert with warning) - Runs register every ready plugin hook onto the per-run HookRegistry after internal policy hooks, with once-per-conversation SessionStart dedup Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Harden Plugin Hook Execution Boundary Address CI and Codex/Copilot review findings on #14755: - Break the agents -> plugins import cycle: the run seam now reads a PluginHookSource wired at startup (mirrors the tool-approval registry) - Tighten plugin ask decisions to deny unless the run has HITL wiring, so an un-resumable interrupt can never strand OpenAI-compatible callers - Scope cross-run dedup keys by authenticated user and handler identity: caller-supplied conversation ids cannot collide across principals, and sibling SessionStart handlers all fire; once handlers persist across runs - Replace a literal NUL byte in source with an escape (file diffed binary) - Kill the whole detached process group on abort, not just the shell - Map exit 2 on events without a decision channel to preventContinuation - Reserve PLUGIN_ROOT/PLUGIN_DATA against allowlist overrides, quote PowerShell args, cap captured output by bytes with one-pass decoding, and serialize payloads inside the executor's error boundary - Fix import ordering flagged by the static checks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Close Plugin Hook Policy and Namespace Gaps Address the second Codex review round on #14755: - Drop updatedInput from plugin command outputs: hooks in one dispatch all receive the original arguments, so a plugin rewrite would reach the tool without the approval policy re-evaluating it (host-only now) - Translate Claude tool aliases (Bash/Write/Edit/Read) to LibreChat runtime names in matchers, with reverse payload mapping, so Claude-authored guards fire instead of planning ready and never matching - Key once-only state by declaration position as well as handler contents, so sibling declarations with identical handlers stay independent - Thread sessionStartSource through createRun and mark the HITL resume rebuild as 'resume', so SessionStart matchers see the real lifecycle Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Translate Regex-Form Claude Tool Aliases Address the third Codex review round on #14755: alias translation now substitutes word-bounded tokens, covering regex matchers like ^Bash$ and ^(Write|Edit)$ that the exact-token pass left registered against Claude names and silently never firing. A regex whose alias sits inside a character class or escape is rejected as unmapped so it fails loudly at plan time instead of never running. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Alias Translation and Reuse Load-Time Plans Address the fourth Codex review round on #14755: - Add the WebSearch -> web_search alias so Claude-authored web-search guards fire against the LibreChat built-in - Apply alias translation only to tool-name events; a StopFailure matcher like ^Bash failed$ stays untouched and keeps matching the error text - Reuse each plugin's load-time hook plan at run registration instead of re-planning up to 512 handlers on every chat turn Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Translate Aliased Tool Inputs and Harden Hook Domains - Present aliased tool inputs under Claude field names (file_path, old_string, new_string, including nested edits), so Write/Edit/Read guards see the fields they check instead of silently allowing - Derive the alias table from canonical tool-name definitions (BashExecutionToolDefinition, CREATE_FILE_TOOL_NAME, Tools.web_search) instead of a parallel hand-authored table - Reject matchers naming Claude built-ins with no runtime equivalent (Task, Glob, Grep, WebFetch, ...) as unmapped at plan time instead of registering guards that never fire - Replace per-event Sets and Stop special-cases with an exhaustive EVENT_TRAITS record over HookEvent, so new engine events demand explicit semantics at compile time - Move cross-run once-state behind a PluginHookOnceStore seam with a least-recently-marked memory default: active conversations refresh their keys each turn, so capacity eviction can no longer re-fire a conversation that is still in use; the seam admits a shared-cache store for multi-replica deployments - Gate portable-only command handlers at plan time on Windows via a new supportsHandler capability (commandWindows or shell powershell required) instead of spawning bash that cannot exist - Kill Windows hook process trees with taskkill /t on abort - Require declaration indices on execution requests, stamped from the plan instead of defaulted at execution time Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Keep Group SIGKILL Escalation Armed After Wrapper Exit An aborted hook whose descendant ignores SIGTERM could leak that descendant: the wrapper shell's exit fired close, which cancelled the scheduled group SIGKILL. The escalation timer is now never cancelled — it is unref'd and killTree already tolerates a vanished process group, so a redundant late sweep is harmless while a surviving descendant is reliably killed at the grace deadline. killGraceMs is configurable on CommandExecutorOptions, with a regression test driving a trap-protected descendant past the wrapper's exit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Once Retention by Conversation and Reject Clear Source - Restructure the once store around conversation scopes: registration touches the scope every run, so rarely-matching once handlers keep their keys while the conversation is active; eviction removes whole idle conversations (capacity counts conversations, not keys) - Reject SessionStart matchers naming the clear lifecycle source at plan time — no LibreChat run-construction path emits clear, so the handler would plan ready and never fire; wildcard warning text now reflects the sources that actually occur - Make the SIGKILL-escalation regression test real: the surviving descendant redirects its stdio away from the captured pipes so the wrapper's close fires while it is still alive, exercising the window a close-time cancellation would leak Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Bound Alias Tokens by Tool-Name Characters and Host Shells - Translate Claude aliases (and reject unsupported built-ins) only when delimited by characters that cannot appear in a runtime tool name: action tool names preserve hyphens, so an alias embedded in a longer name like deploy-Bash-v2_action_example_com stays the literal tool name instead of being rewritten into a matcher that never fires - Reject PowerShell-only command handlers on POSIX hosts at plan time (and skip them at runtime): bash cannot run PowerShell syntax, so the guard would fail open; a handler with both variants still runs its portable command - Handle rejected asynchronous once-store calls: a failed touch logs instead of raising an unhandled rejection during run construction, and a failed markOnce lookup fails open per the store's documented over-fire direction Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Probe Group Liveness Before Cancelled or Delivered SIGKILL The never-cancelled escalation timer could signal a recycled process-group id when an aborted hook's whole tree exits early in the grace window. Escalation now probes the group with signal 0: close cancels the timer only when the group is verifiably empty, and the deadline re-probes before delivering the group SIGKILL, so surviving descendants are still reaped while a fully-dead group never receives a blind late signal. The residual probe-to-signal race is documented as irreducible without pidfd support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Gate Windows Escalation on Root-Process Liveness Windows taskkill /t walks the tree from the root process, so once Node observes the root's exit an escalation pass can reap nothing and a late forced taskkill could only hit a recycled PID. The liveness gate is now platform-aware in one helper: POSIX probes the process group with signal 0, Windows checks the root's observed exit state, and both the close-time cancellation and the deadline delivery consult it — no platform retains a blind late signal. Orphaned SIGTERM-ignoring descendants on Windows are documented as the platform limitation they are without Job Objects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Payload Namespace to Declarations and Reap Stray Workers - Reverse name/input translation now applies only to declarations whose matcher actually required Claude-alias translation: the plan records requiresToolNameTranslation per entry, so a native-authored matcher like ^create_file$ receives native tool names and fields instead of Claude-shaped payloads its guard never expected - Coordinate the two dedup layers via a shouldExecute gate on the executor: a declaration suppressed by spent once-state declines before claiming the per-input dedup slot, so an identical handler under an overlapping matcher can still claim it and fire its own independent once-key instead of being permanently shadowed - Reap process groups that outlive a successful hook: a backgrounded worker left running after normal wrapper exit gets the same term-then-escalate sequence an abort uses, since unsupported async handlers mean no lifecycle owns such processes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🧰 chore: Vendor Pocock Codebase-Design and Architecture Skills Adds mattpocock/skills engineering/codebase-design and engineering/improve-codebase-architecture (MIT, license included) under .claude/skills so future sessions share the deep-module vocabulary (module, interface, depth, seam, adapter, leverage, locality) and the architecture-review process. Force-added past the /.claude/ gitignore deliberately; relocate if project skills should live elsewhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 refactor: Extract Process-Tree Reaping Into a Reaper Module Tree lifecycle — five of the last seven review findings — lived as event-handler wiring inside runCommand with its invariants in comments. It now sits behind a two-method seam: createReaper(child, graceMs) exposes reap() and onClose(), hiding the term-grace-escalate state machine, the per-platform liveness gates, the recycled-id guards, and the clean-exit sweep. The executor shrinks to capture-and-parse, and the reaper is unit-tested directly with real process trees through its own interface instead of only via whole-executor integration runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Scope Translation Per Alternative and Sweep at Root Exit - Track which runtime tool names alias translation produced, so a mixed-namespace matcher like Bash|create_file presents Claude-shaped payloads only for bash_tool invocations while the natively-authored create_file alternative keeps native names and fields; a capability omitting the produced-names list keeps declaration-wide translation - Sweep the process tree at root exit as well as close: a backgrounded descendant holding the captured pipes delays close until it dies, so the exit-time sweep terminates it promptly instead of stalling the hook until its timeout aborts - Pass the primary agent's resolved model and identity into the plugin hook context, so SessionStart payloads carry model and agent_type instead of always omitting them Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Default Wildcard Declarations to the Document Namespace - Matcherless (or wildcard) tool-payload declarations now inherit the hook document's Claude namespace: with no alternatives to carry namespace evidence, the plan marks them for declaration-wide reverse translation, so a wildcard guard inspecting standard Claude names and fields sees Write/file_path instead of silently failing open on native payloads; PostToolBatch entries translate the same way - Recognize aliases delimited by regex metacharacters: dots leave the tool-name boundary class (runtime names never contain them — action ids underscore domain dots), so ^Bash.*$ translates to ^bash_tool.*$ instead of registering a guard that never fires - Expand Claude's ${CLAUDE_PLUGIN_ROOT} spelling in hook commands and export it in the child environment alongside PLUGIN_ROOT - Scope SessionStart once-keys by lifecycle source, so a startup firing no longer suppresses the conversation's resume rebuild Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Normalize Claude Structured Hook Output Stock Claude hooks return decisions under hookSpecificOutput (permissionDecision/permissionDecisionReason), surface context there, and use continue:false plus the legacy approve/block decisions — none of which the sanitizer's native field names recognized, so a guard that works in Claude silently allowed in LibreChat. Parsed JSON now passes through a dialect normalizer first: hookSpecificOutput fields map to decision/reason/additionalContext, continue:false becomes preventContinuation, approve becomes allow, and block becomes deny on events that block by denying. Native fields win when both dialects appear, and the ask-to-deny gate applies to the Claude dialect too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Validate Native Decisions and Slim Once Keys - Strip malformed native output fields before the dialect merge, so a placeholder like {"decision":null} can no longer suppress a valid Claude permissionDecision into a silent allow; only recognized decision tokens take precedence - Preserve the caller's working directory in hook payloads: cwd now reports the run's session context instead of the plugin installation path, which commands already receive as PLUGIN_ROOT and which the executor still uses as each process's working directory - Store a compact sha256 digest instead of the full serialized handler in once keys: declarations may carry 32 KB commands and 256 args, and the previous key embedded them in every retained conversation scope Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc * 🪝 fix: Validate Decisions Per Event Channel and Control Post-Tool Blocks - Accept native decision tokens only from the target event's own vocabulary: "continue" is valid on Stop but malformed on a tool event, where it previously survived validation, blocked the Claude dialect merge, and was then dropped by sanitization into a silent allow - Translate a structured "block" on events with no deny channel (PostToolUse, PostToolUseFailure, and the other prevent-trait events) into preventContinuation with the block reason as stopReason, instead of discarding it and returning a reason that controls nothing - Document why LibreChat runs supply no payload cwd: tool paths address a remote code-execution sandbox rather than the API host where hook commands run, so no host directory describes the run Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWXQZD2WzeAsvee4eRdWWc --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Danny Avila
|
88e08c91e8
|
🧷 fix: Preserve Elicitation Answers Across Aborts (#14745)
* fix: preserve elicitation answers across aborts * chore: sort stream type imports * fix: guard malformed resolved answers * fix: close abort answer race gaps * fix: retain exact answers across pauses * fix: retain answers across resumed pauses * test: satisfy HITL fixture types * fix: retain legacy answers through approvals * fix: preserve answers in reconnect snapshots * test: type legacy answer fixture * fix: bind legacy answers to paused content * test: guard optional resume content * fix: resume questions without streamed content * fix: Preserve legacy answers through abort filtering * test: Narrow reconstructed abort fixture * test: Type abort transform fixture explicitly * fix: Isolate answers with missing ask content * test: Type missing-content ask fixture |
||
|
Danny Avila
|
236ee6c1ab
|
🧭 fix: Re-Anchor Parent Activity Phase Bounds (#14741)
* test: cover parent activity phase finalization * test(e2e): stabilize parent phase coverage * fix(agents): reanchor parent activity phase bounds * fix(agents): preserve delayed tools in activity phases * test(agents): keep phase slice bounds typed * fix(agents): preserve sparse activity phase bounds * test(e2e): read structured phase replies |
||
|
Danny Avila
|
7347cfc195
|
🍡 feat: Batched User Questions With A Single Bounded Answer Form (#14737)
* feat: support batched user questions * test: align batched question fixtures * fix: harden batched question lifecycle * test: submit batched HITL answers in e2e * fix: address batched question review findings * fix: preserve invoke return typing |
||
|
Danny Avila
|
09cbd54f48
|
🪆 fix: Rebase Activity Phase Bounds over Sparse Content (#14729)
The aggregator writes content parts at provider-source indexes, which can skip slots and leave holes in contentParts. Array.prototype.map preserves those holes and the Map constructor iterates them as undefined, so rebaseActivityPhaseBounds threw "Iterator value undefined is not an entry object" at the end of every run with sparse content — deterministic with parent phase summaries enabled, on both the completion and resume paths. Build the identity map with an index loop that skips nullish slots. Holes must stay out of the map: one undefined key would falsely match every hole in previousParts as a retained part and corrupt the rebased bound. |
||
|
Danny Avila
|
a3cec67e08
|
🪆 feat: Add Parent Activity Phase Summaries (#14721)
* feat: add activity phase summaries * fix: preserve activity phase lifecycle semantics * fix: satisfy activity phase type checks * fix: simplify activity phase status mapping * style: format activity phase changes * fix: rebase activity phase bounds after shaping * fix: link activity phase trace ancestry * fix: reconcile activity phase bounds * style: format activity phase reconciliation test * style: align activity phase assertion * fix: retain reasoning across commentary * fix: preserve activity phase boundary state * fix: detect renderable phase children * test: type parallel phase assertion * chore: bump agents SDK for activity phases * fix: retain unphased lane reasoning * fix: preserve tool group expansion across phases * style: format phase expansion regression * fix: preserve phase interaction state efficiently * perf: skip sparse phase segment holes * perf: partition phase segments with offsets * fix: preserve phase boundaries and cursor state * test: align activity phase regressions with CI * test: keep phase context mock hoist-safe |
||
|
Danny Avila
|
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> |
||
Marco Beretta
|
39f5f9d846
|
⚡ perf: Agent List and Model Selector at Scale (#14601)
* perf: cut serial round trips from the agent list query path
The agent list was the slowest path on first page load. Three separate
problems compounded:
- `getListAgentsHandler` chained its reads: two ACL lookups, the avatar
refresh cache probe and the viewer skill scope all resolved serially
ahead of the list query, and `attachOwnerContacts` added two more hops
after it. The four independent reads now resolve together, and the
avatar refresh runs alongside the list query instead of before it -
refreshed paths reach the response through `urlCache`, not through
whatever the list query happened to read. Serial hops per request drop
from 7 to 4 on a warm cache.
- The avatar refresh loaded the user's whole accessible agent set (up to
MAX_AVATAR_REFRESH_AGENTS) to discover which entries were S3-backed.
Scoping the query to `avatar.source` means deployments on any other
file strategy match nothing instead of walking the full set.
- `fetchAllAgentPages` walked cursor pages at the server's default size
of 100, and callers consume the flattened result, so every extra page
was a serial round trip for no benefit. It now requests the server
maximum. Measured over a 2,860 agent account: 29 requests / 1.65s
before, 3 requests / 0.29s after.
Also parallelizes the conversation file reads in `initializeAgent`. The
convo file refs and the execute_code thread walk share no inputs, and the
two code-file lookups depend only on `threadFileIds`, so the chain of six
serial reads on every turn collapses to two. This one is time to first
token the user waits through.
* perf: virtualize the model selector agent list
Opening the agents submenu with a large agent set froze the tab and could
kill it outright. With ~10k accessible agents the submenu blocked for over
15 seconds and took the heap from 96MB to 911MB. Four per-row costs were
being multiplied by the full list, which rendered unwindowed:
- `useIsActiveItem` allocated a MutationObserver per row (10,016 of them
for one dropdown). Replaced with an Ariakit store subscription, which
needs no observer at all and returns a boolean so a row only re-renders
when its own active state flips.
- `useFavorites` ran per row, opening a jotai subscription, a query
subscription and a mutation each time. Hoisted to one call per endpoint.
- Each row rescanned `endpoint.models` to recover `isGlobal`, a field the
parent had already discarded from the array it was mapping. The parent
now passes it down from a lookup map.
- The list itself is now windowed above 100 rows. Ariakit's composite only
knows about mounted rows, so arrow-keying to the window edge previously
found no next item and let focus escape the nested menu, closing it;
`handleBoundaryNavigation` scrolls the next index in, waits for it to
mount, then moves the composite onto it. Navigation inside the window is
left to Ariakit.
Open drops from >15s to 96ms, mounted rows from 10,028 to ~18, DOM nodes
from 123,346 to ~1,000, and the heap no longer grows. Verified in browser:
arrow keys track 1:1 to index 238 and back, and click selection works.
* perf: serve the model selector from the shared VIEW agent query
The model selector asked for EDIT-scoped agents whenever the marketplace
is enabled, while `useAgentsMap` and `useMentions` asked for VIEW. Since
the cache key includes the params, that was two distinct entries, so first
page load ran the paginated walk twice and held two copies of the whole
agent list in memory. Measured against a 10k agent account: 22 list handler
invocations per page load, now 11.
Collapsing the two by asking for the same permission everywhere would have
changed what the selector shows - under the marketplace the EDIT scope is
what makes it "My Agents", with discovery handled by the marketplace entry.
So the list endpoint now marks each row with `isEditable`, resolved from an
ACL read folded into the existing parallel batch (no extra serial hop), and
the selector filters the shared VIEW response instead of refetching. A
VIEW-scoped list for a user with 2861 visible / 361 editable agents returns
exactly 360 rows flagged editable, matching what the EDIT query returned.
`AgentSelect` deliberately keeps its own EDIT query: it reads `skills` and
`skills_enabled`, which `sanitizeViewerSkillScope` strips from VIEW-scoped
responses. It also only mounts when the builder panel is open, so it is not
part of the first-load cost.
The field is set unconditionally rather than omitted when false so that a
client talking to an older server sees `undefined`, keeps every agent, and
degrades to showing too many rather than none.
* fix: address review findings on the agent list at scale
Three issues from review, all confirmed against the code before fixing.
Avatar refresh no longer runs alongside the list query. `updateAgent` writes
through `findOneAndUpdate` on a `timestamps: true` schema, so refreshing an
avatar advances `updatedAt` — the field `getListAgentsByAccess` sorts and
cursors on. A write landing after the first page's snapshot moved that agent
ahead of the returned cursor, dropping it from every later page and silently
truncating the caller's flattened list. This was a regression introduced when
the two were parallelized; serializing them costs nothing on the common path,
because a cache hit returns without issuing any query, so only the
once-per-30-minutes miss pays for the ordering. The new test asserts the write
lands before the list snapshot and fails against the parallel version.
The virtualized list no longer inserts a focusable grid into the combobox.
`List` spreads its props onto `Grid`, whose defaults are `role="grid"`,
`containerRole="row"` and `tabIndex={0}`; inside Ariakit's listbox that added a
tab stop ahead of any row and put grid/row semantics between the listbox and its
options. All three are now neutralized so focus and ARIA stay with the combobox
items.
The list also resets to the top when the filter changes. `Grid` keeps its scroll
offset across prop changes and clamps an out-of-range offset to
`totalRowsHeight - height`, the end of the shorter list. Scrolling deep and then
searching landed on the tail: measured at row 626 of 667 matches, with only
those rows mounted and reachable by keyboard. Keying the list on the search
value restores row 0.
* fix: declare option position and set size for the virtualized model list
Once the model list is windowed, only the mounted slice exists in the listbox,
so a screen reader infers position and total from ~19 elements instead of the
real set — announcing "3 of 19" partway through 10,014 agents.
Model rows now carry aria-posinset and aria-setsize. The marketplace entry and
any model specs share the same numbering, because they are options in the same
listbox: declaring the values on some options while leaving others to be
inferred from the DOM would make the set internally inconsistent. Both are
omitted entirely when the list is short enough to render unwindowed, where the
DOM holds every option and the implicit values are already correct.
Verified against a 10,014 agent account: the marketplace entry reports 1 of
10015, the first models 2 and 3, and after scrolling to row 4999 the leading
mounted model reports 5001 of 10015 with 19 options in the DOM.
* 🩹 fix: Address Follow-Ups on the Agent List at Scale
Corrects residual issues in the agent-list perf work, all inside its own scope.
- Forward `idOnTheSource` through `PermissionService.findAccessibleResources`
so `getUserPrincipals` skips the user-document read. The list handler resolves
three permission sets per request and each was paying its own `User.findById`;
the auth strategies already normalize the field to a value or null.
- Gate the editable-set lookup on its own predicate instead of borrowing
`canReturnSkillConfig`. The two answer unrelated questions and only coincide
today, so redefining the skill flag would have marked every agent editable.
- Log mapping failures in the list response instead of swallowing them.
- Apply the walk page size after the caller's params in `fetchAllAgentPages`.
A caller limit only changed page size, never what the flattened walk returned,
so `defaultAgentParams`' `limit: 10` would have turned one request into 301.
- Carry `isEditable` on the agent rows the create and update mutations write
into the list cache. Mutation responses omit the field, so those rows lost it.
- Document `isEditable` as list-only, ACL-derived, and fail-open on absence.
- Restore the truthiness guard on the thread walk in `initializeAgent`. Widening
it to `!= null` made an empty `parentMessageId` issue a full-conversation read
against an anchor that can never match.
- Await `getConvoFiles` directly rather than calling `.then()` on it, restoring
tolerance for synchronous test doubles.
- Correct the avatar-refresh comment: the projection was never full documents,
and the real reason to filter is that an unfiltered budget is self-reinforcing.
Tests: both new `initialize` tests and both new backend tests are
mutation-verified; the concurrency test fails under either serialization order.
* fix: preserve ACL isEditable when merging agent mutation responses
Mutation responses omit list-only isEditable. Inferring true from write
success promoted VIEW-only rows into the editable subset for MANAGE_AGENTS
callers who can PATCH agents their ACL marks non-editable.
* fix: sort imports in agent mutations test
ESLint import-order check failed on the isEditable cache-preservation test.
* 🧷 fix: Carry isEditable Onto Duplicated Agent List Rows
`useDuplicateAgentMutation` prepended the raw duplicate response to the cached
list, and mutation responses omit the list-only `isEditable` field. The row
survived the "My Agents" filter only by failing open on `undefined`, so it would
disappear the moment a consumer read the flag strictly.
Duplicating grants the caller ownership, so the new row is editable outright;
this is the create case rather than the merge case `mergeAgentListRow` handles.
Last cache write on this path that did not carry the field.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
Danny Avila
|
5ff46d8c67
|
🛟 fix: Stop Agents When Code Resources Cannot Recover (#14651)
* fix: Block Agents When Code Resources Cannot Recover * fix: Preserve Resource Recovery Failures Across Agent Paths * fix: Centralize Fatal Agent Initialization * chore: sort agent imports |
||
|
Dustin Healy
|
6d2f29266c
|
🔑 feat: Refresh-Capable Google Admin OAuth Sessions (#13832)
* 🔑 feat: Refresh-Capable Google Admin OAuth Sessions Google admin sessions cannot be refreshed today. Three gaps add up to that: passport.authenticate('googleAdmin', ...) in api/server/routes/admin/auth.js never sets access_type=offline, so Google omits the refresh_token from its token response; createOAuthHandler in api/server/controllers/auth/oauth.js only forwards a refresh token into the admin exchange payload when the user's provider is 'openid' AND OPENID_REUSE_TOKENS is enabled; and /api/admin/oauth/refresh is openid-only, calling openid-client.refreshTokenGrant against the configured OIDC issuer. OpenID admins refresh transparently because all three are in place for them. This PR closes all three. The googleAdmin authenticate call now passes accessType: 'offline' and prompt: 'consent' so Google issues a refresh token on consent; the chat-side googleLogin is untouched. The shared socialLogin verify callback now passes the IdP refreshToken through as passport's third argument (info), landing on req.authInfo, with the two-argument call shape preserved when no refresh token is present so existing strategy tests stay valid. createOAuthHandler reads req.authInfo?.refreshToken for non-OpenID admin providers and forwards it into the exchange code; the OpenID branch and its OPENID_REUSE_TOKENS gate are unchanged. /api/admin/oauth/refresh now accepts an optional provider field ('openid' | 'google', default 'openid'). The new Google branch POSTs grant_type=refresh_token to https://oauth2.googleapis.com/token, decodes the returned id_token for the sub claim, looks up the admin user by googleId, enforces tenant scope and ACCESS_ADMIN, and mints a fresh LibreChat JWT in the same response shape /oauth/exchange returns. It is gated on GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET being set (returns 503 GOOGLE_NOT_CONFIGURED otherwise); unknown provider values return 400 INVALID_PROVIDER. * 🔁 fix: Harden Google admin refresh against bot review findings Five validated findings from the initial bot pass: socialLogin.js: mirror the OpenID migrate-or-reject pattern on the email fallback. When an existing user is found by email and the stored provider id is empty, persist the refreshed sub so the refresh path can later bind to it. When the stored id is present and differs, reject as AUTH_FAILED to prevent identity-swap, matching the existing OpenID behavior in packages/api/src/auth/openid.ts. oauth.js: scope the non-OpenID admin refresh-token forwarding to provider === 'google'. The previous else branch would have forwarded a Discord refresh token (passport-discord supplies one) into the admin exchange payload even though /api/admin/oauth/refresh only accepts openid or google, leaving the admin client with a token it could not refresh. admin/auth.js (refreshGoogleAdminSession): drop id_token from the mandatory-fields check. Google's OAuth refresh response is documented to include id_token only conditionally, so the previous mandatory check broke refresh whenever Google omitted it. Decode id_token when present (fast path); when absent, call Google's userinfo endpoint with the access token to read sub. Wrap tokenResponse.json() in try/catch and return IDP_INCOMPLETE on parse failure instead of a generic 500. Tighten access_token to a typeof string check. admin/auth.js (refreshGoogleAdminSession): reuse serializeUserForExchange for the response user so the Google refresh shape matches /oauth/exchange and the OpenID branch exactly (full _id, id, email, name, username, role, avatar, provider, openidId). The previous Google-specific subset dropped fields the admin client relies on for later provider-specific refreshes and disambiguation. Tests cover each fix: socialLogin's migration and rejection cases, the oauth.js Discord-gating case, the userinfo fallback path on missing id_token, CLAIMS_INCOMPLETE when both id_token and userinfo are absent, IDP_INCOMPLETE on a non-JSON token body, and the full response shape on the happy path. * 🧪 fix: Add updateUser to appleStrategy test mock for socialLogin migration The shared socialLogin verify callback now invokes `updateUser` when the email-fallback path discovers a same-provider user with an empty provider id, persisting the refreshed sub. The Apple strategy test's `~/models` mock did not stub `updateUser`, so the migration path hit `TypeError: updateUser is not a function` and failed the `should handle existing user and update avatarUrl` case in CI shard 1/3. * 🧹 refactor: Move Google admin refresh into TypeScript @librechat/api helper Per repo guidance (CLAUDE.md): all new backend code must be TypeScript in /packages/api, and /api is a thin JS wrapper. The previous commit landed the Google admin refresh flow as ~120 lines of new JS inside api/server/routes/admin/auth.js, which violates that. This commit extracts the flow into a new TS helper at packages/api/src/auth/googleRefresh.ts and reduces the route handler to a thin dep-wiring wrapper. The helper exports applyGoogleAdminRefresh(deps, options) with the same shape as the OpenID applyAdminRefresh: callers pass findUsers, getUserById, canAccessAdmin, and mintToken as deps so the package stays free of /api model imports and capability/session helpers. The route handler now builds those deps from the existing model + capability + token modules and calls the helper, mapping AdminRefreshError to the documented HTTP responses. While moving the code, the helper now guards getUserById with Types.ObjectId.isValid before the direct-lookup branch, matching the OpenID admin path at packages/api/src/auth/refresh.ts. Without this guard a malformed user_id from the admin client would hit Mongoose findById's CastError and surface as a 500 INTERNAL_ERROR instead of falling through to the documented sub-based lookup. Tests move with the code: packages/api/src/auth/googleRefresh.spec.ts now owns the helper's behavior (token endpoint, userinfo fallback, ObjectId guard, USER_ID_MISMATCH/TENANT_MISMATCH/USER_NOT_FOUND/FORBIDDEN, rotated refresh-token pass-through, GOOGLE_NOT_CONFIGURED, IDP_INCOMPLETE on non-JSON body, CLAIMS_INCOMPLETE when both id_token and userinfo miss). The route-level api/server/routes/admin/auth.refresh.test.js drops the duplicated end-to-end Google cases and keeps a smaller surface: route delegates to applyGoogleAdminRefresh with the right deps + options, maps AdminRefreshError to HTTP status/code, falls through to 500 for unknown errors, and rejects unknown providers with INVALID_PROVIDER. * 🔁 fix: Tighten Google admin refresh and limit social-login changes Brutal-review findings on top of the upstream feature work. socialLogin.js: the migrate-or-reject pattern from the previous commit applied to every provider's chat-side verify callback, not just the admin flow. Gate both branches on `options.existingUsersOnly` so the chat-side googleLogin / facebookLogin / etc. keep their pre-existing email-fallback behavior unchanged. Tests follow: restore the original `should fallback to finding user by email` chat-side case and re-add the migration and mismatch-reject cases as admin-only by passing `{ existingUsersOnly: true }` to socialLogin in those tests. googleRefresh.ts: add a defense-in-depth `isEmailAllowed(user)` dep that the helper invokes before `canAccessAdmin`. Mirrors the `isEmailDomainAllowed` check the initial Google admin login already runs, so a deployment that removes a domain from `registration.allowedDomains` after issuance can no longer mint fresh JWTs for that admin via refresh. The route handler wires it up with `resolveAppConfigForUser` + `isEmailDomainAllowed`, falling back to `baseOnly` config for users without a tenantId. googleRefresh.ts: drop the unreachable `?? ''` defensive coalescing in `fetchGoogleTokenset`. The `GOOGLE_NOT_CONFIGURED` guard upstream already narrows `clientId`/`clientSecret` to non-empty strings; the function takes a narrowed `GoogleAdminRefreshConfiguredOptions` shape and `applyGoogleAdminRefresh` constructs that shape after the guard. * 🔒 fix: Apply brutal-review hardening to Google admin refresh Tighten the Google OAuth refresh flow against all outstanding code review findings: enforce JWT aud claim verification against the configured clientId (ISSUER_MISMATCH on mismatch), reject ambiguous googleId matches (limit:2 in findUsers, USER_ID_MISMATCH when multiple rows match), scope the authInfo refresh-token carrier to the Google provider only, add TOCTOU re-read defense after the admin googleId migration write in socialLogin, deduplicate canAccessAdmin/mintToken closures via buildAdminRefreshClosures shared by both OpenID and Google refresh paths, document rotation semantics on AdminExchangeResponse.refreshToken, standardise all log prefixes to [admin/oauth/refresh], and expand test coverage for all new paths. * 🔒 fix: Reject refresh for users migrated off the Google provider The interactive Google admin login path in socialLogin.js already rejects a user whose provider field is not 'google', returning AUTH_FAILED. Without a matching guard in the refresh path, a user migrated to OpenID could use an unexpired Google refresh token to keep minting admin JWTs indefinitely. Add a PROVIDER_MISMATCH check after resolving the user in both the direct getUserById branch and the findUsers fallback branch of resolveAdminUser, mirroring the provider gate the interactive path enforces. * 🔒 fix: Add ban check and fix domain allowlist on admin OAuth refresh Two gaps in the /api/admin/oauth/refresh route: Add middleware.checkBan to the route chain before preAuthTenantMiddleware, matching the gate that /login/local and createOAuthHandler already apply. Without it a banned admin could keep minting JWTs until their IdP refresh token expired. Replace getAppConfig({ baseOnly: true }) in the non-tenant isEmailAllowed closure with getAppConfig({ role: user.role }), which includes DB-layer overrides from the admin panel. baseOnly returns only YAML-derived config, so any allowedDomains list maintained entirely through the admin panel was silently inert on this path. Extract isEmailAllowedForUser as a shared helper, move it into buildAdminRefreshClosures so both Google and OpenID refresh paths enforce domain policy consistently, and add isEmailAllowed to AdminRefreshDeps in the TS package so applyAdminRefresh can invoke it. * 🔒 fix: Harden admin OAuth refresh against user bans, tenant scope gaps, and cross-tenant migration Post-identity-resolution ban check: the initial checkBan middleware fires before the refresh token is exchanged and req.user is populated, so it can only evaluate IP bans. After applyGoogleAdminRefresh/applyAdminRefresh resolves the user identity, we now synthesize req.user and re-run checkBan against the resolved user's id before emitting the JWT, so a user-level ban is enforced even from a fresh IP. Domain allowlist now includes userId: the getAppConfig call in isEmailAllowedForUser was passing only role, missing user and group-level allowedDomains overrides that the initial OAuth callback's checkDomainAllowed enforces via userId. Both branches now pass userId so buildPrincipals takes the full user+group+role resolution path. The tenant branch is also inlined (replacing resolveAppConfigForUser) to accept userId, wrapped in tenantStorage.run for correct Mongoose scoping and cache-key resolution. Cross-tenant email-fallback migration: the Passport verify callback fires before tenantContextMiddleware, so findUser({email}) is unscoped and can return a same-email user from another tenant. Writing googleId onto that document permanently corrupts the other tenant's account. Migration is now blocked for users with a tenantId; single-tenant users are unaffected. --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Danny Avila
|
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 |
||
|
Danny Avila
|
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 |
||
Dustin Healy
|
3f0a1ec8d9
|
🛡️ fix: Run message-filter PII patterns on a linear-time regex engine (ReDoS) (#14554)
* 🛡️ fix: Run message-filter PII patterns on a linear-time regex engine The messageFilter.pii middleware compiled admin-configured customPatterns with the native RegExp engine and ran them synchronously against every message on the shared event loop, so a catastrophic-backtracking pattern such as (a+)+$ could stall the entire process (native RegExp takes tens of seconds at roughly 32 characters) and take the instance down for every user. Compile these patterns with RE2JS, a linear-time RE2 port with no native addon, so catastrophic backtracking is impossible regardless of the pattern rather than something the code tries to detect. Patterns using features RE2 does not support, such as backreferences, fail to compile and are dropped and logged exactly as an invalid pattern already is. The filter only tests for a match, so this is a drop-in engine swap with no behavior change for valid patterns. * 🛡️ fix: Reject RE2-incompatible messageFilter patterns at config load The customPatterns regex was validated with native RegExp at config load, but the runtime now compiles it with a linear-time engine (RE2) that does not support backreferences or lookaround. Such a pattern passed validation, then failed to compile and was silently dropped at request time, quietly removing PII protection after upgrade. Reject backreferences and lookaround during config validation with an explicit message, and document RE2 syntax in the example config instead of "JavaScript-flavor". The runtime engine remains the authoritative boundary and still drops-and-logs anything this load-time check misses. * 🧹 test: Use direct MessageFilterPiiConfig annotations in the PII specs The added ReDoS cases satisfy the exported MessageFilterPiiConfig type directly, so the `as unknown as` assertions were unnecessary. Annotate the config objects directly, matching the repo's type-safety guidance. * 🧹 fix: Reject named backreferences in messageFilter patterns at config load Extend the config-load check to also reject named backreferences (\k<name>), which are valid JavaScript regex but unsupported by the linear-time runtime engine, so they surface at load rather than being dropped at request time. Together with the existing numeric-backreference and lookaround checks this covers the RE2-incompatible construct set; the runtime engine remains authoritative. * 🛡️ fix: Preserve Unicode whitespace matching in messageFilter starter patterns RE2's \s is ASCII-only, so after the engine swap the built-in api-key and Bearer starters no longer matched a secret separated by non-ASCII whitespace (e.g. a non-breaking space), which native RegExp did match. Broaden the whitespace classes to [\s\p{Zs}] so those patterns keep their original coverage, and add a regression test for a non-breaking-space separator. * 🛡️ fix: Validate messageFilter patterns with the RE2 engine at config load Replace the syntax blacklist (numeric/named backreferences, lookaround) with authoritative validation: config load now compiles each custom pattern with the same linear-time engine the runtime uses, so any RE2-incompatible construct (including control escapes like \cA) is rejected at load with a clear error instead of being silently dropped at request time. The validator is swappable and defaults to native RegExp so browser builds add no engine; the server wires the RE2-backed check at startup via configureMessageFilterRegexValidator in both entry points. * 🛡️ fix: Match the full whitespace set in messageFilter starter patterns RE2's `\s` omits the vertical tab and `\p{Zs}` omits U+2028, U+2029, and U+FEFF, so a separator built from one of those characters slipped past the `api-key` and `Bearer` starter patterns and reached the model. Broaden the starter whitespace class to the full JavaScript whitespace set so those separators are covered again. * fix: fail closed when messageFilter.pii compiles to zero patterns DB and admin config overrides bypass the RE2 schema validation (it only runs at YAML load), so an override whose only pattern is RE2-incompatible was dropped at compile time, left zero patterns, and let the request through. compile() now returns a failClosed flag when a config declared patterns but every one failed to compile; the middleware returns 400 and findPiiMatchInMessages returns a distinct misconfigured match that the OpenAI and Responses controllers surface with an admin-facing message. * 🛡️ fix: Fail closed when any messageFilter.pii custom pattern drops compile() previously set failClosed only when every pattern dropped (patterns.length === 0 && dropped > 0). With the default starters present, a single RE2-incompatible custom override incremented dropped but left patterns.length > 0, so the filter silently enforced only the surviving subset and text matching only the dropped rule passed. failClosed now keys off dropped > 0, so any dropped custom pattern blocks with the misconfigured 400. YAML patterns are RE2-validated at load, so dropped stays 0 for valid configs and only unvalidated DB or admin overrides can trip it. Reframed the two keeps-others-active specs to assert fail-closed and added a default-starters partial-drop regression. * 🧹 fix: Correct the misconfigured JSDoc and drop redundant casts in the PII specs The misconfigured flag now means any configured custom pattern failed to compile, not that every pattern failed, so its JSDoc on PiiMatch is updated to match. The partial-drop regressions now use direct MessageFilterPiiConfig annotations instead of as-unknown-as casts, keeping the specs type-checked, consistent with the rest of the suite. |
||
|
Danny Avila
|
b11978017d
|
🧱 fix: Enforce Agent Runtime File Trust Boundaries (#14577)
* fix: secure agent runtime file metadata * chore: sort agent resource test imports * fix: Align Agent Tool Resource Types * fix: Rehydrate Agent Image Resources * fix: preserve remote agent file authorization |