diff --git a/devlog/_plan/260814_usage_memory_roadmap/020_m0_2_continuation_dedup.md b/devlog/_plan/260814_usage_memory_roadmap/020_m0_2_continuation_dedup.md index 0d0212cf8e..6a7acda715 100644 --- a/devlog/_plan/260814_usage_memory_roadmap/020_m0_2_continuation_dedup.md +++ b/devlog/_plan/260814_usage_memory_roadmap/020_m0_2_continuation_dedup.md @@ -9,93 +9,461 @@ closes: "(split from #1412)" # 020 — M0-2: Stop compounding replayed history +## Audit history + +**P stale check.** WP0 named the wrong mechanism. Compounding is the unconditional concat at +`src/responses/state.ts:895`, not a core.ts prepend. `_providerContinuation` carries provider +scalars, not history, so the draft plan to clear it would have broken Cursor while fixing +nothing. + +**A round 1 — FAIL, 5 blockers.** All verified against the tree. + +| # | Blocker | Disposition | +|---|---------|-------------| +| 1 | Stripping the id destroys Kiro/Cursor provider continuity for that turn | Folded — keep the id, add explicit disposition | +| 2 | Zero prefix length re-acknowledges historical compaction markers and duplicates guidance | Folded — record the recognized prefix | +| 3 | Strict raw prefix match rarely fires: the proxy mutates stored input | **Thesis narrowed** — see below | +| 4 | A bounded serialization SAMPLE can collide, causing a false skip | Folded — hard cap + "not comparable" | +| 5 | A new metric field breaks two privacy-pinned tests | Folded — internal counter, not `ResponseStateMetrics` | + +The reviewer also refuted one bug I suspected in my own plan: stripping the id would NOT have +mis-set `_previousResponseInputExpanded`, because `core.ts:1521` requires both +`body !== originalBody` AND a surviving string id. Good to know, but blocker 1 removes the +strip anyway. + +**A round 3 — FAIL, 4 blockers.** Third consecutive FAIL, so under LOOP-REPAIR-01 this stops +being a patch loop and returns to P with a changed plan. The change: **adopt the anchor rule +#1412 already arrived at** (`hasStableAnchor`) instead of continuing to invent one. That draft +is unmerged and its predicate is not in the tree, but its design was validated against real +incident traffic, and three rounds of independently re-deriving it have now failed. + +| # | Blocker | Disposition | +|---|---------|-------------| +| 11 | Crossing `providerOutputStart` proves position, not identity | **Replan** — require provider-issued id on the anchor item | +| 12 | Spill persistence is a separate versioned shape; scope missed it | Folded — `spill-store.ts` + byte accounting + validation in scope | +| 13 | The thesis line still said layer 1 "bounds" the residual | Folded — stale sentence corrected | +| 14 | The 8 KiB cap was scoped to id-less items only | Folded — applies to every item | + +### Blocker 11: position is not identity + +Round 2's fix required the matched run to cross into `response.output`. That proves an item +sits on the provider side of the boundary — not that only the provider could have produced it. +An id-less assistant message satisfies the numeric anchor on content equality alone, which is +the round-2 ambiguity wearing a different hat. + +The corrected rule needs both: + +- exact bounded canonical content equality for **every** stored item, and +- at least one item at `index >= providerOutputStart` carrying a **non-empty provider-issued + `id` or `call_id`**. + +If no provider-output item supplies stable identity, expand. The reviewer notes this is what +#1412's `hasStableAnchor` does; that predicate is not in this tree +(`rg hasStableAnchor src/` → no match), so this layer implements the rule rather than +importing it. + +On the question I asked directly — whether a client legitimately echoing assistant output +defeats the anchor — the answer is no, and for a satisfying reason: a client echoing provider +output *with the provider's own id* is replaying that exact occurrence, which is precisely what +we want to detect. The hole was only ever id-less output. + +**A round 2 — FAIL, 2 High + 3 confirmations.** The round-1 fixes for B1, B2, B4, B5 were +verified correct. Two findings replace the predicate itself: + +| # | Blocker | Disposition | +|---|---------|-------------| +| 6 | Value equality does not prove two items are the SAME historical occurrence | Folded — provider-output anchor required | +| 7 | "Layer 1 bounds the residual" overstates what M0-1 does | Folded — claim corrected | +| 8 | Type/comment contracts still say "the proxy expanded" | Folded — `src/types.ts` added to scope | +| 9 | The 8 KiB cap is right but the mechanism was underspecified | Folded — iterative writer with depth limit | +| 10 | The internal counter is never reset between tests | Folded — reset added | + +### Blocker 6: equality is not identity, and the counterexample is one item long + +Stored state flattens both sides of a turn into one array: +`items: [...inputItems(request.input), ...response.output]` (`src/responses/state.ts:1047`). +Nothing marks where the client's input ends and the provider's output begins. + +The reviewer's counterexample needs only a single item. Turn 1 stores one user message +`"repeat"` with empty output. Turn 2 is a genuine delta that legitimately begins with another +`"repeat"` followed by `"new"`. The whole stored run fingerprints equal, so a +content-equality predicate skips — and silently deletes a real historical occurrence. That is +precisely the false skip this design calls the unacceptable failure. + +The fix is to require evidence only the PROVIDER could have produced. `rememberResponseState` +knows the boundary at write time (`inputItems(request.input).length`), so it records it, and a +skip requires the matched run to include at least one item from the provider-output side. +A client cannot forge that anchor by repeating itself. + +This expands scope into the stored-state shape, which round 1 had listed as out of scope. That +is the correct trade: the alternative is a predicate that can delete conversation history. + +### Blocker 7: M0-1 does not bound this, and saying so was wrong + +Round 1 justified narrowing by claiming unmatched sessions "hit a 413 instead of an OOM". +The reviewer disproved it in three steps: expansion and parsing happen at +`src/server/responses/core.ts:1510`, long before admission at `:1847`, so the concatenation +and its allocations are already done; admission fails open when no ceiling resolves +(`src/server/responses/input-admission.ts:165`); and compaction turns are exempt (`core.ts:1844`). + +The honest statement, now in the doc: M0-1 refuses known-ceiling, non-compaction requests +before upstream dispatch. It does not prevent pre-admission materialization, and it is not a +reason to defer FU-2 indefinitely. + +### Blocker 3 forces the thesis to narrow — this is the important one + +`injectDeveloperMessage` splices proxy-authored developer items directly into `rawInput` +(`src/server/responses/collaboration.ts:491-497`), and `rememberResponseState` then stores +`[...inputItems(request.input), ...response.output]` (`src/responses/state.ts:1047`) — the +MUTATED input. Separately, passthrough JSON is recorded before client-facing item-id repair +(`src/server/responses/core.ts:2753` then `:2793`). + +So stored state routinely contains items the client never saw, and ids the client saw in a +different form. A strict match on raw stored objects fails at item 0 for exactly those +sessions, and the compounding continues. + +Two honest options: + +- **Reconstruct a client-visible projection** of stored items — model proxy-only injection and + id repair, then compare. That is a second implementation of two subsystems whose output is + already hard to predict, and every future injection site silently degrades it. +- **Narrow the thesis** to what can be recognized soundly. + +This layer takes the second. The claim becomes: *detect the case where the client verbatim +replays a conversation the proxy did not mutate, and stop doubling it.* That is the +stateless-client shape — no injected guidance, no repaired ids — and it is a real population, +but it is NOT all of #1412. + +The gap is stated rather than papered over: **a session where the proxy injected guidance into +stored history is not deduplicated by this layer.** + +What M0-1 does and does not do for that gap (corrected per blocker 7): it refuses +known-ceiling, non-compaction requests before upstream dispatch. It does NOT prevent the +concatenation or the parse, which happen at `core.ts:1510` well before admission at `:1847`, +and it fails open when no ceiling resolves. So the residual is reduced, not bounded, and FU-2 +is real remaining work rather than something M0-1 has already covered. + +The reviewer's judgment on whether to keep this layer at all: keep it. #1412 documents an +observed population — stateless DeepSeek tool-result turns that send full history alongside +`previous_response_id` — where 1x reconstruction was demonstrated. That is the population +this layer fixes. + ## Thesis -When a request already contains the full conversation history (from the client -replaying its own context), the proxy must not prepend stored continuation state -on top. This duplication turns a 127k request into 254k→381k→508k across -continuations, eventually causing OOM or provider rejection. +When the client verbatim replays history the proxy stored unmodified, prepending the stored +copy doubles it, and the doubled turn is stored again, so the next turn triples. Detect that +exact case and skip the prepend. + +Every ambiguity resolves toward expanding: a wrong skip silently truncates real conversation, +while a wrong expand is merely large. Note that layer 1 does not *bound* that residual — it +refuses known-ceiling, non-compaction requests before dispatch, but expansion and parsing +already happened by then (`core.ts:1510` vs `:1847`) and it fails open on unknown ceilings. ## Current state -- `src/server/responses/core.ts:1544` sets `parsed._providerContinuation` - from `previousResponseProviderState` -- The continuation expansion code (around line 1621) checks - `hasUnexpandedPreviousResponse` but does not detect when the client has - already included the full history in the `input` array -- #1412 documents cases where 1x input becomes 2x→3x→4x through repeated - continuation prepending +- `src/responses/state.ts:895` concatenates unconditionally — the compounding site +- `src/responses/state.ts:899` records the prefix length via a WeakMap; `:376` documents it as + the provenance boundary that acknowledges historical compaction markers exactly once +- `src/responses/parser.ts:372` uses `inputIndex >= replayedInputPrefixLength` to decide a new + compaction boundary; Cursor turns that into `contextUsageReset` + (`src/adapters/cursor/request-builder.ts:318`) +- `src/server/responses/collaboration.ts:455` scans only the replay prefix for existing guidance +- `src/server/responses/collaboration.ts:491` mutates `rawInput`, and `state.ts:1047` stores it +- `ResponseStateMetrics` (`state.ts:932`) has exactly 12 fields, pinned by + `tests/memory-watchdog.test.ts:213` as a privacy review gate + +## Design decisions + +### Keep the id; skip only the concatenation (blocker 1) + +`previousResponseProviderState(parsed.previousResponseId)` (`core.ts:1544`) is how Kiro and +Cursor recover their conversation ids; without the id Kiro mints a fresh UUID +(`src/adapters/kiro-wire.ts:112`) and Cursor starts a new conversation +(`src/adapters/cursor/request-builder.ts:283`). So the request keeps `previous_response_id`, +and only the input concatenation is skipped. + +Consequence: `body !== originalBody` AND the id survives, so `core.ts:1521` would now set +`_previousResponseInputExpanded = true`. That is not a lie — the history IS present, the client +supplied it — and it is what the canonical-backend guard (`core.ts:1622`) and the Kiro guard +(`core.ts:2028`) need in order not to treat this turn as a replay miss (#702). + +To keep "is the history present" separate from "did the proxy physically prepend it", the skip +returns a NEW object (a shallow copy with identical content). Identity changes, content does +not, and the disposition is recorded on the WeakMap alongside the prefix length. + +### Record the recognized prefix length anyway (blocker 2) + +The prefix length is not "how many items the proxy inserted" — `state.ts:376` defines it as the +boundary between replayed history and newly appended input. On a skip, the client supplied the +history, but the boundary is identical and still known: it is the number of stored items matched. + +Recording it keeps two behaviors correct that would otherwise silently break: + +- a historical `context_compaction` marker inside the replayed prefix is NOT re-acknowledged + (`parser.ts:372`), so Cursor does not spuriously reset context usage +- `injectDeveloperMessage` still sees existing guidance in the prefix + (`collaboration.ts:455`) and does not inject a duplicate + +### A skip requires a provider-output anchor (blocker 6) + +Content equality proves two items look alike, not that they are the same occurrence. Since a +client can legitimately repeat itself, a run of equal items is not evidence of replay. + +`rememberResponseState` therefore records `providerOutputStart` — the index in `items` where +`response.output` begins. + +Round 3 showed that reaching that index is not enough (blocker 11): position proves an item sits +on the provider side, not that the provider authored it, and an id-less assistant message +satisfies a positional anchor on content equality alone. The final rule is all three of: + +- the matched run covers the whole stored entry, +- it reaches `providerOutputStart`, and +- some matched item at or past that index carries a **non-empty provider-issued `id` or + `call_id`**. + +There is no invariant that provider output always carries ids (`state.ts:1031` accepts arbitrary +output arrays), so when none does, the entry simply never skips. + +Entries written before this field exists have no anchor, so they never skip and expand as +before. + +### Only compare items that can be compared safely (blocker 4) + +A sampled hash can collide, and a collision here silently replaces history — the exact failure +this design calls worse than expansion. And `JSON.stringify(item).slice(n)` serializes the whole +object anyway, so sampling does not even buy the cost saving it was meant to. + +So every item — identified or not — is compared on bounded canonical content. An +`id`/`call_id` participates as additional occurrence evidence and never substitutes for content +equality, so the byte cap applies uniformly: an over-cap identified tool item is non-comparable +exactly like an over-cap message. + +The cap must be enforced DURING canonicalization, not after (blocker 9): serializing first and +measuring afterward still traverses and allocates the whole oversized value, which is the cost +the cap exists to avoid. So the writer is iterative, counts UTF-8 bytes as it goes, aborts the +moment it passes 8 KiB, and carries a node/depth limit so a deeply nested item cannot blow the +stack. Over the cap returns **not comparable**, and any not-comparable item anywhere in the +stored run aborts the whole check — the reviewer confirmed aborting is right, since skipping +just that item could align different occurrences and manufacture a false positive. + +### The counter stays internal (blocker 5) + +`ResponseStateMetrics` is pinned at 12 fields by `tests/memory-watchdog.test.ts:213` precisely +so a new field gets privacy review before reaching `/api/system/memory`. This layer does not +need that surface, so the counter is a test-only export (like the existing +`*ForTests` helpers) and neither pinned test changes. ## File change map -### NEW: src/server/responses/continuation-dedup.ts - -```ts -/** - * Detect whether a request's input array already contains messages that - * overlap with the stored continuation state. If substantial overlap is - * detected, skip the continuation expansion. - * - * Detection strategy: compare the first N message hashes from stored - * continuation against the request's input array. If ≥80% of stored - * messages appear in the input, the client already replayed history. - */ -export function detectHistoryOverlap( - requestInput: unknown[], - storedMessages: unknown[], -): { overlapping: boolean; overlapRatio: number }; - -/** - * Fingerprint a message for overlap detection. Uses a fast hash of - * role + first 200 chars of content. Images/tool-results use their - * type + id as fingerprint. - */ -export function messageFingerprint(message: unknown): string; +### MODIFY: src/responses/state.ts + +```diff ++/** Hard cap for canonicalizing ANY item. Past it, the item is not comparable. */ ++const REPLAY_FINGERPRINT_MAX_BYTES = 8 * 1024; ++/** Node/depth ceiling so a deeply nested item cannot blow the canonicalizer. */ ++const REPLAY_FINGERPRINT_MAX_DEPTH = 64; ++ ++/** ++ * Canonical fingerprint for replay comparison, or null when the item cannot be compared ++ * safely. ++ * ++ * Iterative and byte-counted: it aborts the moment it passes the cap rather than ++ * serializing the whole value and measuring afterwards, because a tool result can be ++ * megabytes and this runs on the request path. String escaping is counted incrementally ++ * for the same reason. ++ * ++ * The cap applies to EVERY item, identified or not. An `id`/`call_id` is additional ++ * occurrence evidence, never a substitute for content equality, so an over-cap tool item ++ * is non-comparable exactly like an over-cap message. ++ */ ++function replayItemFingerprint(item: unknown): string | null; ++ ++/** Non-empty provider-issued `id`/`call_id` on an item, else null. */ ++function providerIssuedIdentity(item: unknown): string | null; ++ ++/** ++ * Number of leading stored items the client already carries verbatim, or 0. ++ * ++ * Requires an exact ordered run: every stored item must match the client input item at the ++ * same index. Any not-comparable item aborts to 0 -- skipping just that item could align ++ * different occurrences and manufacture a false positive. ++ * ++ * Note (FU-2): stored input can contain proxy-injected guidance the client never saw ++ * (collaboration.ts:491 -> state.ts:1047) and ids repaired after recording ++ * (core.ts:2753/2793). Those sessions do not match here and expand as before. ++ */ ++function clientCarriedPrefixLength(stored: readonly unknown[], clientInput: readonly unknown[]): number; ``` -### MODIFY: src/server/responses/core.ts +Record the provider-output boundary at write time. Compute the normalized array ONCE and reuse +it for both fields, so the boundary can never disagree with the items it indexes: + +```diff + const clientThreadId = normalizedClientThreadId(opts?.clientThreadId); ++ const requestItems = inputItems(request.input); + setResidentEntry(response.id, { + createdAt: now(), + ...(clientThreadId ? { clientThreadId } : {}), +- items: [...inputItems(request.input), ...response.output], ++ items: [...requestItems, ...response.output], ++ // Where response.output begins. A replay skip requires a matched item at or past this ++ // index that also carries a provider-issued id -- position alone proves only that an ++ // item sits on the provider side, not that the provider authored it. Entries written ++ // before this field have no anchor and never skip. ++ providerOutputStart: requestItems.length, + }); +``` -Location: Around the continuation expansion block (near line 1621). +Inside `expandPreviousResponseInput`, before building the expanded object: ```diff - const hasUnexpandedPreviousResponse = !!parsed.previousResponseId - && typeof (body as { previous_response_id?: unknown }).previous_response_id === "string"; - -+ // Guard: if the client already included full history in input[], -+ // skip continuation expansion to prevent 1x → 2x → 3x compounding. -+ if (hasUnexpandedPreviousResponse && parsed._providerContinuation) { -+ const { overlapping } = detectHistoryOverlap( -+ parsed.input ?? [], -+ parsed._providerContinuation.messages ?? [], -+ ); -+ if (overlapping) { -+ parsed._previousResponseInputExpanded = true; // mark as already expanded -+ // Clear the continuation to prevent double-prepending -+ parsed._providerContinuation = undefined; -+ } -+ } ++ const clientInput = inputItems(request.input); ++ const stored = materialized.state.items; ++ const anchor = materialized.state.providerOutputStart; ++ // The client already replayed this history verbatim. Prepending the stored copy would ++ // double it, and the doubled turn is stored again, so the next turn triples (#1412: ++ // 127k -> 1.3M). ++ // ++ // Three independent conditions, all required. The run must cover the whole stored entry; ++ // it must reach the provider-output region; and some matched item in that region must ++ // carry a provider-issued id. Content equality alone cannot prove two items are the same ++ // occurrence -- a client repeating its own message produces an equal run without ever ++ // having seen this conversation -- and skipping on that would delete real history. ++ const carried = clientCarriedPrefixLength(stored, clientInput); ++ if ( ++ carried === stored.length ++ && anchor !== undefined ++ && carried > anchor ++ && stored.slice(anchor, carried).some(item => providerIssuedIdentity(item) !== null) ++ ) { ++ replayOverlapSkips += 1; ++ // Keep previous_response_id: Kiro and Cursor recover their conversation ids from it ++ // (kiro-wire.ts:112, cursor/request-builder.ts:283). Only the concatenation is skipped. ++ const unchanged = { ...request }; ++ // Same provenance boundary as a real expansion: the replayed prefix must not ++ // re-acknowledge historical compaction markers (parser.ts:372) and must stay visible ++ // to guidance de-duplication (collaboration.ts:455). ++ replayedInputPrefixLengths.set(unchanged, carried); ++ return unchanged; ++ } ``` +Also: add `providerOutputStart` to the `measureResidentEntry` field list (`state.ts:127`) so +resident byte accounting does not undercount, thread it through the four manual spill-payload +constructions and `materializeEntry` (`state.ts:847`), and validate it on load as a safe +integer in `0..items.length`, discarding anything else to `undefined`. A malformed snapshot +must degrade to "never skip", never to a bad boundary. + +Plus `replayOverlapSkipsForTests()` (NOT a `ResponseStateMetrics` field) and +`replayOverlapSkips = 0` in `clearResponseStateMemoryForTests` (`state.ts:1079`). + +### MODIFY: src/responses/spill-store.ts + +`ResponseSpillPayload` (`spill-store.ts:33`) is the durable spill shape and is versioned +separately from the resident entry, so the anchor has to be added there too or a spilled entry +silently loses it and stops skipping after a restart: + +```diff + export interface ResponseSpillPayload { + version: 1; + responseId: string; + createdAt: number; + clientThreadId?: string; + items: unknown[]; ++ /** Index in `items` where provider output begins; see state.ts replay-overlap detection. */ ++ providerOutputStart?: number; + providers?: OcxProviderContinuationState; + } +``` + +Compatibility here is **one-way, and the doc says so** rather than claiming more (blocker 12). +`validPayload` is a strict key allowlist (`spill-store.ts:264`): unknown keys make a payload +*corrupt*, not merely unrecognized. So a new build reads old payloads fine — the field is +absent and the entry never skips — but an older build would reject a new payload outright. + +That is acceptable for a forward-only upgrade and unacceptable for a downgrade, so the +allowlist gains `providerOutputStart` in the same change that starts writing it. A rollback +across this commit invalidates spilled entries, which degrades to a replay miss (already a +handled path, `state.ts:840`), not to corruption of live state. + +### MODIFY: src/types.ts + +The two provenance fields describe a condition, not an action, so their contracts are corrected +rather than left false (blocker 8). The reviewer confirmed no consumer depends on the literal +"the proxy inserted these" reading: + +```diff +- /** Number of leading raw input items restored from local previous_response_id state. */ ++ /** ++ * Boundary between replayed history and this turn newly appended input. Usually the ++ * items the proxy restored; also set when the CLIENT carried that history verbatim and ++ * the proxy skipped the prepend (see expandPreviousResponseInput). ++ */ + _replayPrefixLen?: number; +- /** True when the proxy expanded a previous_response_id request into a full input replay. */ ++ /** ++ * True when the full history for a previous_response_id request is present in the input -- ++ * whether the proxy expanded it or the client already carried it. Consumers use this to ++ * mean "this request is self-contained", never "the proxy mutated it". ++ */ + _previousResponseInputExpanded?: boolean; +``` + +The matching "Expansion provenance" comment at `src/responses/state.ts:376` is reworded the +same way. + ### NEW: tests/continuation-dedup.test.ts -Test cases: -1. Full history already in input → continuation skipped, chain stays 1x -2. Delta continuation (new messages only) → continuation applied normally -3. Partial overlap (50%) → continuation applied (conservative) -4. Empty input + continuation → continuation applied normally -5. Preserved: call ID, reasoning, image, tool result integrity -6. Preserved: stateless provider behavior unchanged +1. Client verbatim-replays full stored history including identified provider output → 1x +2. The same call three times → still 1x, not 2x/3x +3. Delta continuation (new items only) → expands exactly as today +4. **A client repeating an identical message does NOT skip** — stored `["repeat"]` with empty + output, client sends `["repeat", "new"]` (blocker 6) +5. **A run reaching id-less provider output does NOT skip** (blocker 11) +6. **A stored entry with no provider output never skips** (blocker 6) +7. **A legacy entry without `providerOutputStart` never skips** (back-compat) +8. **A malformed `providerOutputStart` (negative, > items.length, non-integer) degrades to + never-skip** (blocker 12) +9. **A spilled-then-materialized entry keeps its anchor and still skips** (blocker 12) +10. **A snapshot restart preserves the anchor** (blocker 12) +11. Resident byte accounting includes the new field (blocker 12) +12. Partial overlap → expands; reordered items sharing a set but not a sequence → expands +13. Empty client input + stored history → expands normally +14. `previous_response_id` survives a skip, so provider continuity is intact (blocker 1) +15. A skip records the prefix length, so a historical `context_compaction` marker is NOT + re-acknowledged, and guidance is not injected twice (blocker 2) +16. Two id-less items sharing a long prefix but differing in the tail do NOT fingerprint equal +17. **An over-cap item is non-comparable even WITH an id, and canonicalization stops early + rather than serializing the whole value** (blocker 14 / 9) +18. A deeply nested item hits the depth limit → not comparable, no stack overflow +19. `responseStateMetrics()` still returns exactly 12 fields (blocker 5) +20. `clearResponseStateMemoryForTests()` resets the skip counter (blocker 10) +21. Existing `tests/responses-state.test.ts` and the spill suite stay green ## Activation scenario -A Codex session with 50 turns: each turn the client sends the full 50-turn -history as `input[]`. Without this fix, turn 3's request would contain -50 + 50 + 50 = 150 messages. With this fix, the overlap detector recognizes -the duplication and the request stays at 50 messages. +The #1412 population: a stateless provider (DeepSeek tool-result turns) where the client sends +the full conversation AND `previous_response_id`. Today turn 3 sends 50 stored + 50 client = +100 items, then 150. With the skip it stays at 50. + +Observable proof in C: assert the item count equals the client input count AND that +`replayOverlapSkipsForTests()` incremented — a silent no-op is otherwise indistinguishable +from a working skip. + +## Follow-ups (not this layer) + +- **FU-2: client-visible projection for overlap detection.** Recognize replays of history the + proxy mutated (injected guidance, repaired ids). Needs a sanitized #1412 fixture first; + designing it blind is how the round-1 predicate got written. Explicitly NOT covered by M0-1: + admission runs after expansion and parsing, and fails open on unknown ceilings. ## Scope boundary -IN: Dedup detection module + core.ts guard + test file -OUT: Changing Codex client behavior, modifying continuation cache storage, - restructuring the continuation protocol +IN: overlap detection, the `providerOutputStart` anchor and its persistence through resident +entries, spill payloads, byte accounting and load validation, the internal counter and its +reset, the two provenance contracts in `src/types.ts`, the test file +OUT: `ResponseStateMetrics` and `/api/system/memory`, the scope-mismatch path, +`_providerContinuation` semantics, the injection sites in `collaboration.ts`, and +client-visible projection (FU-2) + diff --git a/src/responses/spill-store.ts b/src/responses/spill-store.ts index c338155730..dbf5502018 100644 --- a/src/responses/spill-store.ts +++ b/src/responses/spill-store.ts @@ -36,6 +36,17 @@ export interface ResponseSpillPayload { createdAt: number; clientThreadId?: string; items: unknown[]; + /** + * Index in `items` where the provider output begins, used by replay-overlap detection + * in state.ts. Optional so a payload written before this field still loads (it simply + * never authorizes a skip). + * + * Compatibility is FORWARD-ONLY: `validPayload` is a strict key allowlist, so a build + * predating this field rejects a payload carrying it as corrupt rather than ignoring + * it. Rolling back across this change invalidates spilled entries, which degrades to a + * replay miss — an already-handled path — not to corrupted live state. + */ + providerOutputStart?: number; providers?: OcxProviderContinuationState; } @@ -261,12 +272,19 @@ function validPayload(value: unknown, responseId: string): value is ResponseSpil if (!value || typeof value !== "object" || Array.isArray(value)) return false; const payload = value as Record; const keys = Object.keys(payload); - if (keys.some(key => !["version", "responseId", "createdAt", "clientThreadId", "items", "providers"].includes(key))) return false; + if (keys.some(key => !["version", "responseId", "createdAt", "clientThreadId", "items", "providerOutputStart", "providers"].includes(key))) return false; if (payload.version !== 1 || payload.responseId !== responseId) return false; if (typeof payload.createdAt !== "number" || !Number.isFinite(payload.createdAt)) return false; if (payload.clientThreadId !== undefined && (typeof payload.clientThreadId !== "string" || payload.clientThreadId.trim().length === 0)) return false; if (!Array.isArray(payload.items)) return false; + // A malformed boundary must degrade to "never skip", never to a bad index: reject the + // payload outright so materialization treats it as corrupt rather than trusting it. + if (payload.providerOutputStart !== undefined) { + const anchor = payload.providerOutputStart; + if (typeof anchor !== "number" || !Number.isSafeInteger(anchor) + || anchor < 0 || anchor > payload.items.length) return false; + } if (payload.providers !== undefined) { if (!payload.providers || typeof payload.providers !== "object" || Array.isArray(payload.providers)) return false; for (const providerState of Object.values(payload.providers)) { @@ -289,6 +307,7 @@ export function writeResponseSpillDurably( createdAt: state.createdAt, ...(state.clientThreadId ? { clientThreadId: state.clientThreadId } : {}), items: state.items, + ...(state.providerOutputStart !== undefined ? { providerOutputStart: state.providerOutputStart } : {}), ...(state.providers ? { providers: state.providers } : {}), }; const serialized = JSON.stringify(payload); diff --git a/src/responses/state.ts b/src/responses/state.ts index 06aa9329bb..a08ed1012e 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -39,6 +39,8 @@ interface ResidentResponseState { createdAt: number; clientThreadId?: string; items: unknown[]; + /** Index in `items` where provider output begins; see clientCarriedPrefixLength. */ + providerOutputStart?: number; providers?: OcxProviderContinuationState; sizeBytes: number; } @@ -47,6 +49,8 @@ interface SpilledResponseState { kind: "spill"; createdAt: number; clientThreadId?: string; + /** Mirrors the spilled payload boundary so a spilled entry keeps its anchor. */ + providerOutputStart?: number; providers?: OcxProviderContinuationState; spill: ResponseSpillRef; sizeBytes: number; @@ -130,6 +134,7 @@ function measureResidentEntry(id: string, entry: ResidentInput): ResidentRespons createdAt: entry.createdAt, ...(entry.clientThreadId ? { clientThreadId: entry.clientThreadId } : {}), items: entry.items, + ...(entry.providerOutputStart !== undefined ? { providerOutputStart: entry.providerOutputStart } : {}), ...(entry.providers ? { providers: entry.providers } : {}), }); return sizeBytes === null ? null : { kind: "resident", ...entry, sizeBytes }; @@ -252,12 +257,14 @@ function replaceSpillEntryAtomically( createdAt: candidate.createdAt, ...(candidate.clientThreadId ? { clientThreadId: candidate.clientThreadId } : {}), items: candidate.items, + ...(candidate.providerOutputStart !== undefined ? { providerOutputStart: candidate.providerOutputStart } : {}), ...(candidate.providers ? { providers: candidate.providers } : {}), }); const base: Omit = { kind: "spill", createdAt: candidate.createdAt, ...(candidate.clientThreadId ? { clientThreadId: candidate.clientThreadId } : {}), + ...(candidate.providerOutputStart !== undefined ? { providerOutputStart: candidate.providerOutputStart } : {}), ...(candidate.providers ? { providers: candidate.providers } : {}), spill: ref, }; @@ -332,6 +339,7 @@ function admitOversizedCandidate( createdAt: candidate.createdAt, ...(candidate.clientThreadId ? { clientThreadId: candidate.clientThreadId } : {}), items: candidate.items, + ...(candidate.providerOutputStart !== undefined ? { providerOutputStart: candidate.providerOutputStart } : {}), ...(candidate.providers ? { providers: candidate.providers } : {}), }); // Enforce the ceiling against the REAL envelope: the spill payload adds @@ -373,9 +381,11 @@ function admitOversizedCandidate( } } -// Expansion provenance must stay proxy-private: a WeakMap distinguishes replayed history from the +// Replay provenance must stay proxy-private: a WeakMap distinguishes replayed history from the // newly appended input suffix without adding an unknown field that native passthrough could send -// upstream. The parser uses this boundary to acknowledge historical compaction markers exactly once. +// upstream. The parser uses this boundary to acknowledge historical compaction markers exactly +// once. It records the boundary whether the proxy prepended the history or the client already +// carried it — the boundary is the same either way, and only its provenance differs. const replayedInputPrefixLengths = new WeakMap(); const replayFailures = new WeakMap(); let loaded = false; @@ -419,12 +429,23 @@ function loadSnapshotEntry(id: string, value: unknown): void { const clientThreadId = typeof rec.clientThreadId === "string" && rec.clientThreadId.trim().length > 0 ? rec.clientThreadId.trim() : undefined; + // A malformed boundary degrades to "never skip" rather than to a bad index: an untrusted + // snapshot must not be able to authorize dropping conversation history. + const anchorFor = (itemCount: number): number | undefined => { + const raw = (rec as { providerOutputStart?: unknown }).providerOutputStart; + return Number.isSafeInteger(raw) && (raw as number) >= 0 && (raw as number) <= itemCount + ? raw as number + : undefined; + }; if (rec.kind === "spill") { if (!isSpillRef(rec.spill)) return; const base: Omit = { kind: "spill", createdAt: rec.createdAt, ...(clientThreadId ? { clientThreadId } : {}), + // Item count is unknown until materialization, so accept any non-negative integer + // here; the spill payload validator re-checks it against the real array. + ...(anchorFor(Number.MAX_SAFE_INTEGER) !== undefined ? { providerOutputStart: anchorFor(Number.MAX_SAFE_INTEGER) } : {}), ...(rec.providers ? { providers: rec.providers } : {}), spill: rec.spill, }; @@ -451,6 +472,7 @@ function loadSnapshotEntry(id: string, value: unknown): void { createdAt: rec.createdAt, ...(clientThreadId ? { clientThreadId } : {}), items: rec.items, + ...(anchorFor(rec.items.length) !== undefined ? { providerOutputStart: anchorFor(rec.items.length) } : {}), ...(providers ? { providers } : {}), }); if (!resident) { @@ -740,6 +762,94 @@ function inputItems(input: unknown): unknown[] { return [input]; } +/** Hard cap for canonicalizing ANY item. Past it, the item is not comparable. */ +const REPLAY_FINGERPRINT_MAX_BYTES = 8 * 1024; +/** Depth ceiling so a pathologically nested item cannot blow the canonicalizer. */ +const REPLAY_FINGERPRINT_MAX_DEPTH = 64; + +let replayOverlapSkips = 0; + +/** + * Canonical, order-stable fingerprint for one input item, or null when the item cannot be + * compared safely. + * + * Byte-counted DURING the walk rather than serialize-then-measure: a tool result can be + * megabytes and this runs on the request path, so the point of the cap is to stop early, + * not to discover afterwards that we should have. Object keys are sorted so two + * semantically identical items cannot differ by key order alone. + * + * The cap applies to EVERY item. An `id`/`call_id` is additional occurrence evidence, never + * a substitute for content equality, so an over-cap identified tool item is non-comparable + * exactly like an over-cap message. + */ +function replayItemFingerprint(item: unknown): string | null { + const out: string[] = []; + let bytes = 0; + const push = (text: string): boolean => { + bytes += Buffer.byteLength(text, "utf8"); + if (bytes > REPLAY_FINGERPRINT_MAX_BYTES) return false; + out.push(text); + return true; + }; + const walk = (value: unknown, depth: number): boolean => { + if (depth > REPLAY_FINGERPRINT_MAX_DEPTH) return false; + if (value === null || typeof value !== "object") return push(JSON.stringify(value) ?? "null"); + if (Array.isArray(value)) { + if (!push("[")) return false; + for (const element of value) { + if (!walk(element, depth + 1)) return false; + if (!push(",")) return false; + } + return push("]"); + } + if (!push("{")) return false; + for (const key of Object.keys(value as Record).sort()) { + if (!push(JSON.stringify(key))) return false; + if (!walk((value as Record)[key], depth + 1)) return false; + if (!push(",")) return false; + } + return push("}"); + }; + return walk(item, 0) ? out.join("") : null; +} + +/** Non-empty provider-issued `id`/`call_id` on an item, else null. */ +function providerIssuedIdentity(item: unknown): string | null { + if (!item || typeof item !== "object" || Array.isArray(item)) return null; + const record = item as { id?: unknown; call_id?: unknown }; + for (const candidate of [record.id, record.call_id]) { + if (typeof candidate === "string" && candidate.trim().length > 0) return candidate; + } + return null; +} + +/** + * Number of leading stored items the client already carries verbatim, or 0. + * + * Requires an exact ordered run: every stored item must match the client input item at the + * same index. Any not-comparable item aborts to 0 — skipping just that item could align two + * different occurrences and manufacture a false positive, and a false positive here deletes + * real conversation history. + * + * Known gap (FU-2): stored input can contain proxy-injected guidance the client never saw, + * and ids repaired after recording. Those sessions do not match here and expand as before. + */ +function clientCarriedPrefixLength(stored: readonly unknown[], clientInput: readonly unknown[]): number { + if (stored.length === 0 || clientInput.length < stored.length) return 0; + for (let index = 0; index < stored.length; index += 1) { + const storedPrint = replayItemFingerprint(stored[index]); + if (storedPrint === null) return 0; + const clientPrint = replayItemFingerprint(clientInput[index]); + if (clientPrint === null || storedPrint !== clientPrint) return 0; + } + return stored.length; +} + +/** Test-only: replay prepends skipped because the client already carried the history. */ +export function replayOverlapSkipsForTests(): number { + return replayOverlapSkips; +} + function pruneResponses(at = now()): void { for (const [id, state] of states) { if (at - state.createdAt > RESPONSE_TTL_MS) deleteEntry(id); @@ -765,6 +875,7 @@ function pruneResponses(at = now()): void { createdAt: entry.createdAt, ...(entry.clientThreadId ? { clientThreadId: entry.clientThreadId } : {}), items: entry.items, + ...(entry.providerOutputStart !== undefined ? { providerOutputStart: entry.providerOutputStart } : {}), ...(entry.providers ? { providers: entry.providers } : {}), }); if (swapResidentForSpill(oldestId, entry, ref)) spillCounters.writes += 1; @@ -807,6 +918,7 @@ export function evictOldestResponseContinuationForBudget(): number { createdAt: entry.createdAt, ...(entry.clientThreadId ? { clientThreadId: entry.clientThreadId } : {}), items: entry.items, + ...(entry.providerOutputStart !== undefined ? { providerOutputStart: entry.providerOutputStart } : {}), ...(entry.providers ? { providers: entry.providers } : {}), }); if (swapResidentForSpill(id, entry, ref)) spillCounters.writes += 1; @@ -848,6 +960,9 @@ function materializeEntry( createdAt: result.payload.createdAt, ...(result.payload.clientThreadId ? { clientThreadId: result.payload.clientThreadId } : {}), items: result.payload.items, + ...(result.payload.providerOutputStart !== undefined + ? { providerOutputStart: result.payload.providerOutputStart } + : {}), ...(result.payload.providers ? { providers: result.payload.providers } : {}), }); if (!state) { @@ -892,6 +1007,39 @@ export function expandPreviousResponseInput(body: unknown, clientThreadId?: stri replayScopeMismatchDrops += 1; return freshRequest; } + // The client already replayed this history verbatim. Prepending the stored copy would + // double it, and the doubled turn is stored again, so the next turn triples (#1412 saw + // 127k of real context reach 1.3M tokens this way). + // + // Three conditions, all required. The run must cover the whole stored entry; it must reach + // the provider-output region; and some matched item in that region must carry a + // provider-issued id. The last one is the load-bearing part: content equality alone proves + // two items look alike, not that they are the same occurrence, so a client that merely + // repeats its own message would otherwise authorize a skip that deletes real history. + // There is no invariant that provider output always carries ids, so an entry whose output + // has none simply never skips. + { + const clientInput = inputItems(request.input); + const stored = materialized.state.items; + const anchor = materialized.state.providerOutputStart; + const carried = clientCarriedPrefixLength(stored, clientInput); + if ( + carried === stored.length + && anchor !== undefined + && carried > anchor + && stored.slice(anchor, carried).some(item => providerIssuedIdentity(item) !== null) + ) { + replayOverlapSkips += 1; + // Keep previous_response_id: Kiro and Cursor recover their conversation ids from it + // (kiro-wire.ts, cursor/request-builder.ts). Only the concatenation is skipped. + const unchanged = { ...request }; + // Same provenance boundary a real expansion would record, so the replayed prefix does + // not re-acknowledge historical compaction markers (parser.ts) and stays visible to + // guidance de-duplication (collaboration.ts). + replayedInputPrefixLengths.set(unchanged, carried); + return unchanged; + } + } const expanded = { ...request, input: [...materialized.state.items, ...inputItems(request.input)], @@ -1044,10 +1192,17 @@ export function rememberResponseState( }); } const clientThreadId = normalizedClientThreadId(opts?.clientThreadId); + // Compute the normalized array once and reuse it for both fields, so the recorded + // boundary can never disagree with the items it indexes. + const requestItems = inputItems(request.input); setResidentEntry(response.id, { createdAt: now(), ...(clientThreadId ? { clientThreadId } : {}), - items: [...inputItems(request.input), ...response.output], + items: [...requestItems, ...response.output], + // Where response.output begins. A replay skip requires a matched item at or past this + // index that also carries a provider-issued id — position alone proves only that an item + // sits on the provider side, not that the provider authored it. + providerOutputStart: requestItems.length, // Always preserve the Cursor conversation id so the next tool-result turn can continue the SAME // Cursor conversation (multi-turn continuation). Separately track whether Cursor's own // checkpoint/cache is safe to reuse: a turn that ended with a pending client tool call produced an @@ -1093,6 +1248,7 @@ export function clearResponseStateMemoryForTests(): void { spillCounters.writeFailures = 0; spillCounters.readFailures = 0; replayScopeMismatchDrops = 0; + replayOverlapSkips = 0; persistAttemptHookForTests = null; loaded = false; } diff --git a/src/types.ts b/src/types.ts index f6b2f55ac3..6939c07900 100644 --- a/src/types.ts +++ b/src/types.ts @@ -32,11 +32,19 @@ export interface OcxParsedRequest { stream: boolean; options: OcxRequestOptions; _rawBody?: unknown; - /** Number of leading raw input items restored from local previous_response_id state. */ + /** + * Boundary between replayed history and this turn's newly appended input. Usually the + * items the proxy restored from local previous_response_id state; also set when the + * CLIENT already carried that history verbatim and the proxy skipped the prepend. + */ _replayPrefixLen?: number; /** Parsed-message index before the first conversational item in a continuation's current delta. */ _continuationConversationMessageIndex?: number; - /** True when the proxy expanded a previous_response_id request into a full input replay. */ + /** + * True when the full history for a previous_response_id request is present in the input — + * whether the proxy expanded it or the client already sent it. Consumers read this as + * "this request is self-contained", never as "the proxy mutated it". + */ _previousResponseInputExpanded?: boolean; /** Provider-private stable Cursor conversation id resolved from the Responses previous_response_id chain. */ _cursorConversationId?: string; diff --git a/tests/continuation-dedup.test.ts b/tests/continuation-dedup.test.ts new file mode 100644 index 0000000000..2d14502dbc --- /dev/null +++ b/tests/continuation-dedup.test.ts @@ -0,0 +1,329 @@ +/** + * Replay-overlap detection (#1412): when the client already carries the stored history, + * prepending the stored copy doubles it, and the doubled turn is stored again. + * + * The dangerous failure here is the FALSE SKIP -- dropping history the client did not + * actually send -- so most of these cases assert that a plausible-looking overlap still + * expands. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + clearResponseStateForTests, + expandPreviousResponseInput, + previousResponseReplayPrefixLength, + rememberResponseState, + replayOverlapSkipsForTests, + responseStateMetrics, + setResponseStateByteCapForTests, +} from "../src/responses/state"; + +let home: string; +let priorHome: string | undefined; + +beforeEach(() => { + priorHome = process.env["OPENCODEX_HOME"]; + home = mkdtempSync(join(tmpdir(), "ocx-dedup-")); + process.env["OPENCODEX_HOME"] = home; + clearResponseStateForTests(); +}); + +afterEach(() => { + clearResponseStateForTests(); + rmSync(home, { recursive: true, force: true }); + if (priorHome === undefined) delete process.env["OPENCODEX_HOME"]; + else process.env["OPENCODEX_HOME"] = priorHome; +}); + +/** A completed response whose output carries a provider-issued id. */ +function identifiedResponse(id: string, text: string) { + return { + id, + status: "completed", + output: [{ type: "message", id: `msg_${id}`, role: "assistant", content: text }], + }; +} + +/** A completed response whose output carries NO stable identity. */ +function anonymousResponse(id: string, text: string) { + return { + id, + status: "completed", + output: [{ type: "message", role: "assistant", content: text }], + }; +} + +describe("replay overlap: skips a verbatim client-carried history", () => { + test("does not double when the client replays the whole stored conversation", () => { + const first = identifiedResponse("resp_1", "answer one"); + rememberResponseState({ model: "m", input: "question one", store: true }, first); + + const clientInput = [ + { role: "user", content: "question one" }, + first.output[0], + { role: "user", content: "question two" }, + ]; + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: clientInput, + }) as { previous_response_id?: string; input: unknown[] }; + + expect(result.input).toEqual(clientInput); + expect(replayOverlapSkipsForTests()).toBe(1); + }); + + test("stays 1x across repeated identical turns", () => { + const first = identifiedResponse("resp_rep", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + const clientInput = [{ role: "user", content: "ask" }, first.output[0]]; + + for (let turn = 0; turn < 3; turn += 1) { + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: clientInput, + }) as { input: unknown[] }; + expect(result.input).toHaveLength(clientInput.length); + } + expect(replayOverlapSkipsForTests()).toBe(3); + }); + + test("keeps previous_response_id so provider continuity survives", () => { + // Kiro and Cursor recover their conversation ids from this field; stripping it would + // silently start a new upstream conversation. + const first = identifiedResponse("resp_keep", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, first.output[0]], + }) as { previous_response_id?: string }; + + expect(result.previous_response_id).toBe("resp_keep"); + }); + + test("records the replay prefix boundary so historical markers stay acknowledged", () => { + // The parser uses this boundary to avoid re-acknowledging an old compaction marker, + // and guidance de-duplication scans the same prefix. + const first = identifiedResponse("resp_prefix", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, first.output[0], { role: "user", content: "next" }], + }); + + expect(previousResponseReplayPrefixLength(result)).toBe(2); + }); +}); + +describe("replay overlap: refuses to skip without provider-authored evidence", () => { + test("a client repeating its own message does not authorize a skip", () => { + // The decisive case. Stored history is one client message with empty output; the client + // legitimately sends that same message again as a NEW occurrence. Content equality alone + // would skip here and silently delete the earlier occurrence. + const first = { id: "resp_repeat", status: "completed", output: [] as unknown[] }; + rememberResponseState({ model: "m", input: "repeat", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: "resp_repeat", + input: [{ role: "user", content: "repeat" }, { role: "user", content: "new" }], + }) as { input: unknown[] }; + + expect(result.input).toHaveLength(3); + expect(replayOverlapSkipsForTests()).toBe(0); + }); + + test("id-less provider output does not authorize a skip", () => { + // Position inside response.output proves only where an item sits, not who authored it. + const first = anonymousResponse("resp_anon", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, first.output[0]], + }) as { input: unknown[] }; + + expect(result.input).toHaveLength(4); + expect(replayOverlapSkipsForTests()).toBe(0); + }); + + test("a partial prefix match expands rather than truncating history", () => { + const first = identifiedResponse("resp_partial", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, { role: "user", content: "different" }], + }) as { input: unknown[] }; + + expect(result.input).toHaveLength(4); + expect(replayOverlapSkipsForTests()).toBe(0); + }); + + test("the same items in a different order expand", () => { + const first = identifiedResponse("resp_order", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [first.output[0], { role: "user", content: "ask" }], + }) as { input: unknown[] }; + + expect(replayOverlapSkipsForTests()).toBe(0); + expect(result.input).toHaveLength(4); + }); + + test("a delta continuation expands exactly as before", () => { + const first = identifiedResponse("resp_delta", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "only the new part" }], + }) as { input: unknown[] }; + + expect(result.input).toEqual([ + { role: "user", content: "ask" }, + first.output[0], + { role: "user", content: "only the new part" }, + ]); + expect(replayOverlapSkipsForTests()).toBe(0); + }); + + test("empty client input expands", () => { + const first = identifiedResponse("resp_empty", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [], + }) as { input: unknown[] }; + + expect(result.input).toHaveLength(2); + expect(replayOverlapSkipsForTests()).toBe(0); + }); +}); + +describe("replay overlap: comparison is bounded", () => { + test("items sharing a long prefix but differing in the tail are not equal", () => { + // A sampled hash would collide here and skip, silently replacing history. + const shared = "x".repeat(2_000); + const first = { + id: "resp_tail", + status: "completed", + output: [{ type: "message", id: "msg_tail", role: "assistant", content: `${shared}A` }], + }; + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [ + { role: "user", content: "ask" }, + { type: "message", id: "msg_tail", role: "assistant", content: `${shared}B` }, + ], + }) as { input: unknown[] }; + + expect(replayOverlapSkipsForTests()).toBe(0); + expect(result.input).toHaveLength(4); + }); + + test("an over-cap item is not comparable even with an id, so the turn expands", () => { + const huge = "y".repeat(64 * 1024); + const first = { + id: "resp_huge", + status: "completed", + output: [{ type: "message", id: "msg_huge", role: "assistant", content: huge }], + }; + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, first.output[0]], + }) as { input: unknown[] }; + + expect(replayOverlapSkipsForTests()).toBe(0); + expect(result.input).toHaveLength(4); + }); + + test("a deeply nested item is not comparable and does not overflow the stack", () => { + let nested: Record = { leaf: true }; + for (let depth = 0; depth < 500; depth += 1) nested = { nested }; + const first = { + id: "resp_deep", + status: "completed", + output: [{ type: "message", id: "msg_deep", role: "assistant", content: nested }], + }; + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, first.output[0]], + }) as { input: unknown[] }; + + expect(replayOverlapSkipsForTests()).toBe(0); + expect(result.input).toHaveLength(4); + }); +}); + +describe("replay overlap: contracts held elsewhere", () => { + test("the anchor survives a spill round-trip", () => { + // Regression: writeResponseSpillDurably rebuilds the payload field by field, so an + // anchor threaded everywhere else was still dropped on the way to disk — and a spilled + // entry silently stopped being deduplicable. + setResponseStateByteCapForTests(1024); + try { + const big = "e".repeat(4096); + const first = { + id: "resp_spilled", + status: "completed", + output: [{ type: "message", id: "msg_spilled", role: "assistant", content: big }], + }; + rememberResponseState({ model: "m", input: "ask", store: true }, first); + + const result = expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, first.output[0], { role: "user", content: "next" }], + }) as { input: unknown[] }; + + expect(result.input).toHaveLength(3); + expect(replayOverlapSkipsForTests()).toBe(1); + } finally { + setResponseStateByteCapForTests(null); + } + }); + + test("the skip counter is not published on the memory surface", () => { + // /api/system/memory pins exactly 12 privacy-reviewed scalar fields. + expect(Object.keys(responseStateMetrics())).toHaveLength(12); + }); + + test("clearing state for tests resets the skip counter", () => { + const first = identifiedResponse("resp_reset", "answer"); + rememberResponseState({ model: "m", input: "ask", store: true }, first); + expandPreviousResponseInput({ + model: "m", + previous_response_id: first.id, + input: [{ role: "user", content: "ask" }, first.output[0]], + }); + expect(replayOverlapSkipsForTests()).toBe(1); + + clearResponseStateForTests(); + expect(replayOverlapSkipsForTests()).toBe(0); + }); +}); diff --git a/tests/responses-state.test.ts b/tests/responses-state.test.ts index e4c53693f6..b6afe0163d 100644 --- a/tests/responses-state.test.ts +++ b/tests/responses-state.test.ts @@ -1046,7 +1046,7 @@ describe("Responses previous_response_id state", () => { ); const items = [{ role: "user", content: "한글🙂" }, ...output]; const expected = Buffer.byteLength(JSON.stringify({ - responseId: "resp_다국어", createdAt: at, items, providers, + responseId: "resp_다국어", createdAt: at, items, providerOutputStart: 1, providers, }), "utf8"); expect(getStoredResponseBytesForTests()).toBe(expected); } finally {