feat: multi-turn conversation state management (DEV-127) - #843
feat: multi-turn conversation state management (DEV-127)#843LukasParke wants to merge 3 commits into
Conversation
Implement the session/conversation state store per the approved multi-turn state design (docs/multi-turn-state.md / PR #124): - ConversationStateStore: create / get / appendTurn / put / expire / clear over a pluggable ConversationStateBackend (load/save/delete/list) - Full turn-data serialization (messages, tool results, metadata) with schema validation; CorruptedStateError on missing/corrupted state - TTL-based expiry driven by updatedAt, backend-storage delegation intact - InMemory (deep-copy isolation) and JSON-file backends, both zero-deps - createStateAccessor() bridge to the StateAccessor contract for callModel - Unit tests: CRUD, expiry, corruption, concurrent access, path-traversal hardening, durability across store instances
Integrate the ConversationStateStore into the request/turn flow so a conversation id carries context across turns through the normal entry point: - New callModelWithState() entry point (funcs + OpenRouter SDK method via Speakeasy custom-code regions): takes conversationId + stateStore, derives the StateAccessor, and delegates to callModel — single-turn callers that omit a conversation id keep the existing callModel path with no store interaction - ModelResult.initStream() now persists each new turn's input items into the stored message history before the request, so later turns see prior input + output (previously only response output was saved) - createStateAccessor() guards the bound conversation id on save: a random-id state created by ModelResult on first turn can no longer strand the conversation under an unreachable key — missing/expired state falls back to a fresh document under the caller's id - Store/backends importable via package subpath exports (no edits to generated src/index.ts) Tests (7 new, end-to-end through the entry point with the API mocked at betaResponsesSend): first-turn creation, multi-turn context visibility, expired-state fallback, id-guard regression, input validation, and single-turn no-store behavior. Unit suite 186/186, eslint clean, no new tsc errors.
Cover the four integration scenarios end-to-end through the turn pipeline (betaResponsesSend mocked, runs in the CI unit project): - 3+ turn conversation retaining context from every earlier turn, in order - parallel conversations with isolated state (interleaved + concurrent) - state expiry/cleanup: mid-conversation TTL fallback, store.expire() sweeps, store.clear() + fresh restart - missing or corrupted state: first-turn create, CorruptedStateError on invalid JSON/schema in both memory and file backends, sibling isolation Fix a gap the tests exposed: InMemoryConversationStateBackend now implements list(), so store-wide expire() actually sweeps on the documented testing backend (previously a silent no-op). Docs: docs/multi-turn-state.md describes how state is keyed (conversationId + id guard), configured (per-store ttlMs/now; no env vars), expired, cleared, and recovered from corruption, with testing recipes. Linked from README. Unit suite 232/232 (11 new), eslint clean, no new tsc errors.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } else if (baseRequest.input) { | ||
| // First turn of a fresh state: the whole input is the new turn. | ||
| const inputArray = Array.isArray(baseRequest.input) | ||
| ? baseRequest.input | ||
| : [baseRequest.input]; | ||
| newTurnItems = inputArray as models.BaseInputsUnion[]; | ||
| } | ||
|
|
||
| // Persist the new-turn input into the stored message history so later | ||
| // turns can see it (the response output is appended separately once the | ||
| // API responds, and tool results as they execute). | ||
| if (this.stateAccessor && this.currentState && newTurnItems) { | ||
| await this.saveStateSafely({ | ||
| messages: appendToMessages(this.currentState.messages, newTurnItems), | ||
| }); |
There was a problem hiding this comment.
🔴 Follow-up turns send raw text where the service expects structured messages, so continuing a conversation can fail
The caller's plain-text prompt is stored into the conversation history verbatim (appendToMessages(this.currentState.messages, newTurnItems) at src/lib/model-result.ts:1071-1074) instead of being wrapped as a user message, so every later turn sends a request the service can reject.
Impact: A second or third turn of a stored conversation can be rejected by the API (or lose the user's earlier wording), breaking multi-turn chats that pass a plain string prompt.
Mechanism: unwrapped string items inside the input array
When input is a string, initStream builds newTurnItems = [baseRequest.input] (src/lib/model-result.ts:1060-1066) and persists that array element as-is into state.messages. On the next turn the same function builds the API input via appendToMessages(this.currentState.messages, newTurnItems) (src/lib/model-result.ts:1052), producing e.g. ["My name is Ada.", {type:'message',role:'assistant',...}, "What is my name?"].
models.InputsUnion allows a bare string or an array of item objects (src/models/inputsunion.ts:442+); the array variant has no string member, i.e. array elements must be items such as InputsMessage. The rest of the pipeline recognizes this: makeFollowupRequest explicitly wraps a non-array input as { role: 'user', content: originalInput } (src/lib/model-result.ts:847-853). The new persistence path skips that normalization, so stored history (and the resulting request) contains raw strings mixed with item objects. The new tests assert this behavior (tests/unit/call-model-with-state.test.ts:189 expects messages[0] to be the raw string) but mock betaResponsesSend, so no real request validation occurs.
Suggested fix: normalize string inputs into { role: 'user', content: <string> } items before appending them to state.messages (and before appending them to an existing-history input array).
Prompt for agents
In src/lib/model-result.ts initStream, the new-turn items derived from baseRequest.input are appended verbatim into conversation state messages and, on continuation turns, into the request input array. When the caller passes `input` as a plain string, the stored/sent array ends up containing a bare string element mixed with structured response items. The Responses API input array requires item objects (role/content messages, function_call_output, etc.) — a bare string is only valid as the whole `input`, not as an array element. Note makeFollowupRequest already handles this by wrapping a non-array input as { role: 'user', content: input }. Fix by normalizing string (and any non-item) inputs to user message items before appending them to state.messages and before merging them with existing history for the request. Existing tests in tests/unit/call-model-with-state.test.ts and tests/unit/multi-turn-integration.test.ts assert the raw-string shape and will need updating.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Persist the new-turn input into the stored message history so later | ||
| // turns can see it (the response output is appended separately once the | ||
| // API responds, and tool results as they execute). | ||
| if (this.stateAccessor && this.currentState && newTurnItems) { | ||
| await this.saveStateSafely({ | ||
| messages: appendToMessages(this.currentState.messages, newTurnItems), | ||
| }); |
There was a problem hiding this comment.
🟡 A prompt is recorded in the conversation history before the request succeeds, duplicating it on retry
The new turn's prompt is written into the saved conversation history (saveStateSafely({ messages: ... }) at src/lib/model-result.ts:1071-1074) before the request to the model is made, so a failed request leaves the prompt stored and a retry stores it twice.
Impact: After a network error or rate-limit failure, the user's message is remembered as if it had been asked, and retrying the same turn makes it appear twice in the conversation.
Mechanism: persist-before-send ordering in initStream
initStream persists messages: appendToMessages(this.currentState.messages, newTurnItems) and only afterwards calls betaResponsesSend (src/lib/model-result.ts:1070-1078). If betaResponsesSend fails (!apiResult.ok throws) the stored document already contains the new input with no corresponding assistant output. A caller retrying the same turn with the same conversationId loads that history and appends the identical input again, so the request carries the prompt twice and history diverges from what the model actually answered.
A safer ordering is to persist the new-turn input together with the response (or only after the API call is accepted), or to make the append idempotent for an unanswered trailing input.
Was this helpful? React with 👍 or 👎 to provide feedback.
| private pathFor(id: string): string { | ||
| // Sanitize the id to prevent path traversal; ids are `conv_<uuid>` by | ||
| // default but callers may supply arbitrary strings. | ||
| const safe = normalize(id).replace(/[^a-zA-Z0-9_.-]/g, '_'); | ||
| const p = join(this.dir, `${safe}.json`); | ||
| if (!p.startsWith(this.dir + sep)) { | ||
| throw new CorruptedStateError(id, 'id resolves outside state directory'); | ||
| } | ||
| return p; | ||
| } | ||
|
|
||
| async load(id: string): Promise<ConversationState<TTools> | null> { | ||
| let raw: string; | ||
| try { | ||
| raw = await readFile(this.pathFor(id), 'utf8'); | ||
| } catch (err) { | ||
| if ((err as NodeJS.ErrnoException).code === 'ENOENT') return null; | ||
| throw err; | ||
| } | ||
| return deserializeState<TTools>(raw, id); | ||
| } |
There was a problem hiding this comment.
🟨 File state backend loads and validates arbitrary on-disk JSON without id binding
FileConversationStateBackend.load reads a JSON document from disk and returns it after only structural validation (assertValidState), never verifying that the document's id matches the requested conversation id (src/lib/conversation-state-store.ts:296-305, src/lib/conversation-state-store.ts:103-127). Because pathFor sanitizes ids lossily (every non [A-Za-z0-9_.-] character becomes _), distinct conversation ids collide onto the same file, so a caller can load a document belonging to another conversation/tenant and have it replayed as their own message history.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Implements multi-turn conversation state management for the TypeScript SDK per the approved design RFC (docs/multi-turn-state.md, ratifies merged PR #124):
Test plan
Linear: DEV-127