From ac2aef00f6ebed74cde89b51d28e77da5db6c97b Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 24 Aug 2026 22:29:13 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=AB=97=20fix:=20Drain=20Quoted=20Excerpts?= =?UTF-8?q?=20Into=20Mid-Run=20Steering=20(#15175)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ๐Ÿงญ fix: Carry Quoted Excerpts Through Mid-Run Steering "Add to chat" quote chips were dropped by every during-run steer path: the steer POST had no quotes concept, so a composer-origin steer left the chip staged (gluing onto the NEXT send) and a queued item steered into the live run lost its quotes silently. Quotes now ride the steer protocol end to end: - POST + admission: `quotes` on the steer body, normalized like the chat route's (getReferencedQuotes caps), part of the idempotency fingerprint only when present so pre-existing receipts still replay. - Injection: merged into the model-bound turn as Markdown blockquotes at both boundaries (text-only and media paths), mirroring prependQuotes. - Persistence + replay: the STEER content part stores `quotes` separately from the typed text; stampSteerPartMedia re-merges them per turn (even with resendFiles off) via the SDK's transient media stamp, with the quote block folded into the token budget. - UI: composer steers/interrupt-steers drain the chips (skill picks stay staged โ€” they configure a NEW turn's run); SteerPart and the in-flight bubble render the same MessageQuotes reference blocks as user bubbles; queued/failed rows show a quote count; reconnect reseeds fall back to the server item's quotes when no local chip survives. - buildMessages keeps its zero-await path to the parallel context kickoff via a synchronous stamp-target probe. * ๐Ÿงญ fix: Keep Quotes in the Client-Safe Steer Projection toPendingSteer is the projection behind resume-state pendingSteers, abort responses, and terminal leftover claims โ€” dropping quotes there would lose them on exactly the recovery paths the reconnect reseed's server fallback relies on. * ๐Ÿงช test: In-Flight Steer Bubble Renders Carried Quotes * ๐Ÿ” fix: Re-Stage Quotes When a Pre-Quotes Replica Accepts the Steer Codex flagged the rolling-deploy window: an old replica 202s a quoted steer while dropping the excerpts, so the client cleared the chips for context the model never received. The 202 (fresh and receipt replay) now echoes quotesAccepted from the DURABLE item; a missing echo on a quote-bearing composer-origin steer re-stages the excerpts as composer chips โ€” the pre-steer behavior, so they ride the next send instead of vanishing โ€” and strips them from the surviving chip so a later terminal conversion cannot duplicate them. Queued-origin steers keep quotes on the item, whose restore paths already return it intact. The residual cross-version lost-ACK retry stays fail-closed as a 409 idempotency conflict (failed chip with retry controls). * ๐Ÿ” fix: Close the Remaining Cross-Version Quote-Loss Windows Codex round 2: - Send now of a quoted queued item against a pre-quotes replica now re-stages the excerpts too (the row is consumed and the words inject bare, so the composer is their only remaining home); the strip clears the chip's captured origin copy so reclaims and terminal conversions cannot duplicate them. - A quoted retry whose lost first ACK was accepted by a pre-quotes replica now REPLAYS that legacy receipt instead of 409ing: the stored fingerprint matching the quote-less hash of the same words proves the cross-version case, and the replayed 202's missing echo drives the re-stage. Different quotes against a quote-bearing receipt still conflict. - TSteerAppliedEvent.part gains the quotes field (typed SSE consumers). * ๐Ÿงช test: Drop the Stale Narrow SteerDrainOutput Alias The spec's local intersection re-declared injectedMessages with content: string, predating the SDK pin that declares the field natively (content: string | MessageContentComplex[]). Under CI's clean install the hook's BaseHookOutput is no longer assignable to that narrower alias; the plain PostToolBatchHookOutput is the correct type for every drain/boundary assertion. Verified against the published 3.6.16 dist and the local one. * ๐Ÿ” fix: Honor the Generation Owner's Quote Capability End to End Codex round 4: - steerQuotesCapable rides job metadata (createJob + HITL resume rewrite), mirroring preemptCapable's owner-recorded pattern: an upgraded admission replica no longer stores quotes โ€” or claims them accepted โ€” for a generation whose older owning drain would silently drop them at injection. The missing echo drives the client re-stage, and a later capable handover cannot double-deliver restored context. - Applied events reconcile dropped quotes: when a quote-less applied part settles a quote-bearing chip (the lost-202 ordering the ACK-echo path cannot see), resolveSteerChip and both reconnect settle paths re-stage the chip's excerpts before removing their only copy. mergeRestagedQuotes dedupe keeps every trigger idempotent for the same excerpts. * ๐Ÿ” fix: Re-Read Quote Capability at the Last Moment and Cap Restaged Chips Codex round 5: - A HITL resume rewrites steerQuotesCapable without changing the generation's createdAt, so the enqueue fence cannot see a capable-to-legacy handover landing during admission's awaits. Re-read the owner's flag immediately before item construction (paid for only by quote-bearing requests); the residual between re-read and enqueue commit matches preemptCapable's documented race. - mergeRestagedQuotes now respects the 10-quote contract with the staged chips winning: a restored tail that cannot ride the next send is dropped explicitly instead of rendering as a chip the submission would silently discard. MAX_QUOTE_COUNT moves to utils/steer as the single client source; QuoteButton imports it. * ๐Ÿ” fix: Steer Quote Coverage for Preflights, Memory, and Single-Scan Stamping Codex round 6: - Stored-message policy inspection now extracts steer-part quotes as quote fragments (path /content/N/quotes/M), so conversation import and shared link preflights inspect the newly persisted field exactly like top-level message.quotes. - The memory copy gets its own quote-merge stamp (text only, resendFiles false): formatAgentMessages ignores part.quotes, so without it a steer whose substance lives in its excerpt reached the chat model but never memory extraction. - collectSteerStampTargets replaces the boolean probe: buildMessages collects once and hands the targets to stampSteerPartMedia, keeping the zero-await fast path without scanning the history twice. * ๐Ÿ” fix: Redis Quote Plumbing, Conversion-Race Guard, and Quote-Bound Recovery Proof Codex round 7: - RedisJobStore.deserializeJob now restores steerQuotesCapable (the explicit mapper otherwise dropped it on every read, leaving quote steering inert in Redis deployments), with the round-trip spec extended. - Both Lua parked-steer projections (terminal close + generation replacement) forward item.quotes, matching toPendingSteer โ€” a lost final no longer strips excerpts from durable recovery in Redis mode. - The no-echo restage reads the SURVIVING chip (reclaimRejectedChipQuotes): a terminal conversion that beat the delayed 202 already moved the quotes onto the queued follow-up, and re-staging them again double-delivered. Regression-tested with the conversion-before-ACK ordering. - RecoveredSteerPayload binds normalized, order-significant quotes (builder, validator, TS matcher, and the Lua decode+matcher): a stale client presenting the same recoverySteerId with altered or missing quotes cannot consume the parked source. Quote-less sources keep matching quote-less recoveries. * ๐Ÿ” fix: Execution-Bound Quote Capability with an Atomic Enqueue Predicate Codex round 8: - steerQuotesCapable becomes a transient assertion translated (at createJob and in ApprovalLifecycle.resolve) into steerQuotesExecutionId, valid only while it equals the LIVE providerExecutionId. A legacy replica winning a HITL resume rewrites the execution id without knowing the marker, so its stale assertion self-invalidates โ€” a bare boolean could not be cleared by code that predates it. - The fenced enqueue evaluates that equality atomically (all three Redis scripts decode-and-strip like the existing preemptCapable normalization; both InMemory sites mirror it) and returns the persisted item, so the quotesAccepted echo reflects exactly what was stored even when a handover lands between admission's read and the commit. The last-moment re-read is gone โ€” the transaction is the authority. - Tests: capable-resume re-binding, legacy-resume omit-not-clear invalidation, the admission-vs-handover race (capability read true, then execution rewritten before enqueue), and the Redis round-trip of the marker. * ๐Ÿ” fix: Full Redis Parking Coverage and Loss-Moment Quote Restaging Codex round 9: - The two remaining Redis parking projections (terminal status CAS and stale-running cleanup) forward item.quotes โ€” every field-picked steer projection now carries them (audited: 2 Lua 'projected' + 2 Lua 'clientItem' + toPendingSteer). - The ordinary no-echo ACK no longer re-stages: the steer has not injected yet, so the quotes stay carried on the pending chip. A quote-less applied event re-stages them at the actual loss; a terminal leftover conversion carries them onto the recovered row, whose normal send delivers quotes on any server โ€” re-staging at the ACK let that leftover auto-send bare text while the excerpts glued onto an unrelated draft. Only the settled receipt replay (already injected, no future event) reclaims immediately. * ๐Ÿ” fix: Legacy-Replayable Receipts with Separate Quote Identity Codex round 10: an upgraded-first receipt stored a quote-inclusive fingerprint no pre-quotes replica could recompute, so a lost-ACK retry routed through one 409'd already-accepted words with duplicate-send controls. The durable fingerprint reverts to the quote-independent 3-field hash โ€” the one shape EVERY deployed version computes, replayable across a rolling deploy in both directions โ€” and quote identity moves beside it as requestedQuotesFingerprint (of the REQUESTED quotes, pre any capability strip, so an incapable-owner acceptance still replays its own retries). Absent records (legacy-written or quote-less) accept any same-words retry, preserving the round-5 rule; present records must match exactly, keeping different-quotes clientSteerId reuse a 409 on quote-aware readers. Under the keep-on-chip client contract a legacy replay's missing echo is harmless โ€” the excerpts stay carried on the pending chip. * ๐Ÿงช chore: Re-Trigger CI After Dropped Workflow Events --- .../__tests__/request.resumeMetadata.spec.js | 1 + api/server/controllers/agents/client.js | 54 ++++- api/server/controllers/agents/client.test.js | 54 +++++ api/server/controllers/agents/request.js | 6 +- api/server/controllers/agents/resume.js | 3 + .../components/Chat/Input/InFlightSteers.tsx | 4 + .../Chat/Input/PendingQuoteChips.tsx | 8 +- .../Chat/Input/PendingSteerChips.tsx | 43 +++- .../src/components/Chat/Input/QuoteButton.tsx | 5 +- .../Input/__tests__/InFlightSteers.test.tsx | 13 + .../components/Chat/Messages/Content/Part.tsx | 1 + .../Chat/Messages/Content/Parts/SteerPart.tsx | 6 + .../Parts/__tests__/SteerPart.test.tsx | 21 ++ client/src/data-provider/SSE/mutations.ts | 8 + .../hooks/Chat/__tests__/useSteering.spec.tsx | 208 +++++++++++++++- client/src/hooks/Chat/useSteerConvert.ts | 6 +- client/src/hooks/Chat/useSteering.ts | 112 ++++++++- .../SSE/__tests__/useResumableSSE.spec.ts | 18 +- client/src/hooks/SSE/useResumableSSE.ts | 53 ++++- client/src/hooks/SSE/useResumeOnLoad.ts | 21 +- client/src/locales/en/translation.json | 1 + client/src/store/families.ts | 8 +- client/src/utils/__tests__/steer.spec.ts | 55 +++++ client/src/utils/steer.ts | 91 ++++++- .../agents/steering/__tests__/media.spec.ts | 163 ++++++++++++- .../agents/steering/__tests__/request.spec.ts | 180 ++++++++++++++ .../agents/steering/__tests__/runtime.spec.ts | 55 ++++- packages/api/src/agents/steering/index.ts | 4 +- packages/api/src/agents/steering/media.ts | 223 ++++++++++++------ packages/api/src/agents/steering/request.ts | 77 +++++- packages/api/src/agents/steering/runtime.ts | 8 +- .../protection/adapters/submissions.spec.ts | 28 +++ .../src/protection/adapters/submissions.ts | 25 ++ packages/api/src/stream/ApprovalLifecycle.ts | 14 +- .../api/src/stream/GenerationJobManager.ts | 12 +- packages/api/src/stream/SteerRecovery.ts | 23 +- packages/api/src/stream/SteeringLifecycle.ts | 1 + .../stream/__tests__/RedisJobStore.spec.ts | 2 + .../RedisJobStore.stream_integration.spec.ts | 14 +- .../stream/__tests__/protocolRollout.spec.ts | 4 +- ...protocolRollout.stream_integration.spec.ts | 4 +- .../__tests__/steerReceiptIntegrity.spec.ts | 2 + .../api/src/stream/__tests__/steering.spec.ts | 116 ++++++++- .../implementations/InMemoryJobStore.ts | 12 + .../stream/implementations/RedisJobStore.ts | 29 ++- .../api/src/stream/interfaces/IJobStore.ts | 33 +++ packages/api/src/stream/metadata.ts | 3 + packages/api/src/types/stream.ts | 4 + .../data-provider/src/types/assistants.ts | 4 + packages/data-provider/src/types/runs.ts | 6 + 50 files changed, 1686 insertions(+), 160 deletions(-) diff --git a/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js b/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js index a5c747364f..59e24d3c8d 100644 --- a/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js +++ b/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js @@ -738,6 +738,7 @@ describe('ResumableAgentController resume metadata', () => { model: 'gpt-3.5-turbo', /** The OWNING replica's seal capability, read by the steer route. */ preemptCapable: true, + steerQuotesCapable: true, agent_id: undefined, isTemporary: true, responseMessageId: expect.stringMatching(/^[0-9a-f-]{36}$/), diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 320abe0691..6c1b6a6136 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -58,6 +58,7 @@ const { isSteeringSupported, isSteerPreemptSupported, buildSteerMedia, + collectSteerStampTargets, stampSteerPartMedia, createActivityLabelWiring, createActivityPhaseWiring, @@ -387,6 +388,9 @@ class AgentClient extends BaseClient { ...(item.clientSteerId && { clientSteerId: item.clientSteerId }), createdAt: item.createdAt, ...(item.files?.length && { files: item.files }), + // Persisted separately from the text (mirroring `message.quotes`) so the + // UI renders reference blocks and replay re-merges them per turn. + ...(item.quotes?.length && { quotes: item.quotes }), }; this.contentParts.push(part); this.steerOffsetState.offset += 1; @@ -1892,22 +1896,33 @@ class AgentClient extends BaseClient { payload = formattedMessages; this.modelBoundSteerFileIdsBySourceMessageId = new Map(); - if (this.options.resendFiles) { - /** Persisted steer parts of past turns replay with their attachments: - * one batched owner-scoped fetch, re-encoded per turn and stamped as a - * transient `media` array (same resend semantics as message files). - * The stamp lands after the loop above finalized its counts, so the - * re-encoded media (minus the text part the steer part already counted) - * is folded into the budget here โ€” large steered attachments must - * shrink the window like any other resent media. */ + /** Persisted steer parts of past turns replay with their attachments and + * quotes: one batched owner-scoped fetch, re-encoded per turn and + * stamped as a transient `media` array (same resend semantics as + * message files). Runs regardless of `resendFiles` because quote-bearing + * parts must re-merge their excerpts every turn (mirroring + * `prependQuotes` above); file encoding stays gated on the setting via + * the flag. The stamp lands after the loop above finalized its counts, + * so the re-encoded media (minus the text part the steer part already + * counted) is folded into the budget here โ€” large steered attachments + * and quote blocks must shrink the window like any other resent media. + * The synchronous collection keeps steer-free histories on the + * zero-await path to the parallel context kickoff below, and the + * collected targets feed the stamp directly so the history is scanned + * once. */ + const resendSteerFiles = this.options.resendFiles === true; + const steerStampTargets = collectSteerStampTargets(payload, resendSteerFiles); + if (steerStampTargets.length > 0) { const stamped = await stampSteerPartMedia({ client: this, user: this.options.req?.user, payload, + targets: steerStampTargets, // addPreviousAttachments already fetched steer-part refs in its single // per-turn historical-files query โ€” no second round trip. docsById: this.authorizedHistoricalFiles, getFiles: db.getFiles, + resendFiles: resendSteerFiles, }); for (const { sourceMessageId, fileIds } of stamped) { if (typeof sourceMessageId !== 'string' || sourceMessageId.length === 0) { @@ -1927,8 +1942,8 @@ class AgentClient extends BaseClient { for (const { index, media, steerText } of stamped) { /** Count the FULL stamped content and subtract only the steer body * (already counted inside the assistant message): extracted file - * context prepended into the text part must hit the budget too, or - * large steered documents bypass pruning. */ + * context and merged quote blocks prepended into the text part must + * hit the budget too, or large steered documents bypass pruning. */ const fullTokens = countFormattedMessageTokens({ role: 'user', content: media }, encoding); const bodyTokens = steerText ? countFormattedMessageTokens( @@ -1949,6 +1964,25 @@ class AgentClient extends BaseClient { memoryFormattedMessages[i] ?? buildMemoryFormattedMessage(orderedMessages[i]), ); } + /** The memory copy feeds `processMemory` through the same + * `formatAgentMessages` replay, which reads `part.media`/`part.steer` + * and ignores `part.quotes` โ€” so a steer whose substance lives in its + * quote must be quote-merged here too or memory extraction never sees + * it. Quote merge only (`resendFiles: false`): file media is exactly + * what the memory copy exists to exclude, and text-only stamps touch + * no file fetch or encode. Runs after the fill above so late-built + * copies are stamped too. */ + const memorySteerTargets = collectSteerStampTargets(memoryPayload, false); + if (memorySteerTargets.length > 0) { + await stampSteerPartMedia({ + client: this, + user: this.options.req?.user, + payload: memoryPayload, + targets: memorySteerTargets, + getFiles: db.getFiles, + resendFiles: false, + }); + } } this.memoryPayload = hasFileContext ? memoryPayload : null; messages = orderedMessages; diff --git a/api/server/controllers/agents/client.test.js b/api/server/controllers/agents/client.test.js index 073daddbf0..9b8d3200e9 100644 --- a/api/server/controllers/agents/client.test.js +++ b/api/server/controllers/agents/client.test.js @@ -3749,6 +3749,60 @@ describe('AgentClient - titleConvo', () => { ); }); + it('quote-merges historical steer parts into the prompt AND the memory copy', async () => { + const previousFileContext = + 'Attached document(s):\n```md\n# "previous.txt"\nPrevious turn file body\n```'; + + const result = await client.buildMessages( + [ + { + messageId: 'msg-1', + parentMessageId: null, + sender: 'User', + text: 'Summarize.', + isCreatedByUser: true, + fileContext: previousFileContext, + }, + { + messageId: 'msg-2', + parentMessageId: 'msg-1', + sender: 'Assistant', + text: '', + isCreatedByUser: false, + content: [ + { type: ContentTypes.TEXT, text: 'working on it' }, + { + type: ContentTypes.STEER, + [ContentTypes.STEER]: 'remember this', + steerId: 's1', + quotes: ['the important fact'], + }, + ], + }, + { + messageId: 'msg-3', + parentMessageId: 'msg-2', + sender: 'User', + text: 'Continue.', + isCreatedByUser: true, + }, + ], + 'msg-3', + {}, + ); + + const merged = '> the important fact\n\nremember this'; + const promptSteer = result.prompt[1].content.find((part) => part.type === ContentTypes.STEER); + expect(promptSteer.media).toEqual([{ type: ContentTypes.TEXT, text: merged }]); + // The memory copy replays through the same formatter, which ignores + // `part.quotes` โ€” it needs its own merged stamp or memory extraction + // never sees the excerpt. + const memorySteer = client.memoryPayload[1].content.find( + (part) => part.type === ContentTypes.STEER, + ); + expect(memorySteer.media).toEqual([{ type: ContentTypes.TEXT, text: merged }]); + }); + it('persists canonical token counts while counting request file context for the prompt', async () => { const { countFormattedMessageTokens } = require('@librechat/api'); const currentFile = makeTextFile('current-file', 'current.txt', 'Current turn file body'); diff --git a/api/server/controllers/agents/request.js b/api/server/controllers/agents/request.js index 0ab98fdb0c..75a1ee744c 100644 --- a/api/server/controllers/agents/request.js +++ b/api/server/controllers/agents/request.js @@ -444,7 +444,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit const isRecoveredSteerRequest = recoveredSteerId != null; const recoveryUserMessageId = rawOverrideUserMessageId; const recoveredSteerPayload = isRecoveredSteerRequest - ? buildRecoveredSteerPayload(text, req.body?.files) + ? buildRecoveredSteerPayload(text, req.body?.files, req.body?.quotes) : undefined; /** A recovered steer is handed off as a new ordinary user turn. Edit, * regenerate, continue, and arbitrary override-id shapes can reuse an @@ -1106,6 +1106,10 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit // route may land on a different replica whose own SDK probe would // answer for the wrong process during a rolling deploy. preemptCapable: isSteerPreemptSupported(), + // Same owner-recorded pattern: this build's drain merges queued steer + // quotes into the injected turn. Admission on another replica must + // not store/acknowledge quotes an older owner would drop. + steerQuotesCapable: true, // Persist the originating agent so a HITL resume can refuse to rebuild this // paused run on a different agent (see resume.js). agent_id: endpointOption.agent_id ?? req.body?.agent_id, diff --git a/api/server/controllers/agents/resume.js b/api/server/controllers/agents/resume.js index 30e3c8c73c..5d54b6c96f 100644 --- a/api/server/controllers/agents/resume.js +++ b/api/server/controllers/agents/resume.js @@ -1222,6 +1222,9 @@ const ResumeAgentController = async (req, res, next, initializeClient, addTitle) pendingAction.actionId, { preemptCapable: isSteerPreemptSupported(), + // The handover owner's quote handling replaces the previous + // replica's flag, mirroring `preemptCapable` above. + steerQuotesCapable: true, providerExecutionId, providerDrained: true, ...(resolvedAskUserQuestion && { resolvedAskUserQuestions }), diff --git a/client/src/components/Chat/Input/InFlightSteers.tsx b/client/src/components/Chat/Input/InFlightSteers.tsx index 633e6250a9..e959b64693 100644 --- a/client/src/components/Chat/Input/InFlightSteers.tsx +++ b/client/src/components/Chat/Input/InFlightSteers.tsx @@ -16,6 +16,7 @@ import { import FilePreviewDialog from '~/components/Chat/Messages/Content/FilePreviewDialog'; import { supportsGenerationProtocolV2, useArmSteerMutation } from '~/data-provider'; import { steerOverlayHeightFamily, escalatingSteerFamily } from '~/store/steer'; +import MessageQuotes from '~/components/Chat/Messages/Content/MessageQuotes'; import MarkdownLite from '~/components/Chat/Messages/Content/MarkdownLite'; import FileContainer from '~/components/Chat/Input/Files/FileContainer'; import { useSteerCancel, useSteerReclaim, useLocalize } from '~/hooks'; @@ -482,6 +483,9 @@ const InFlightSteer = memo(function InFlightSteer({ {localize(preempting ? 'com_ui_steer_in_flight_preempt' : 'com_ui_steer_in_flight')}
+ {/* Same reference blocks the applied `SteerPart` shows, outside the + * collapse so the excerpts stay visible while a long steer clips. */} +
-