fix(comments): preserve pre-share comments during identity recovery

apps/daemon/src/collab/comment-file-identity-recovery.ts

@@ -0,0 +1,61 @@
+import type Database from 'better-sqlite3';
+import type { CollabCloudComment } from '@open-design/contracts';
+import { isProjectCommentAnchorConversationId } from '../db.js';
+import { createCommentRelayOutboxStore } from './comment-relay-outbox.js';
+
+/** Rebind only explicit local pre-share comments, with their durable outbound
+ * snapshots. The caller commits this together with fresh receipt-backed facts.
+ * Artifact IDs, inbound comments and identity-less history remain unchanged.
+ */
+export function rebindLocalCommentFileIdentity(db: Database.Database, input: {
+  projectId: string; workspaceId: string; creatorMemberId: string;
+  filePath: string; localSourceFileId: string; sourceFileId: string;
+}): void {
+  if (!db.inTransaction) throw new Error('Comment identity recovery requires a transaction');
+  const comments = db.prepare(`SELECT id, conversation_id AS conversationId FROM preview_comments
+    WHERE project_id = ? AND file_path = ? AND source_file_id = ?
+      AND COALESCE(author_kind, 'member') = 'member' AND author_app_user_id IS NULL
+      AND cloud_revision_seq IS NULL`)
+    .all(input.projectId, input.filePath, input.localSourceFileId) as Array<{ id: string; conversationId: string }>;
+  if (!comments.length) return;
+  const outbox = createCommentRelayOutboxStore(db);
+  const rebind = (serialized: string, commentId: string): CollabCloudComment => {
+    const comment = JSON.parse(serialized) as CollabCloudComment;
+    if (comment.id !== commentId || comment.projectId !== input.projectId
+      || comment.sourceFilePath !== input.filePath || comment.sourceFileId !== input.localSourceFileId) {
+      throw new Error('COMMENT_FILE_LIFETIME_CONFLICT');
+    }
+    return { ...comment, sourceFileId: input.sourceFileId };
+  };
+  for (const comment of comments) {
+    if (isProjectCommentAnchorConversationId(comment.conversationId)) continue;
+    db.prepare('UPDATE preview_comments SET source_file_id = ? WHERE project_id = ? AND id = ?')
+      .run(input.sourceFileId, input.projectId, comment.id);
+    const queued = db.prepare(`SELECT payload_json AS payload, publication_json AS publication,
+        relay_scope AS relayScope, expected_owner_member_id AS expectedOwner
+      FROM comment_relay_outbox WHERE workspace_id = ? AND workspace_member_id = ?
+        AND team_id = ? AND project_id = ? AND file_path = ? AND comment_id = ?`)
+      .get(input.workspaceId, input.creatorMemberId, input.workspaceId, input.projectId, input.filePath, comment.id) as {
+        payload: string; publication: string | null; relayScope: 'personal' | 'team'; expectedOwner: string | null;
+      } | undefined;
+    if (queued) {
+      if (queued.expectedOwner !== input.creatorMemberId) throw new Error('COMMENT_FILE_LIFETIME_CONFLICT');
+      // enqueue rotates the payload fingerprint/event key and revision fence:
+      // an ACK for the old UUID cannot delete this replacement snapshot.
+      outbox.enqueue({ workspaceId: input.workspaceId, workspaceMemberId: input.creatorMemberId,
+        teamId: input.workspaceId, relayScope: queued.relayScope, projectId: input.projectId,
+        expectedOwnerMemberId: queued.expectedOwner, comment: rebind(queued.payload, comment.id),
+        ...(queued.publication ? { publication: JSON.parse(queued.publication) } : {}),
+      });
+    }
+    const journal = db.prepare(`SELECT payload_json AS payload FROM published_comment_mutations
+      WHERE resource_team_id = ? AND owner_member_id = ? AND project_id = ? AND file_path = ? AND comment_id = ?`)
+      .get(input.workspaceId, input.creatorMemberId, input.projectId, input.filePath, comment.id) as { payload: string } | undefined;
+    if (journal) {
+      db.prepare(`UPDATE published_comment_mutations SET payload_json = ?
+        WHERE resource_team_id = ? AND owner_member_id = ? AND project_id = ? AND file_path = ? AND comment_id = ?`)
+        .run(JSON.stringify(rebind(journal.payload, comment.id)), input.workspaceId, input.creatorMemberId,
+          input.projectId, input.filePath, comment.id);
+    }
+  }
+}

apps/daemon/src/collab/local-comment-files.ts

@@ -1,7 +1,8 @@
 import { assertCommentFileNotPending } from './comment-file-retirements.js';
 import type Database from 'better-sqlite3';
 import type { CommentFileFact } from '@open-design/contracts';
-import { ensureWorkspaceArtifactForPath } from '../chat-artifacts/store.js';
+import { ensureWorkspaceArtifactForPath, getLiveWorkspaceArtifactByPath } from '../chat-artifacts/store.js';
+import { rebindLocalCommentFileIdentity } from './comment-file-identity-recovery.js';
 import { getProject, getWorkspaceProjectByProjectId, updateProject } from '../db.js';
 import type { runVelaCommand } from '../integrations/vela-command.js';
 import { commitCommentFileFacts, retiredCommentFileMutation } from './comment-file-authority.js';
@@ -60,6 +61,7 @@ export function createLocalCommentFiles(db: Database.Database) {
     paths: Array<{ sourceFilePath: string; entryPath: string; sourceFileId?: string }>; run: typeof runVelaCommand;
   }) {
     creator(input.projectId, input.workspaceId, input.creatorMemberId);
+    const recoveries = new Map<string, string>();
     const files = input.paths.map(file => {
       if (!canonicalCommentFilePath(file.sourceFilePath)) throw new Error('COMMENT_FILE_AUTHORITY_UNAVAILABLE');
       assertCommentFileNotPending(db, input.projectId, { filePath: file.sourceFilePath });
@@ -67,13 +69,34 @@ export function createLocalCommentFiles(db: Database.Database) {
         .some(fact => fact.sourceFilePath === file.sourceFilePath);
       const sourceFileId = !known && file.sourceFileId ? file.sourceFileId : identity(input.projectId, file.sourceFilePath);
       if (file.sourceFileId && file.sourceFileId !== sourceFileId) throw new Error('COMMENT_FILE_LIFETIME_CONFLICT');
+      const local = !known && file.sourceFileId ? getLiveWorkspaceArtifactByPath(db, input.projectId, file.sourceFilePath) : null;
+      if (local && local.id !== sourceFileId) {
+        assertCommentFileNotPending(db, input.projectId, { sourceFileId: local.id });
+        recoveries.set(file.sourceFilePath, local.id);
+      }
       return { sourceFilePath: file.sourceFilePath, entryPath: file.entryPath, sourceFileId, retired: false };
     });
     if (!files.length) return;
     const state = await commitCommentFileFacts({ ...input, files });
-    creator(input.projectId, input.workspaceId, input.creatorMemberId);
-    const adopted = commentFilesAtReceipt(parseCommentFileState(getProject(db, input.projectId)?.metadata?._commentFiles), state, input);
-    updateProject(db, input.projectId, { metadata: { ...getProject(db, input.projectId)?.metadata, _commentFiles: adopted } });
+    db.transaction(() => {
+      creator(input.projectId, input.workspaceId, input.creatorMemberId);
+      const previous = parseCommentFileState(getProject(db, input.projectId)?.metadata?._commentFiles);
+      const adopted = commentFilesAtReceipt(previous, state, input);
+      for (const file of files) {
+        const localSourceFileId = recoveries.get(file.sourceFilePath);
+        if (!localSourceFileId) continue;
+        assertCommentFileNotPending(db, input.projectId, { filePath: file.sourceFilePath, sourceFileId: localSourceFileId });
+        if (getLiveWorkspaceArtifactByPath(db, input.projectId, file.sourceFilePath)?.id !== localSourceFileId
+          || previous?.files.some(fact => fact.sourceFileId === localSourceFileId)
+          || state.files.some(fact => fact.sourceFileId === localSourceFileId)) {
+          throw new Error('COMMENT_FILE_LIFETIME_CONFLICT');
+        }
+        rebindLocalCommentFileIdentity(db, { projectId: input.projectId, workspaceId: input.workspaceId,
+          creatorMemberId: input.creatorMemberId, filePath: file.sourceFilePath,
+          localSourceFileId, sourceFileId: file.sourceFileId });
+      }
+      updateProject(db, input.projectId, { metadata: { ...getProject(db, input.projectId)?.metadata, _commentFiles: adopted } });
+    })();
   }
   /** One catalog CAS and one local adoption for the captured original lifetimes.
    * A failed later receipt must not retire an earlier file in a folder operation. */

apps/daemon/tests/local-comment-files.test.ts

@@ -13,6 +13,8 @@ import { createSqlitePublicFilePublicationStore } from '../src/collab/public-fil
 import { createPublicFilePublicationRecorder } from '../src/collab/public-file-publication-recording.js';
 import { enqueuePublishedFileComments } from '../src/collab/published-file-comment-backfill.js';
 import { createCommentRelayOutboxStore } from '../src/collab/comment-relay-outbox.js';
+import { previewCommentToCloud } from '../src/collab/collab-cloud-service.js';
+import { recordCommentRelayPublicationMapping, recordPublishedCommentMutation } from '../src/collab/comment-relay-publication-mapping.js';
 import type { CollabCloudComment } from '@open-design/contracts';
 
 let root: string | undefined;
@@ -102,6 +104,114 @@ it('reuses an active cloud identity when the local mapping is missing and admits
   expect(normalizeCurrentFileComment(cloud.state(), comment)?.sourceFileId).toBe(sourceFileId);
 });
 
+it('atomically rebinds pre-share comments and pending relay snapshots before publication backfill', async () => {
+  const db = setup();
+  const files = createLocalCommentFiles(db);
+  const localId = files.identity('p', 'a.html');
+  const target = (filePath: string) => ({ filePath, elementId: 'hero', selector: '#hero', label: 'Hero',
+    position: { x: 1, y: 2, width: 3, height: 4 } });
+  const original = upsertPreviewComment(db, 'p', 'c', { id: 'local-comment', sourceFileId: localId,
+    authorMemberId: 'creator', note: 'before sharing', target: target('a.html') });
+  if (!original) throw new Error('missing local comment');
+  upsertPreviewComment(db, 'p', 'c', { id: 'different-file', sourceFileId: localId, note: 'wrong path', target: target('b.html') });
+  upsertPreviewComment(db, 'p', 'c', { id: 'identityless', note: 'legacy', target: target('a.html') });
+  const retiredId = '22222222-2222-4222-8222-222222222222';
+  upsertPreviewComment(db, 'p', 'c', { id: 'different-lifetime', sourceFileId: retiredId, note: 'other lifetime', target: target('a.html') });
+  upsertPreviewComment(db, 'p', 'c', { id: 'inbound', sourceFileId: localId, note: 'inbound user', target: target('a.html') });
+  db.prepare("UPDATE preview_comments SET author_kind = 'user', author_app_user_id = 'external-user' WHERE id = 'inbound'").run();
+  upsertPreviewComment(db, 'p', 'c', { id: 'already-synced', sourceFileId: localId, note: 'remote lifetime', target: target('a.html') });
+  db.prepare("UPDATE preview_comments SET cloud_revision_seq = 42 WHERE id = 'already-synced'").run();
+  insertProject(db, { id: 'other', name: 'Other', createdAt: 1, updatedAt: 1 });
+  insertConversation(db, { id: 'other-c', projectId: 'other', title: 'Other', createdAt: 1, updatedAt: 1 });
+  upsertPreviewComment(db, 'other', 'other-c', { id: 'other-project', sourceFileId: localId, note: 'foreign project', target: target('a.html') });
+  const outbox = createCommentRelayOutboxStore(db);
+  outbox.enqueue({ workspaceId: 'w', workspaceMemberId: 'creator', teamId: 'w', relayScope: 'personal',
+    projectId: 'p', expectedOwnerMemberId: 'creator', comment: previewCommentToCloud(original, '') });
+  const queuedBefore = outbox.listDue(Date.now())[0]!;
+  const publications = createSqlitePublicFilePublicationStore(db);
+  db.transaction(() => {
+    const scope = { resourceTeamId: 'w', ownerMemberId: 'creator', projectId: 'p', filePath: 'a.html' };
+    recordCommentRelayPublicationMapping(db, scope, { slug: 'recovered', token: 'old-revision', publicFilePath: 'index.html' });
+    expect(recordPublishedCommentMutation(db, scope, { ...queuedBefore.comment, note: 'latest stopped edit' })).toBe(true);
+  })();
+  const remote: CommentFileState = { schemaVersion: 1, revision: 3, files: [{ sourceFileId: '11111111-1111-4111-8111-111111111111',
+    sourceFilePath: 'a.html', entryPath: 'a.html', resourceId: 'root', versionId: 'old', retired: false }] };
+  const cloud = catalog(remote);
+  const sourceFileId = files.identity('p', 'a.html', remote);
+  const input = { projectId: 'p', workspaceId: 'w', creatorMemberId: 'creator', resourceId: 'selected', versionId: 'fresh',
+    paths: [{ sourceFilePath: 'a.html', entryPath: 'index.html', sourceFileId }], run: cloud.run };
+  await files.admit(input);
+  expect(getProjectPreviewComment(db, 'p', original.id)).toMatchObject({ sourceFileId, note: original.note,
+    authorMemberId: original.authorMemberId, createdAt: original.createdAt, updatedAt: original.updatedAt });
+  const queuedAfter = outbox.listDue(Date.now())[0]!;
+  expect(queuedAfter.comment).toMatchObject({ id: original.id, sourceFileId, sourceFilePath: 'a.html' });
+  expect(queuedAfter.revision).toBeGreaterThan(queuedBefore.revision);
+  expect(queuedAfter.eventKey).not.toBe(queuedBefore.eventKey);
+  expect(outbox.acknowledge(queuedBefore, 'delivered')).toBe(false);
+  expect(getProjectPreviewComment(db, 'p', 'different-file')?.sourceFileId).toBe(localId);
+  expect(getProjectPreviewComment(db, 'p', 'identityless')?.sourceFileId).toBeUndefined();
+  expect(getProjectPreviewComment(db, 'p', 'different-lifetime')?.sourceFileId).toBe(retiredId);
+  expect(getProjectPreviewComment(db, 'p', 'inbound')?.sourceFileId).toBe(localId);
+  expect(getProjectPreviewComment(db, 'p', 'already-synced')?.sourceFileId).toBe(localId);
+  expect(getProjectPreviewComment(db, 'other', 'other-project')?.sourceFileId).toBe(localId);
+  const record = createPublicFilePublicationRecorder(db, publications, enqueuePublishedFileComments);
+  const scope = { sourceFileId, resourceTeamId: 'w', ownerMemberId: 'creator', projectId: 'p', filePath: 'a.html' };
+  record(scope, { slug: 'recovered', url: null, fileName: 'a.html' }, [{ sourcePath: 'a.html', publishedPath: 'index.html' }]);
+  const backfilled = outbox.listDue(Date.now());
+  expect(backfilled).toHaveLength(1);
+  expect(backfilled[0]!.comment).toMatchObject({ id: original.id, sourceFileId, note: 'latest stopped edit' });
+  const eventKey = backfilled[0]!.eventKey;
+  await files.admit(input);
+  record(scope, { slug: 'recovered', url: null, fileName: 'a.html' }, [{ sourcePath: 'a.html', publishedPath: 'index.html' }]);
+  expect(outbox.listDue(Date.now())).toHaveLength(1);
+  expect(outbox.listDue(Date.now())[0]!.eventKey).toBe(eventKey);
+});
+
+it.each(['cloud admission', 'local commit'])('keeps comments, relay snapshots and facts unchanged on failed %s', async failure => {
+  const db = setup();
+  const files = createLocalCommentFiles(db);
+  const localId = files.identity('p', 'a.html');
+  const original = upsertPreviewComment(db, 'p', 'c', { id: 'pending', sourceFileId: localId, authorMemberId: 'creator', note: 'local',
+    target: { filePath: 'a.html', elementId: 'hero', selector: '#hero', label: 'Hero', position: { x: 0, y: 0, width: 1, height: 1 } } });
+  if (!original) throw new Error('missing comment');
+  const outbox = createCommentRelayOutboxStore(db);
+  outbox.enqueue({ workspaceId: 'w', workspaceMemberId: 'creator', teamId: 'w', relayScope: 'personal', projectId: 'p',
+    expectedOwnerMemberId: 'creator', comment: previewCommentToCloud(original, '') });
+  const queued = outbox.listDue(Date.now());
+  const remote: CommentFileState = { schemaVersion: 1, revision: 3, files: [{ sourceFileId: '11111111-1111-4111-8111-111111111111',
+    sourceFilePath: 'a.html', entryPath: 'a.html', resourceId: 'root', versionId: 'old', retired: false }] };
+  const cloud = catalog(remote);
+  if (failure === 'local commit') db.exec(`CREATE TRIGGER fail_identity_commit BEFORE UPDATE OF metadata_json ON projects
+    BEGIN SELECT RAISE(ABORT, 'local commit failed'); END`);
+  await expect(files.admit({ projectId: 'p', workspaceId: 'w', creatorMemberId: 'creator', resourceId: 'selected', versionId: 'fresh',
+    paths: [{ sourceFilePath: 'a.html', entryPath: 'index.html', sourceFileId: files.identity('p', 'a.html', remote) }],
+    run: async args => {
+      if (failure === 'cloud admission' && args[1] === 'upsert') throw new Error('cloud admission failed');
+      return cloud.run(args);
+    } })).rejects.toThrow(`${failure} failed`);
+  expect(getProjectPreviewComment(db, 'p', original.id)).toMatchObject({ sourceFileId: localId, note: original.note });
+  expect(outbox.listDue(Date.now())).toEqual(queued);
+  expect(getProject(db, 'p')?.metadata?._commentFiles).toBeUndefined();
+});
+
+it('does not migrate comments from an artifact identity already retired in the catalog', async () => {
+  const db = setup();
+  const files = createLocalCommentFiles(db);
+  const localId = files.identity('p', 'a.html');
+  const original = upsertPreviewComment(db, 'p', 'c', { id: 'retired-comment', sourceFileId: localId, note: 'old lifetime',
+    target: { filePath: 'a.html', elementId: 'hero', selector: '#hero', label: 'Hero', position: { x: 0, y: 0, width: 1, height: 1 } } });
+  if (!original) throw new Error('missing comment');
+  const active = { sourceFileId: '11111111-1111-4111-8111-111111111111', sourceFilePath: 'a.html',
+    entryPath: 'a.html', resourceId: 'root', versionId: 'old', retired: false };
+  const remote: CommentFileState = { schemaVersion: 1, revision: 3, files: [active, { ...active, sourceFileId: localId, retired: true }] };
+  const cloud = catalog(remote);
+  await expect(files.admit({ projectId: 'p', workspaceId: 'w', creatorMemberId: 'creator', resourceId: 'selected', versionId: 'fresh',
+    paths: [{ sourceFilePath: 'a.html', entryPath: 'index.html', sourceFileId: files.identity('p', 'a.html', remote) }],
+    run: cloud.run })).rejects.toThrow('COMMENT_FILE_LIFETIME_CONFLICT');
+  expect(getProjectPreviewComment(db, 'p', original.id)?.sourceFileId).toBe(localId);
+  expect(getProject(db, 'p')?.metadata?._commentFiles).toBeUndefined();
+});
+
 it('does not persist recovered identity when cloud admission fails', async () => {
   const db = setup();
   const files = createLocalCommentFiles(db);