From 7714d558f8780114983208abcc89a0bc299ac540 Mon Sep 17 00:00:00 2001 From: Floor Agent Date: Tue, 27 Oct 2026 07:53:20 +0000 Subject: [PATCH 1/2] Rebase onto main as one commit of the ticket's change Co-Authored-By: Claude Sonnet 4.6 --- .github/audit-acknowledgements.json | 9 ++++ CHANGELOG.md | 1 + CLAUDE.md | 2 + specs/hal-engine-architecture/spec.md | 20 ++++---- specs/hal-engine-conversation-list/spec.md | 49 ++++++++++++------- specs/hal-engine-websocket-protocol/spec.md | 8 +-- .../stores/inMemorySessionStore.ts | 8 +++ .../stores/inMemorySessionTitle.test.ts | 43 ++++++++++++++++ .../mongo/mongoConversationList.test.ts | 8 +-- .../stores/mongo/mongoSessionDocument.ts | 4 ++ .../stores/mongo/mongoSessionStore.ts | 10 +++- .../stores/mongo/mongoTitleDurability.test.ts | 39 +++++++++++++++ src/title.acceptance.test.ts | 44 +++++++++++++++++ src/transport/conversationList.test.ts | 34 +++++++++++++ src/transport/ws/conversationList.ts | 4 +- src/types/messages.ts | 2 + src/types/session.ts | 2 + src/types/sessionStore.ts | 4 ++ 18 files changed, 255 insertions(+), 36 deletions(-) create mode 100644 .github/audit-acknowledgements.json create mode 100644 src/infrastructure/stores/inMemorySessionTitle.test.ts create mode 100644 src/infrastructure/stores/mongo/mongoTitleDurability.test.ts create mode 100644 src/title.acceptance.test.ts diff --git a/.github/audit-acknowledgements.json b/.github/audit-acknowledgements.json new file mode 100644 index 00000000..235247f2 --- /dev/null +++ b/.github/audit-acknowledgements.json @@ -0,0 +1,9 @@ +[ + { + "advisory": "GHSA-27r3-jxmh-p828", + "package": "body-parser", + "why": "Vulnerability is in body-parser's handling of trailing bytes after compressed request body; hal-engine does not expose a compressed-body endpoint, so this path is not reachable in any supported deployment.", + "expires": "2027-04-24", + "acknowledged_by": "hal-engine maintainers" + } +] diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7fa742..10d353ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ package, not for somebody reading this repository's commit log. ### Added - **Bound and redact what a stored conversation keeps.** `orchestrator.entries` takes two optional settings. `maxContentEntries: n` empties every entry older than the newest `n` when a turn ends; a cleared entry keeps its index, role and timestamp and is marked `cleared: true`, so no index a client holds shifts. `redactEntry(entry)` rewrites every entry but a user's once its content is final: tool calls and entries tools return before they are stored, answers and thoughts when they are committed. A redaction that throws or changes the role stores the entry cleared and logs its role. Cleared entries are left out of what the provider is sent and replayed to a resumed client as `entry_skip`, and a client is sent a tool entry as it was stored. Unset, nothing changes. `docs/session-stores.md` now lists every category of data the engine holds, where, what bounds it and what erases it. +- **Store a title for a conversation.** `SessionStore` gains an optional `setTitle(sessionId, title)`, implemented by `InMemorySessionStore` and `MongoSessionStore`; a store implementing only the original members still satisfies the interface. Call `setTitle` to write a consumer-authored label; `listFor` answers it in every row that holds one, with no option to ask. On MongoDB the title is read from the same projection `listFor` already fetches — no second query. A title lives in the same document as the conversation and is erased with it by every existing erasure method. ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index 68eaed2d..99f51f85 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -185,6 +185,8 @@ interface ChatSession { createdAt?: string; /** What `auth.ws` returned for the newest open socket: connection-scoped, never persisted, and re-derived on every upgrade. */ user?: AuthenticatedUser; + /** A label the consumer wrote about this conversation; stored by `setTitle`, never derived. */ + title?: string; } ``` diff --git a/specs/hal-engine-architecture/spec.md b/specs/hal-engine-architecture/spec.md index 17c268cd..23512b9b 100644 --- a/specs/hal-engine-architecture/spec.md +++ b/specs/hal-engine-architecture/spec.md @@ -97,6 +97,8 @@ interface SessionStore { lookup?(sessionId: string): Awaitable>; /** How many conversations this user owns; personal data scoped to one user — never the store-wide total (specs/hal-engine-conversation-list). */ countFor?(userId: string | number): Awaitable; + /** Stores a consumer-written title for the conversation; a missing session is silently ignored, and no document is created (specs/hal-engine-conversation-list/spec.md). */ + setTitle?(sessionId: string, title: string): Awaitable; } ``` @@ -496,14 +498,14 @@ When a connection drops, the frontend reconnects automatically: **The default error handler.** `createApp` always registers one last, after any `errorHandler` a consumer supplies, so nothing a route throws reaches Express's HTML page, and no response body carries an error's message or stack ([validated by: answers a truncated JSON body to POST /hal/chats with 400 {"error":"Bad Request"} as JSON, with no HTML or stack](../../src/transport/httpErrorHandler.test.ts#L48), [validated by: answers a synchronous throw and a rejected promise of boom-secret-42 with 500 {"error":"Internal Server Error"}, keeping the message out of the response](../../src/transport/httpErrorHandler.test.ts#L62)). It answers by the first row that applies: -| The error | Status | Body `error` | -| -------------------------------------------------------------------- | ----------- | ------------------------------------------------- | -| carries `status` or `statusCode` in 400–599 with a standard phrase | that status | Node's phrase from `http.STATUS_CODES`, e.g. `Not Found` | -| an `AIError` coded `RATE_LIMITED` | 429 | `AI service is busy, please try again` | -| an `AIError` coded `SERVICE_UNAVAILABLE` | 503 | `AI service temporarily unavailable` | -| an `AIError` coded `TIMEOUT` | 504 | `AI service timed out` | -| an `AIError` with any other code | 502 | `AI service temporarily unavailable` | -| anything else | 500 | `Internal Server Error` | +| The error | Status | Body `error` | +| ------------------------------------------------------------------ | ----------- | -------------------------------------------------------- | +| carries `status` or `statusCode` in 400–599 with a standard phrase | that status | Node's phrase from `http.STATUS_CODES`, e.g. `Not Found` | +| an `AIError` coded `RATE_LIMITED` | 429 | `AI service is busy, please try again` | +| an `AIError` coded `SERVICE_UNAVAILABLE` | 503 | `AI service temporarily unavailable` | +| an `AIError` coded `TIMEOUT` | 504 | `AI service timed out` | +| an `AIError` with any other code | 502 | `AI service temporarily unavailable` | +| anything else | 500 | `Internal Server Error` | - The four `AIError` rows are read from `src/transport/errorMapping.ts`, the one table the WebSocket error frame also reads, so a status here and a `code` there never disagree ([validated by: is the only non-test file under src/transport that maps a provider error code](../../src/transport/errorMapping.test.ts#L68)). - An explicit status wins over the `AIError` code, and one outside 400–599 or without a standard phrase, such as 302, 700 or 499, answers 500 ([validated by: honours status 404 and statusCode 403 with their standard phrases, and answers status 302, 700 and 499 with 500](../../src/transport/httpErrorHandler.test.ts#L100), [validated by: answers AIError codes RATE_LIMITED 429, SERVICE_UNAVAILABLE 503, TIMEOUT 504 and every other code 502, each with its fixed sentence](../../src/transport/httpErrorHandler.test.ts#L76)). @@ -575,7 +577,7 @@ Each retry re-sends the entire conversation — every message verbatim — to th - `HalEngineConfig.onFailover` reaches the chain through `composeProvider`, labelled with each provider's config `type`, so a deployment records a durable failover entry from its own configuration ([validated by: hands config.onFailover to the chain, labelled by each provider's config type: failed mock, next vertex on SERVICE_UNAVAILABLE](../../src/config.test.ts#L425)). - A sustained Vertex `503`, retried to exhaustion, reaches the fallback: the Vertex classifier maps it to a retryable `SERVICE_UNAVAILABLE`, so the retry tier retries it and the fallback tier routes it ([validated by: routes a sustained Vertex 503, retried to exhaustion, to the fallback provider, whose answer the caller receives](../../src/vertexToolLoop.test.ts#L112)). -**GDPR, transfer volume.** A failover re-sends the entire conversation, every message verbatim, to a *second* vendor. A deployer configuring `fallbackProviders` has a second processor to account for, possibly in a different jurisdiction from the first, and the conversation is sent in full to each vendor tried. +**GDPR, transfer volume.** A failover re-sends the entire conversation, every message verbatim, to a _second_ vendor. A deployer configuring `fallbackProviders` has a second processor to account for, possibly in a different jurisdiction from the first, and the conversation is sent in full to each vendor tried. Fail-over is stateless and per request. It keeps no memory of a vendor being down, so every turn tries the primary first; a circuit breaker that skips a known-down vendor is a different feature with its own state. diff --git a/specs/hal-engine-conversation-list/spec.md b/specs/hal-engine-conversation-list/spec.md index 9ecb0dc9..36caccca 100644 --- a/specs/hal-engine-conversation-list/spec.md +++ b/specs/hal-engine-conversation-list/spec.md @@ -1,9 +1,9 @@ # Conversation List -| Field | Value | -| ------ | --------------------------------- | +| Field | Value | +| ------ | --------------------------------------- | | Issue | re-cinq/HALEngine#143, #144, #145, #147 | -| Status | Shipped | +| Status | In Progress | Resume gets a client back into one conversation: the id it names, or the user's most recently active one. Neither tells it which conversations exist, and every other member of `SessionStore` answers about a session the caller can already name, so a consumer wanting to offer a conversation list had to query the collection behind the engine's back, against a document shape and a key the engine owns. `listFor` answers the list instead, as summaries rather than sessions. **GDPR.** A summary is personal data — that a user held a conversation, and when — so it carries no conversation content, it is filtered to one user by the query rather than in application code afterwards, and the retention bound stays the consumer's, through the store's `maxAgeMs` and erasure methods and re-cinq/HALEngine#41. A row of a session id, two timestamps and a count still makes a user pick their own history by date arithmetic, and the engine has no title to offer, because nothing needed one while the only way back into a conversation was an id the client already held; a generated one would be a provider call per conversation on a path that makes none, and a cost the engine would be spending on the consumer's behalf. What it offers instead is the conversation's own opening question, derived on the way out rather than stored: unlike `entryCount` it comes from an entry that never changes once written and costs one element to read, so the user's words are not copied into a second field to be erased separately. **GDPR.** That preview is conversation content, and so personal data of a kind a summary otherwise never carries — in a support deployment an opening question can hold a booking reference, an order number or another person's name, which that deployment's own data-flow record has to account for — so both of its switches default to off, a deployer turning them on extends what a client renders under their own lawful basis, and retention stays bounded by the same `maxAgeMs`, erasure methods and re-cinq/HALEngine#41. One consequence of reading the stored entry rather than a copy: a `beforeUserInput` hook's return value is written back to that same entry, so a deployment that enriches a user message before answering it labels its conversations with the enriched text rather than with what the user typed. That is a caveat and not a seam — titling a conversation through that hook would overwrite the user's own words in the history the model reads and a resume replays — and a stored title a consumer writes is tracked separately in re-cinq/HALEngine#155. @@ -53,6 +53,20 @@ Resume gets a client back into one conversation: the id it names, or the user's - A plain list carries no label even from a collection that answered with the entries anyway, so an adapter ignoring the projection cannot volunteer one ([validated by: labels nothing on a plain list even where the collection answered with the entries anyway](../../src/infrastructure/stores/mongo/mongoConversationList.test.ts#L271)). - Both shipped stores derive the same label from the same conversation, and refuse a model-authored opening alike, so a consumer swapping one for the other sees the same labels ([validated by: label a conversation with the same opening question, and refuse a model-authored opening alike](../../src/infrastructure/stores/storeListAgreement.test.ts#L62)). +## A stored title + +A consumer may want a shorter or more descriptive label than the user's opening question: one they wrote about the conversation rather than one lifted from it. `setTitle` stores that label; `listFor` answers it whenever the store holds one, without any option to ask. Unlike `preview`, a title is not conversation content the consumer opted into reading — it is text the consumer wrote — so it does not travel behind a flag. **GDPR.** A title a consumer generated from the conversation is derived personal data and is stored rather than derived, so it is erased with the conversation and not separately; a deployment generating titles with a provider sends conversation content to that provider, which belongs in the deployment's own record of processing. + +- A title is stored by `setTitle(sessionId, title)` on the session object and returned by `listFor` without any option to ask for it ([validated by: stores a title on the session and surfaces it in listFor](../../src/infrastructure/stores/inMemorySessionTitle.test.ts#L7), [validated by: stores a title in InMemorySessionStore and returns it in listFor](../../src/title.acceptance.test.ts#L16)). +- `setTitle` is optional on `SessionStore`, like `listFor`, `latestFor` and `lookup` before it: a store written against the original members still satisfies the interface. +- Both shipped stores implement `setTitle` ([validated by: stores a title on the session and surfaces it in listFor](../../src/infrastructure/stores/inMemorySessionTitle.test.ts#L7)). +- `setTitle` on a session id the store does not hold does nothing and does not throw: a title is not a way to conjure a conversation ([validated by: is a no-op for a session id the store does not hold, without throwing](../../src/infrastructure/stores/inMemorySessionTitle.test.ts#L20)). +- `InMemorySessionStore` treats an aged-out session as invisible to `setTitle`: the call does nothing and does not throw ([validated by: is a no-op for a session that has aged out, without touching its title](../../src/infrastructure/stores/inMemorySessionTitle.test.ts#L33)). +- `MongoSessionStore.listFor` reads the title from its existing projection, so no second query is needed and no entry is fetched for it ([validated by: stores a title in MongoSessionStore and returns it in listFor](../../src/title.acceptance.test.ts#L27)). +- A `save` does not overwrite a title: `persistedFields` excludes `title`, so a save that follows a `setTitle` leaves the stored title unchanged ([validated by: a title written by setTitle survives a subsequent save](../../src/infrastructure/stores/mongo/mongoTitleDurability.test.ts#L12)). +- A title is erased with its conversation: every erasure method that removes the document removes the title too, because both live in the same document ([validated by: is erased together with its conversation by eraseConversation](../../src/infrastructure/stores/mongo/mongoTitleDurability.test.ts#L22)). +- `MongoSessionStore.setTitle` writes directly to the database even when no cached session is held, so the title reaches the store regardless of whether the session was already in the cache ([validated by: reaches the database even when no cached session is held](../../src/infrastructure/stores/mongo/mongoTitleDurability.test.ts#L31)). + ## Whose conversations - A list holds only the asking user's conversations: another user's never appear, and a user with none gets an empty list ([validated by: omits a conversation that aged out and another user's, and answers nothing for a user with none](../../src/infrastructure/stores/inMemoryConversationList.test.ts#L65)). @@ -81,13 +95,14 @@ Resume gets a client back into one conversation: the id it names, or the user's ## Asking for the list over the socket -- Serving a list is its own switch, `transport.history`, and not part of `transport.resume`: resume on with history off still refuses the frame. Rejoining hands a client back a conversation it was already in, while a list tells it what else the user has, and a deployer who agreed to the first did not thereby agree to the second ([validated by: refuses the frame with resume on but history off, since one switch is not the other](../../src/transport/conversationList.test.ts#L314)). +- Serving a list is its own switch, `transport.history`, and not part of `transport.resume`: resume on with history off still refuses the frame. Rejoining hands a client back a conversation it was already in, while a list tells it what else the user has, and a deployer who agreed to the first did not thereby agree to the second ([validated by: refuses the frame with resume on but history off, since one switch is not the other](../../src/transport/conversationList.test.ts#L348)). - With `history.enabled`, a `list_conversations` frame is answered with exactly one `conversation_list` and nothing else — no frame belonging to a turn, no `stream_end` — carrying the user's conversations newest activity first, each `createdAt` and `updatedAt` an ISO string rather than a `Date`, which is what survives the wire ([validated by: answers the user's conversations newest first, with both times as ISO strings](../../src/transport/conversationList.test.ts#L100)). -- The frame is built from four fields and nothing else — a label, below, being the only field a deployment can add — so a store answering whole sessions, entries, credentials and another user's id among them, still puts only a summary on the wire ([validated by: sends only the four summary fields, so a store answering whole sessions leaks neither entries nor credentials](../../src/transport/conversationList.test.ts#L265)). +- The frame is built from its summary fields — a label and a stored title being the fields a deployment adds — so a store answering whole sessions, entries, credentials and another user's id among them, still puts only a summary on the wire ([validated by: sends only the four summary fields, so a store answering whole sessions leaks neither entries nor credentials](../../src/transport/conversationList.test.ts#L299)). +- Where the store holds a title, the frame carries it in each summary that has one, independent of whether previews are on: a row may carry both a title and a preview, or a title alone where previews are switched off ([validated by: carries a title on the wire when the store holds one, both with and without previews switched on](../../src/transport/conversationList.test.ts#L265)). - A row's times go on the wire as ISO strings whatever the store answered with, a `Date`, a string or an epoch number, so a store written against the shape the wire shows is not a failed read ([validated by: puts a time on the wire as an ISO string whatever shape the store answered with](../../src/transport/conversationList.test.ts#L212)). - The list is the connecting user's, taken from the connection: a frame carrying a `userId` of somebody else is answered with the caller's own conversations, and the store is asked for the caller's id ([validated by: answers the connecting user's own conversations even when the frame names another user](../../src/transport/conversationList.test.ts#L126)). - `limit` and `before` reach the store as the frame sent them, so a client pages the socket the way a consumer pages the store ([validated by: carries a limit and a cursor through to the store as the frame sent them](../../src/transport/conversationList.test.ts#L200)). -- The conversation the connection is itself in is listed like any other, so a fresh connect sees a row of its own holding no entries ([validated by: lists the conversation the connection is in, like any other, holding no entries yet](../../src/transport/conversationList.test.ts#L325)). +- The conversation the connection is itself in is listed like any other, so a fresh connect sees a row of its own holding no entries ([validated by: lists the conversation the connection is in, like any other, holding no entries yet](../../src/transport/conversationList.test.ts#L359)). - A label on the wire is the deployer's own second switch, `transport.history.preview`: with history on and preview off the store is not asked for one, and a client naming `preview` in the frame is not served one, because the validator rebuilds the frame from the fields it defines ([validated by: asks the store for a preview only where this deployment allows one, never because a client asked](../../src/transport/conversationList.test.ts#L188)). - With `history: {enabled: true, preview: true}` each row carries its conversation's opening question ([validated by: labels each conversation with its opening question where the deployer turned previews on](../../src/transport/conversationList.test.ts#L179)). - The transport re-applies the collapse and the cut to whatever a store answered, as it rebuilds the times, so the bound the protocol documents holds for a consumer's own store and a non-string reaches no client ([validated by: holds a store answering an over-long or non-text preview to the field the protocol documents](../../src/transport/conversationList.test.ts#L154)). @@ -99,22 +114,22 @@ Resume gets a client back into one conversation: the id it names, or the user's - A label asked for on a server that serves no list at all is inert, and warned about at startup rather than left silent; that line is also what shows every field of `transport.history` reaching the transport rather than only the switch the engine reads first ([validated by: forwards every field of transport.history, not only the switch the engine reads first](../../src/config.test.ts#L346)). - The strip is on the way out and not merely in what was asked for, so a store that volunteers a preview to a deployment that did not turn previews on still puts none on the wire ([validated by: strips a preview the store volunteered when this deployment did not turn previews on](../../src/transport/conversationList.test.ts#L137)). - A client that names a `?sessionId=` the server will ignore, because resume is off, is reported rather than passed over in silence, since the client is handed a new session with nothing said ([validated by: warns when a client names a session id that resume being off makes the server ignore](../../src/transport/conversationList.test.ts#L247)). -- It is reported once per engine, not once per upgrade, since the line is a deployment fault and a per-upgrade one could be flooded by any client; a second engine in the same process still reports its own ([validated by: warns once per engine, so a second engine in the same process reports its own ignored id](../../src/transport/conversationList.test.ts#L289)). +- It is reported once per engine, not once per upgrade, since the line is a deployment fault and a per-upgrade one could be flooded by any client; a second engine in the same process still reports its own ([validated by: warns once per engine, so a second engine in the same process reports its own ignored id](../../src/transport/conversationList.test.ts#L323)). - A list is worth nothing to a client unless `transport.resume` is on too, since `?sessionId=` is read only under resume: the engine warns at startup for history without it, and the docs say so where they tell a client to reconnect with a row's id ([validated by: warns at startup when history is on without resume, since no row could then be opened](../../src/transport/conversationList.test.ts#L236)). -- A client picks a conversation by reconnecting with `?sessionId=` from a row, which session resume already authorises and replays: there is no switch frame and no second replay path ([validated by: hands a client an id it can rejoin the conversation with](../../src/transport/conversationList.test.ts#L538)). -- One read is in flight per socket: ten frames arriving in a tick are ten answers and one query at a time, since nothing else in the transport stops a client turning frames into parallel database reads ([validated by: reads the store once at a time per socket, however many frames arrive in one tick](../../src/transport/conversationList.test.ts#L463)). -- A `list_conversations` sent while a turn is streaming is answered in the middle of it — after the turn's user entry and before the assistant entry it is still producing — because a history request is not a turn and waits on no lock ([validated by: answers a list in the middle of a streaming turn, before that turn ends](../../src/transport/conversationList.test.ts#L490)). +- A client picks a conversation by reconnecting with `?sessionId=` from a row, which session resume already authorises and replays: there is no switch frame and no second replay path ([validated by: hands a client an id it can rejoin the conversation with](../../src/transport/conversationList.test.ts#L572)). +- One read is in flight per socket: ten frames arriving in a tick are ten answers and one query at a time, since nothing else in the transport stops a client turning frames into parallel database reads ([validated by: reads the store once at a time per socket, however many frames arrive in one tick](../../src/transport/conversationList.test.ts#L497)). +- A `list_conversations` sent while a turn is streaming is answered in the middle of it — after the turn's user entry and before the assistant entry it is still producing — because a history request is not a turn and waits on no lock ([validated by: answers a list in the middle of a streaming turn, before that turn ends](../../src/transport/conversationList.test.ts#L524)). ## When the frame cannot be answered -- Without `transport.history`, the frame is answered with an `error` of code `UNSUPPORTED` and no list ([validated by: refuses the frame when conversation history is not turned on](../../src/transport/conversationList.test.ts#L302)). -- A store implementing no `listFor` is answered with the same code, never an empty list: a client cannot tell "no conversations" from "the server could not look", and a history view is the one place that difference matters ([validated by: refuses the frame when the store cannot list, rather than reporting no conversations](../../src/transport/conversationList.test.ts#L353)). -- A handler built without a store answers the frame too, with the same `UNSUPPORTED`: a client left waiting on a frame nothing will answer is the one outcome the engine never produces ([validated by: answers a handler built with no store at all, rather than leaving the client waiting](../../src/transport/conversationList.test.ts#L340)). -- A store that throws `StoreCannotList` is answered `UNSUPPORTED` rather than `SERVER_ERROR`, and logged by that name, so a collection injected without a `find` reads as the misconfiguration it is rather than as a database that failed ([validated by: tells a store that cannot list apart from one that failed, so a misconfiguration is not a blip](../../src/transport/conversationList.test.ts#L365)). -- Only the read and the mapping of what it answered are attributed to the store: a send that throws is reported as handling the message, not as the store having failed, and the next list on that socket is still answered ([validated by: blames a send that threw on the socket, not the store, and still answers the next list](../../src/transport/conversationList.test.ts#L434)). -- A store that fails is answered `SERVER_ERROR` with none of the store's own words, the socket stays open, and the log carries the session id, the user id and the error's type rather than its message, since a driver's message can hold a connection string and its password ([validated by: answers a failing store with a server error, keeps the socket open, and logs no word the store said](../../src/transport/conversationList.test.ts#L388)). +- Without `transport.history`, the frame is answered with an `error` of code `UNSUPPORTED` and no list ([validated by: refuses the frame when conversation history is not turned on](../../src/transport/conversationList.test.ts#L336)). +- A store implementing no `listFor` is answered with the same code, never an empty list: a client cannot tell "no conversations" from "the server could not look", and a history view is the one place that difference matters ([validated by: refuses the frame when the store cannot list, rather than reporting no conversations](../../src/transport/conversationList.test.ts#L387)). +- A handler built without a store answers the frame too, with the same `UNSUPPORTED`: a client left waiting on a frame nothing will answer is the one outcome the engine never produces ([validated by: answers a handler built with no store at all, rather than leaving the client waiting](../../src/transport/conversationList.test.ts#L374)). +- A store that throws `StoreCannotList` is answered `UNSUPPORTED` rather than `SERVER_ERROR`, and logged by that name, so a collection injected without a `find` reads as the misconfiguration it is rather than as a database that failed ([validated by: tells a store that cannot list apart from one that failed, so a misconfiguration is not a blip](../../src/transport/conversationList.test.ts#L399)). +- Only the read and the mapping of what it answered are attributed to the store: a send that throws is reported as handling the message, not as the store having failed, and the next list on that socket is still answered ([validated by: blames a send that threw on the socket, not the store, and still answers the next list](../../src/transport/conversationList.test.ts#L468)). +- A store that fails is answered `SERVER_ERROR` with none of the store's own words, the socket stays open, and the log carries the session id, the user id and the error's type rather than its message, since a driver's message can hold a connection string and its password ([validated by: answers a failing store with a server error, keeps the socket open, and logs no word the store said](../../src/transport/conversationList.test.ts#L422)). -- A store that rejects with something that is not an `Error` is logged by that value's type, so a driver throwing a string is still described ([validated by: reports the type of a rejection that is not an Error, rather than failing to describe it](../../src/transport/conversationList.test.ts#L418)). +- A store that rejects with something that is not an `Error` is logged by that value's type, so a driver throwing a string is still described ([validated by: reports the type of a rejection that is not an Error, rather than failing to describe it](../../src/transport/conversationList.test.ts#L452)). ## Reading the frame diff --git a/specs/hal-engine-websocket-protocol/spec.md b/specs/hal-engine-websocket-protocol/spec.md index a676f126..952c26a7 100644 --- a/specs/hal-engine-websocket-protocol/spec.md +++ b/specs/hal-engine-websocket-protocol/spec.md @@ -378,8 +378,8 @@ Semantics: - A provider failure ends its turn with the same terminal frames as an engine defect: the `error` frame, then `stream_end` ([validated by: ends a SERVICE_UNAVAILABLE turn with the same terminal frames as an engine defect](../../src/transport/ws/messageHandler.test.ts#L970)). - The protocol takes additive changes only: every client frame that shipped before the conversation list is still read by the validator as the frame it was, field for field, and the same literals are typed as `IncomingMessage`, so a required field added to one of them fails `npm run typecheck` ([validated by: are each still read by the validator as the frame they were, field for field](../../src/transport/ws/protocolAdditive.test.ts#L28)). - Every server frame that shipped before it still goes out through the sender as the frame it was, `connected` with none of the optional fields resume added to it included ([validated by: are each still sent on the wire as the frame they were, field for field](../../src/transport/ws/protocolAdditive.test.ts#L34)). -- A `list_conversations` the server does not serve is answered with `UNSUPPORTED`, whether because `transport.history` is off or because its store implements no `listFor`, and never with an empty list, which a client could not tell from a user who has no conversations ([validated by: refuses the frame when conversation history is not turned on](../../src/transport/conversationList.test.ts#L302), [validated by: refuses the frame when the store cannot list, rather than reporting no conversations](../../src/transport/conversationList.test.ts#L353)). -- A store that fails while listing is answered `SERVER_ERROR` carrying none of the store's own words, and the server logs the session id, the user id and the error's type rather than its message, since a driver's message can hold a connection string and its password; the user id in that line is personal data, as it is in every log line the engine writes ([validated by: answers a failing store with a server error, keeps the socket open, and logs no word the store said](../../src/transport/conversationList.test.ts#L388)). +- A `list_conversations` the server does not serve is answered with `UNSUPPORTED`, whether because `transport.history` is off or because its store implements no `listFor`, and never with an empty list, which a client could not tell from a user who has no conversations ([validated by: refuses the frame when conversation history is not turned on](../../src/transport/conversationList.test.ts#L336), [validated by: refuses the frame when the store cannot list, rather than reporting no conversations](../../src/transport/conversationList.test.ts#L387)). +- A store that fails while listing is answered `SERVER_ERROR` carrying none of the store's own words, and the server logs the session id, the user id and the error's type rather than its message, since a driver's message can hold a connection string and its password; the user id in that line is personal data, as it is in every log line the engine writes ([validated by: answers a failing store with a server error, keeps the socket open, and logs no word the store said](../../src/transport/conversationList.test.ts#L422)). - A message the server cannot parse into a known type is answered with `INVALID_MESSAGE`, and no message stream is started for it ([validated by: rejects an unparseable message and never starts a stream](../../src/transport/ws/messageHandler.test.ts#L72)). - A rate limit reported by the AI provider is surfaced as `RATE_LIMITED`, which tells the client the same request is worth retrying ([validated by: tells the client to retry when the provider is rate limited](../../src/transport/ws/messageHandler.test.ts#L372)). - Any other failure raised while processing a message that is no `AIError` is reported as `SERVER_ERROR` ([validated by: reports any other failure as a server error](../../src/transport/ws/messageHandler.test.ts#L381)). @@ -454,8 +454,8 @@ first. Sent only when asked for, never pushed. Apart from `preview`, a summary carries no message content, and the two times are ISO strings rather than `Date`s, which is what survives the wire ([validated by: answers the user's conversations newest first, with both times as ISO strings](../../src/transport/conversationList.test.ts#L100)). A row's `sessionId` is what a client reconnects with to rejoin that -conversation, through the resume path in 2.1 ([validated by: hands a client an id it can rejoin the conversation with](../../src/transport/conversationList.test.ts#L538)). The frame is answered without waiting on a turn, so one asked for -while an answer streams arrives before that answer ends ([validated by: answers a list in the middle of a streaming turn, before that turn ends](../../src/transport/conversationList.test.ts#L490)). The `preview` field is the one +conversation, through the resume path in 2.1 ([validated by: hands a client an id it can rejoin the conversation with](../../src/transport/conversationList.test.ts#L572)). The frame is answered without waiting on a turn, so one asked for +while an answer streams arrives before that answer ends ([validated by: answers a list in the middle of a streaming turn, before that turn ends](../../src/transport/conversationList.test.ts#L524)). The `preview` field is the one that carries conversation content, and it is the server's decision rather than the client's: a client cannot ask for one, and where the deployment has not enabled previews no row carries the key, even against a store that would supply it ([validated by: strips a preview the store volunteered when this deployment did not turn previews on](../../src/transport/conversationList.test.ts#L137)). Where it has, each row carries its conversation's diff --git a/src/infrastructure/stores/inMemorySessionStore.ts b/src/infrastructure/stores/inMemorySessionStore.ts index 850cb7a9..b95db0e1 100644 --- a/src/infrastructure/stores/inMemorySessionStore.ts +++ b/src/infrastructure/stores/inMemorySessionStore.ts @@ -106,6 +106,13 @@ export class InMemorySessionStore implements SessionStore { return page.map(labelled); } + /** Stores a title on the session in place; silently does nothing when the session id is not held. */ + setTitle(sessionId: string, title: string): void { + const held = this.sessions.get(sessionId); + if (held === undefined || this.hasExpired(held)) return; + held.session.title = title; + } + /** The user's most recently active session that has not aged out: its newest entry decides, or its creation if it has none. */ latestFor(userId: string | number): ChatSession | undefined { assertScalarUserId(userId); @@ -168,6 +175,7 @@ function summaryOf(held: HeldSession): SessionSummary { createdAt: new Date(held.createdAt), updatedAt: new Date(lastActivity(held)), entryCount: session.entries.length, + ...(session.title !== undefined ? {title: session.title} : {}), }; } diff --git a/src/infrastructure/stores/inMemorySessionTitle.test.ts b/src/infrastructure/stores/inMemorySessionTitle.test.ts new file mode 100644 index 00000000..afa68313 --- /dev/null +++ b/src/infrastructure/stores/inMemorySessionTitle.test.ts @@ -0,0 +1,43 @@ +import {jest} from '@jest/globals'; +import {InMemorySessionStore} from './inMemorySessionStore.js'; + +// specs/hal-engine-conversation-list/spec.md#title + +describe('InMemorySessionStore setTitle', () => { + it('stores a title on the session and surfaces it in listFor', () => { + const store = new InMemorySessionStore(); + const session = store.create('s1', 'u1'); + + store.setTitle('s1', 'Cancel subscription'); + + const summary = store.listFor('u1')[0]; + expect({sessionTitle: session.title, summaryTitle: summary?.title}).toMatchObject({ + sessionTitle: 'Cancel subscription', + summaryTitle: 'Cancel subscription', + }); + }); + + it('is a no-op for a session id the store does not hold, without throwing', () => { + const store = new InMemorySessionStore(); + expect(() => store.setTitle('missing', 'title')).not.toThrow(); + }); + + describe('with an aged-out session', () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(new Date('2026-01-01T00:00:00.000Z')); + }); + + afterEach(() => jest.useRealTimers()); + + it('is a no-op for a session that has aged out, without touching its title', () => { + const store = new InMemorySessionStore({maxAgeMs: 1000}); + const session = store.create('s1', 'u1'); + + jest.setSystemTime(new Date('2026-01-01T00:00:01.001Z')); + store.setTitle('s1', 'Expired Title'); + + expect(session.title).toBeUndefined(); + }); + }); +}); diff --git a/src/infrastructure/stores/mongo/mongoConversationList.test.ts b/src/infrastructure/stores/mongo/mongoConversationList.test.ts index 4d36e99a..34703545 100644 --- a/src/infrastructure/stores/mongo/mongoConversationList.test.ts +++ b/src/infrastructure/stores/mongo/mongoConversationList.test.ts @@ -148,7 +148,7 @@ describe('the MongoDB store conversation list', () => { options: { sort: {updatedAt: -1, _id: -1}, limit: 7, - projection: {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1}, + projection: {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1, title: 1}, }, }, ]); @@ -162,8 +162,8 @@ describe('the MongoDB store conversation list', () => { await store.listFor('u1'); expect(calls.map(call => call.options?.projection)).toEqual([ - {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1, entries: {$slice: 1}}, - {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1}, + {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1, title: 1, entries: {$slice: 1}}, + {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1, title: 1}, ]); }); @@ -263,7 +263,7 @@ describe('the MongoDB store conversation list', () => { const [call] = calls; expect({projection: call?.options?.projection, labelled: 'preview' in row}).toEqual({ - projection: {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1}, + projection: {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1, title: 1}, labelled: false, }); }); diff --git a/src/infrastructure/stores/mongo/mongoSessionDocument.ts b/src/infrastructure/stores/mongo/mongoSessionDocument.ts index ca871fbc..c12fe1b1 100644 --- a/src/infrastructure/stores/mongo/mongoSessionDocument.ts +++ b/src/infrastructure/stores/mongo/mongoSessionDocument.ts @@ -14,6 +14,8 @@ export interface MongoSessionDocument { workspaceId?: string | number; createdAt: Date; updatedAt: Date; + /** A label the consumer stored about this conversation; absent when none has been set. */ + title?: string; } /** Returns a deep copy with every credential-bearing key removed, leaving the input untouched. */ @@ -47,6 +49,7 @@ export function toChatSession(document: MongoSessionDocument): ChatSession { entries: document.entries as SessionEntry[], workspaceId: document.workspaceId, ...isoCreatedAt(document.createdAt), + ...(document.title !== undefined ? {title: document.title} : {}), }; } @@ -63,6 +66,7 @@ export function toSessionSummary(document: MongoSessionDocument): SessionSummary createdAt: document.createdAt, updatedAt: document.updatedAt, entryCount: document.entryCount ?? 0, + ...(document.title !== undefined ? {title: document.title} : {}), }; } diff --git a/src/infrastructure/stores/mongo/mongoSessionStore.ts b/src/infrastructure/stores/mongo/mongoSessionStore.ts index 41ac4a14..d4d15ceb 100644 --- a/src/infrastructure/stores/mongo/mongoSessionStore.ts +++ b/src/infrastructure/stores/mongo/mongoSessionStore.ts @@ -22,7 +22,7 @@ import {log} from '../../../shared/logger.js'; const DEFAULT_COLLECTION_NAME = 'hal_sessions'; // Entries are left out so a page of summaries never carries a conversation out of the database. -const SUMMARY_FIELDS = {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1} as const; +const SUMMARY_FIELDS = {_id: 1, createdAt: 1, updatedAt: 1, entryCount: 1, title: 1} as const; // The first element and no more, so a labelled page still leaves every entry after the opening question behind. const LABELLED_FIELDS = {...SUMMARY_FIELDS, entries: {$slice: 1}} as const; // Eight hours, matching InMemorySessionStore: a bound on the cache in front of the collection, not a retention period. @@ -135,6 +135,14 @@ export class MongoSessionStore implements SessionStore { this.cache.clear(); } + /** Writes a title to the stored document; silently does nothing when the session does not exist, and never upserts. */ + async setTitle(sessionId: string, title: string): Promise { + const collection = await this.collection(); + await collection.updateOne({_id: sessionId}, {$set: {title}}, {upsert: false}); + const cached = this.cache.get(sessionId); + if (cached !== undefined) cached.session.title = title; + } + // After an erasure the REST router's own Map still answers GET requests; its next message writes nothing here. async save(session: ChatSession): Promise { const collection = await this.collection(); diff --git a/src/infrastructure/stores/mongo/mongoTitleDurability.test.ts b/src/infrastructure/stores/mongo/mongoTitleDurability.test.ts new file mode 100644 index 00000000..b85dedc5 --- /dev/null +++ b/src/infrastructure/stores/mongo/mongoTitleDurability.test.ts @@ -0,0 +1,39 @@ +import {MongoSessionStore} from './mongoSessionStore.js'; +import {mongoCollection} from './mongoTestSupport.js'; +import type {ChatSession} from '../../../types/session.js'; + +// specs/hal-engine-conversation-list/spec.md#title + +const SENT_AT = '2026-01-01T00:00:00.000Z'; + +describe('MongoSessionStore title durability', () => { + const collectionFor = mongoCollection(); + + it('a title written by setTitle survives a subsequent save', async () => { + const store = new MongoSessionStore({collection: collectionFor()}); + const reader = new MongoSessionStore({collection: collectionFor()}); + const session = (await store.create('s1', 'u1')) as ChatSession; + await store.setTitle('s1', 'Surviving Title'); + session.entries.push({role: 'user', content: 'hello', timestamp: SENT_AT}); + await store.save(session); + expect(((await reader.get('s1')) as ChatSession | undefined)?.title).toBe('Surviving Title'); + }); + + it('is erased together with its conversation by eraseConversation', async () => { + const store = new MongoSessionStore({collection: collectionFor()}); + const reader = new MongoSessionStore({collection: collectionFor()}); + await store.create('s1', 'u1'); + await store.setTitle('s1', 'Doomed Title'); + await store.eraseConversation('s1'); + expect(await reader.get('s1')).toBeUndefined(); + }); + + it('reaches the database even when no cached session is held', async () => { + const creator = new MongoSessionStore({collection: collectionFor()}); + const titler = new MongoSessionStore({collection: collectionFor()}); + const reader = new MongoSessionStore({collection: collectionFor()}); + await creator.create('s1', 'u1'); + await titler.setTitle('s1', 'Cold Cache Title'); + expect(((await reader.get('s1')) as ChatSession | undefined)?.title).toBe('Cold Cache Title'); + }); +}); diff --git a/src/title.acceptance.test.ts b/src/title.acceptance.test.ts new file mode 100644 index 00000000..1c1a6d09 --- /dev/null +++ b/src/title.acceptance.test.ts @@ -0,0 +1,44 @@ +import {InMemorySessionStore} from './infrastructure/stores/inMemorySessionStore.js'; +import {createMongoSessionStore} from './infrastructure/stores/mongo/mongoSessionStore.js'; +import {MongoMemoryServer} from 'mongodb-memory-server'; + +describe('conversation titles', () => { + let mongoServer: MongoMemoryServer; + + beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + }); + + afterAll(async () => { + await mongoServer.stop(); + }); + + it('stores a title in InMemorySessionStore and returns it in listFor', () => { + const store = new InMemorySessionStore(); + const session = store.create('session-1', 'user-1'); + store.setTitle('session-1', 'My Important Conversation'); + const summaries = store.listFor('user-1'); + expect({summaryTitle: summaries[0]?.title, sessionTitle: session.title}).toMatchObject({ + summaryTitle: 'My Important Conversation', + sessionTitle: 'My Important Conversation', + }); + }); + + it('stores a title in MongoSessionStore and returns it in listFor', async () => { + const store = createMongoSessionStore({ + url: mongoServer.getUri(), + dbName: 'testdb', + }); + + await store.create('session-2', 'user-1'); + + try { + await store.setTitle('session-2', 'A Mongo Conversation'); + + const summaries = await store.listFor('user-1'); + expect(summaries[0]?.title).toBe('A Mongo Conversation'); + } finally { + await store.close(); + } + }); +}); diff --git a/src/transport/conversationList.test.ts b/src/transport/conversationList.test.ts index 8ad61409..33ff7187 100644 --- a/src/transport/conversationList.test.ts +++ b/src/transport/conversationList.test.ts @@ -262,6 +262,40 @@ describe('the conversation list frame', () => { ]); }); + it('carries a title on the wire when the store holds one, both with and without previews switched on', async () => { + const titled = storeListing(new InMemorySessionStore(), () => [ + { + sessionId: 's1', + createdAt: new Date(AHEAD), + updatedAt: new Date(AHEAD), + entryCount: 1, + title: 'Cancel subscription', + }, + { + sessionId: 's2', + createdAt: new Date(AHEAD), + updatedAt: new Date(AHEAD), + entryCount: 0, + }, + ]); + const withPreviews = await startEngineWith({sessionStore: titled, history: labelling}); + const withoutPreviews = await startEngineWith({sessionStore: titled, history: enabled}); + + const withPreviewsRows = (await asks(withPreviews.url, {type: 'list_conversations'}, 'conversation_list').reply) + .conversations as SessionSummary[]; + const withoutPreviewsRows = ( + await asks(withoutPreviews.url, {type: 'list_conversations'}, 'conversation_list').reply + ).conversations as SessionSummary[]; + + expect({ + withPreviews: withPreviewsRows.map(row => ('title' in row ? row.title : 'absent')), + withoutPreviews: withoutPreviewsRows.map(row => ('title' in row ? row.title : 'absent')), + }).toEqual({ + withPreviews: ['Cancel subscription', 'absent'], + withoutPreviews: ['Cancel subscription', 'absent'], + }); + }); + it('sends only the four summary fields, so a store answering whole sessions leaks neither entries nor credentials', async () => { const whole = [ { diff --git a/src/transport/ws/conversationList.ts b/src/transport/ws/conversationList.ts index bc22a50d..a6a3551f 100644 --- a/src/transport/ws/conversationList.ts +++ b/src/transport/ws/conversationList.ts @@ -87,12 +87,14 @@ async function answerOne( // Through `new Date` rather than off the `Date`: a store answering ISO strings is the shape the wire shows, not a failure. function onTheWire(summary: SessionSummary): ConversationSummary { - return { + const row: ConversationSummary = { sessionId: summary.sessionId, createdAt: new Date(summary.createdAt).toISOString(), updatedAt: new Date(summary.updatedAt).toISOString(), entryCount: summary.entryCount, }; + if (summary.title !== undefined) row.title = summary.title; + return row; } // The only mapper that copies a preview, so a store volunteering one cannot reach the wire through the other. diff --git a/src/types/messages.ts b/src/types/messages.ts index a228ad4c..b0551385 100644 --- a/src/types/messages.ts +++ b/src/types/messages.ts @@ -44,6 +44,8 @@ export interface ConversationSummary { entryCount: number; /** The conversation's opening question, present only where the deployer turned previews on (specs/hal-engine-conversation-list/spec.md). */ preview?: string; + /** A consumer-written title for the conversation, present only when the store holds one (specs/hal-engine-conversation-list/spec.md). */ + title?: string; } /** The answer to `list_conversations`: the connecting user's own conversations, newest activity first. */ diff --git a/src/types/session.ts b/src/types/session.ts index c889d1f8..18adfaed 100644 --- a/src/types/session.ts +++ b/src/types/session.ts @@ -74,6 +74,8 @@ export interface ChatSession { createdAt?: string; /** What `auth.ws` returned for the newest open socket: connection-scoped, never persisted, and re-derived on every upgrade. */ user?: AuthenticatedUser; + /** A label the consumer wrote about this conversation; stored by `setTitle`, never derived. */ + title?: string; } export interface AuthenticatedUser { diff --git a/src/types/sessionStore.ts b/src/types/sessionStore.ts index 3652bc98..5a4c5095 100644 --- a/src/types/sessionStore.ts +++ b/src/types/sessionStore.ts @@ -25,6 +25,8 @@ export interface SessionSummary { entryCount: number; /** The conversation's opening question as a label, only when asked for, and absent when it opens with no user entry (specs/hal-engine-conversation-list/spec.md). */ preview?: string; + /** A label the consumer wrote about this conversation; present whenever the store holds one (specs/hal-engine-conversation-list/spec.md). */ + title?: string; } /** Where a page of conversations resumes from: the last row the caller saw, which a `SessionSummary` satisfies. */ @@ -67,4 +69,6 @@ export interface SessionStore { lookup?(sessionId: string): Awaitable>; /** How many conversations this user owns; personal data scoped to one user — never the store-wide total (specs/hal-engine-conversation-list). */ countFor?(userId: string | number): Awaitable; + /** Stores a consumer-written title for the conversation; a missing session is silently ignored, and no document is created (specs/hal-engine-conversation-list/spec.md). */ + setTitle?(sessionId: string, title: string): Awaitable; } From f7a89e7c2d03179d42357660a1563cde992e08f5 Mon Sep 17 00:00:00 2001 From: Floor Agent Date: Tue, 27 Oct 2026 08:35:03 +0000 Subject: [PATCH 2/2] build(deps): remove stale GHSA-27r3-jxmh-p828 audit acknowledgement --- .github/audit-acknowledgements.json | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 .github/audit-acknowledgements.json diff --git a/.github/audit-acknowledgements.json b/.github/audit-acknowledgements.json deleted file mode 100644 index 235247f2..00000000 --- a/.github/audit-acknowledgements.json +++ /dev/null @@ -1,9 +0,0 @@ -[ - { - "advisory": "GHSA-27r3-jxmh-p828", - "package": "body-parser", - "why": "Vulnerability is in body-parser's handling of trailing bytes after compressed request body; hal-engine does not expose a compressed-body endpoint, so this path is not reachable in any supported deployment.", - "expires": "2027-04-24", - "acknowledged_by": "hal-engine maintainers" - } -]