fix(store): derive comment sessionId from its surface

createComment stored the caller's sessionId verbatim instead of
deriving it from the surface the comment attaches to. A comment could
land in a session that doesn't own its surface, breaking listComments
joins and the unread/aggregation logic. The HTTP/MCP flow happened to
pass surface.sessionId, so it was safe today; any future caller of the
Store interface could split them.

Both JsonFileStore and SqlStore now resolve the surface first and
derive sessionId from it, falling back to input.sessionId only when no
surface (or an unknown one) is provided.

Author: benvinegar

.changeset/cold-cars-arrive.md

@@ -0,0 +1,5 @@
+---
+"sideshow": patch
+---
+
+Fix a comment/session mismatch in both stores. `createComment` stored the caller's `sessionId` verbatim instead of deriving it from the surface the comment attaches to. A comment could land in a session that doesn't own its surface, breaking `listComments` joins and the unread/aggregation logic. The HTTP/MCP flow happened to pass `surface.sessionId`, so it was safe today; any future caller of the `Store` interface could split them. Both `JsonFileStore` and `SqlStore` now derive `sessionId` from the surface when one is provided.

server/storage.ts

@@ -347,20 +347,23 @@ export class JsonFileStore implements Store {
 
   async createComment(input: CreateCommentInput) {
     await this.load();
-    if (!this.sessions.has(input.sessionId)) return null;
     const surface = input.surfaceId ? this.surfaces.get(input.surfaceId) : null;
+    // Derive the session from the surface so a comment can never land in a
+    // session that doesn't own its surface.
+    const sessionId = surface ? surface.sessionId : input.sessionId;
+    if (!this.sessions.has(sessionId)) return null;
     const comment: Comment = {
       id: newId(),
       seq: ++this.lastSeq,
-      sessionId: input.sessionId,
+      sessionId,
       surfaceId: surface?.id ?? null,
       surfaceTitle: surface?.title ?? null,
       author: input.author.trim() || "user",
       text: input.text,
       createdAt: new Date().toISOString(),
     };
     this.comments.push(comment);
-    this.touch(input.sessionId);
+    this.touch(sessionId);
     await this.persist();
     return clone(comment);
   }

test/storeContract.ts

@@ -392,6 +392,34 @@ export function runStoreContract(name: string, makeStore: () => Store | Promise<
     assert.equal(ghost?.surfaceId, null);
   });
 
+  contract("a comment's session follows its surface, not the caller's sessionId", async (store) => {
+    // Two sessions; a surface in session A. A comment that passes B's
+    // sessionId but the surface's id must still be filed under A — the
+    // comment's session should always match the surface's session.
+    const a = await store.createSession({ agent: "a" });
+    const b = await store.createSession({ agent: "b" });
+    const surface = await store.createSurface({
+      sessionId: a.id,
+      parts: [htmlPart("<p>x</p>")],
+    });
+    assert.ok(surface);
+    const comment = await store.createComment({
+      sessionId: b.id,
+      surfaceId: surface.id,
+      author: "user",
+      text: "mismatch",
+    });
+    assert.ok(comment);
+    assert.equal(comment.sessionId, a.id);
+    assert.equal(comment.surfaceId, surface.id);
+    // listed under A, not B
+    assert.deepEqual(
+      (await store.listComments({ sessionId: a.id })).map((c) => c.text),
+      ["mismatch"],
+    );
+    assert.deepEqual(await store.listComments({ sessionId: b.id }), []);
+  });
+
   contract("comment seq is strictly monotonic, even across deletes", async (store) => {
     const first = await store.createSession({ agent: "a" });
     const c1 = await store.createComment({ sessionId: first.id, author: "user", text: "1" });

workers/sqlStore.ts

@@ -368,27 +368,30 @@ export class SqlStore implements Store {
   }
 
   async createComment(input: CreateCommentInput) {
-    if (!(await this.getSession(input.sessionId))) return null;
     const surface = input.surfaceId ? await this.getSurface(input.surfaceId) : null;
+    // Derive the session from the surface so a comment can never land in a
+    // session that doesn't own its surface.
+    const sessionId = surface ? surface.sessionId : input.sessionId;
+    if (!(await this.getSession(sessionId))) return null;
     const id = newId();
     const createdAt = new Date().toISOString();
     const author = input.author.trim() || "user";
     this.sql.exec(
       "INSERT INTO comments (id, sessionId, surfaceId, surfaceTitle, author, text, createdAt) VALUES (?, ?, ?, ?, ?, ?, ?)",
       id,
-      input.sessionId,
+      sessionId,
       surface?.id ?? null,
       surface?.title ?? null,
       author,
       input.text,
       createdAt,
     );
     const seq = this.sql.exec("SELECT last_insert_rowid() AS seq").one().seq as number;
-    this.touch(input.sessionId);
+    this.touch(sessionId);
     return {
       id,
       seq,
-      sessionId: input.sessionId,
+      sessionId,
       surfaceId: surface?.id ?? null,
       surfaceTitle: surface?.title ?? null,
       author,