Redesign review comment cards: author border, hover linking, unresolve

planning/collab/test-vectors/event-signature.json

@@ -43,8 +43,14 @@
             "snapshotId": "snap-vec-1",
             "baseHash": "hash-vec-1",
             "position": {
-              "byteRange": [0, 5],
-              "lineRange": [1, 1]
+              "byteRange": [
+                0,
+                5
+              ],
+              "lineRange": [
+                1,
+                1
+              ]
             }
           },
           "body": "hello"
@@ -70,7 +76,10 @@
           "authorId": "p-vec-2",
           "deviceId": "d-vec-2",
           "createdAt": 1700000001500,
-          "parentEventIds": ["evt-zzz", "evt-aaa"],
+          "parentEventIds": [
+            "evt-zzz",
+            "evt-aaa"
+          ],
           "snapshotId": "snap-vec-2"
         },
         "body": {
@@ -82,8 +91,14 @@
             "snapshotId": "snap-vec-2",
             "baseHash": "hash-vec-2",
             "position": {
-              "byteRange": [10, 14],
-              "lineRange": [3, 3]
+              "byteRange": [
+                10,
+                14
+              ],
+              "lineRange": [
+                3,
+                3
+              ]
             }
           },
           "operation": {
@@ -114,7 +129,9 @@
           "authorId": "p-vec-3",
           "deviceId": "d-vec-3",
           "createdAt": 1700000002000,
-          "parentEventIds": ["evt-parent-3"]
+          "parentEventIds": [
+            "evt-parent-3"
+          ]
         },
         "body": {
           "type": "comment_resolved",
@@ -171,7 +188,11 @@
           "authorId": "p-vec-5",
           "deviceId": "d-vec-5",
           "createdAt": 1700000004500,
-          "parentEventIds": ["evt-mid-5", "evt-aaa-5", "evt-zzz-5"],
+          "parentEventIds": [
+            "evt-mid-5",
+            "evt-aaa-5",
+            "evt-zzz-5"
+          ],
           "snapshotId": "snap-vec-5"
         },
         "body": {
@@ -186,6 +207,36 @@
         "signature": "doERvA05RpnAWdc5u1l2MZZQP2ZlpXXTds4ZHLh70w9m11mgOJSjahCXJrioirwj1jNr8vkXgFqmulTeA5zgBw",
         "signingKeyId": "tMHs6Jjs4k4k5gEjL5XGoYlxaJoN1mnm14IYU3whw4k"
       }
+    },
+    {
+      "name": "CommentReopened with one parent — the resolve inverse (attn-bb6t.4); pins the reopen body shape",
+      "signingKey": {
+        "private": "ZmZmZmZmZmZmZmZmZmZmZmZmZmZmZmZmZmZmZmZmZmY",
+        "public": "NLTZBDFWy23PC-sKKUm3VZyUDSvLbb6MU6mzAnjjp0Y"
+      },
+      "event": {
+        "meta": {
+          "v": 2,
+          "eventId": "placeholder-event-id-6",
+          "roomId": "room-vec-6",
+          "authorId": "p-vec-6",
+          "deviceId": "d-vec-6",
+          "createdAt": 1700000005000,
+          "parentEventIds": [
+            "evt-parent-6"
+          ]
+        },
+        "body": {
+          "type": "comment_reopened",
+          "threadId": "thr-vec-6",
+          "reopenedBy": "p-reopener-6"
+        }
+      },
+      "expected": {
+        "canonicalSignedBytes": "{\"body\":{\"reopenedBy\":\"p-reopener-6\",\"threadId\":\"thr-vec-6\",\"type\":\"comment_reopened\"},\"meta\":{\"authorId\":\"p-vec-6\",\"createdAt\":1700000005000,\"deviceId\":\"d-vec-6\",\"parentEventIds\":[\"evt-parent-6\"],\"roomId\":\"room-vec-6\",\"v\":2}}",
+        "signature": "79cQXHqjmkWD8oidDPTFbw2c80b-FT4oCzPYs0PbHOEdowA-g-OAREoPwBj07e_Wcx3xcgX4CqNxorUjJ3zmBw",
+        "signingKeyId": "97dnbJTffo_ZmY44-f_wTYWIsA0YAXZOPxh3o_6uRHc"
+      }
     }
   ]
 }

src/ipc.rs

@@ -157,6 +157,9 @@ pub enum IpcMessage {
     #[serde(rename = "review_resolve_comment", rename_all = "camelCase")]
     ReviewResolveComment { room_id: RoomId, thread_id: String },
 
+    #[serde(rename = "review_reopen_comment", rename_all = "camelCase")]
+    ReviewReopenComment { room_id: RoomId, thread_id: String },
+
     #[serde(rename = "review_stop", rename_all = "camelCase")]
     ReviewStop {
         #[serde(default)]
@@ -571,6 +574,9 @@ pub fn handle_message(body: &str, state: &Arc<Mutex<AppState>>, proxy: &EventLoo
             IpcMessage::ReviewResolveComment { room_id, thread_id } => {
                 submit_review_command(state, ReviewCommand::ResolveComment { room_id, thread_id });
             }
+            IpcMessage::ReviewReopenComment { room_id, thread_id } => {
+                submit_review_command(state, ReviewCommand::ReopenComment { room_id, thread_id });
+            }
             IpcMessage::ReviewStop { room_id } => {
                 submit_review_command(state, ReviewCommand::Stop { room_id });
             }

src/review/crypto/signing.rs

@@ -734,12 +734,32 @@ mod tests {
             resulting_hash: id::<ContentHash>("hash-after-apply-5"),
         };
 
+        // Vector 6: CommentReopened — the resolve inverse (attn-bb6t.4).
+        // Pinned alongside vector 3 so both halves of the resolve/reopen pair
+        // have a locked canonical shape for the TS implementation.
+        let seed6: [u8; 32] = [0x66u8; 32];
+        let meta6 = EventMeta {
+            v: 2,
+            event_id: id::<EventId>("placeholder-event-id-6"),
+            room_id: id::<RoomId>("room-vec-6"),
+            author_id: id::<ParticipantId>("p-vec-6"),
+            device_id: id::<DeviceId>("d-vec-6"),
+            created_at: 1_700_000_005_000,
+            parent_event_ids: vec![id::<EventId>("evt-parent-6")],
+            snapshot_id: None,
+        };
+        let body6 = ReviewEventBody::CommentReopened {
+            thread_id: "thr-vec-6".to_string(),
+            reopened_by: id::<ParticipantId>("p-reopener-6"),
+        };
+
         for (label, seed, meta, body) in [
             ("vec1", seed1, &meta1, &body1),
             ("vec2", seed2, &meta2, &body2),
             ("vec3", seed3, &meta3, &body3),
             ("vec4", seed4, &meta4, &body4),
             ("vec5", seed5, &meta5, &body5),
+            ("vec6", seed6, &meta6, &body6),
         ] {
             let sk = DeviceSigningKey::from_bytes(&seed).unwrap();
             let vk = sk.verifying_key();

src/review/manager.rs

@@ -161,6 +161,10 @@ pub enum ReviewCommand {
     /// event so the resolution persists and propagates to every peer (a
     /// resolution is a shared fact, not a local view tweak).
     ResolveComment { room_id: RoomId, thread_id: String },
+    /// Reopen a resolved comment thread. Mints a durable `CommentReopened`
+    /// event; same reasoning as `ResolveComment` — reopening is a shared
+    /// fact, so it travels rather than living in one client's view state.
+    ReopenComment { room_id: RoomId, thread_id: String },
     /// Owner edited a shared file — republish a fresh snapshot so connected
     /// reviewers see the update. No-op when `path` isn't part of any share.
     PublishSnapshot { path: PathBuf },
@@ -1117,6 +1121,14 @@ impl ReviewManager {
                 self.resolve_comment(bootstrapper, room_id, thread_id);
                 return;
             }
+            (
+                ReviewCommand::ReopenComment { room_id, thread_id },
+                Some(bootstrapper),
+                Some(_runtime),
+            ) => {
+                self.reopen_comment(bootstrapper, room_id, thread_id);
+                return;
+            }
             (
                 ReviewCommand::SendCollab { room_id, payload },
                 Some(bootstrapper),
@@ -1843,8 +1855,8 @@ impl ReviewManager {
     /// normal outbox path, so the resolution persists locally and propagates
     /// to peers. The frontend's `reconstructThreads` flips the thread's
     /// `resolved` flag off the same event, so the card collapses to its
-    /// resolved strip when the `EventImported` round-trips. Reopening is a
-    /// future `CommentReopened` event (not yet modeled).
+    /// resolved strip when the `EventImported` round-trips. The inverse is
+    /// [`Self::reopen_comment`], which mints `CommentReopened`.
     fn resolve_comment(&self, bootstrapper: &Arc<Bootstrapper>, room_id: &RoomId, thread_id: &str) {
         let emit_err = |msg: String| {
             (self.update_tx)(ReviewUpdate::Error {
@@ -1877,6 +1889,44 @@ impl ReviewManager {
         );
     }
 
+    /// Reopen a resolved comment thread — the inverse of
+    /// [`Self::resolve_comment`] (attn-bb6t.4). Mints a durable
+    /// `CommentReopened` event carrying the reopener's participant id, so the
+    /// thread comes back for every peer rather than only in the clicking
+    /// client's view. Projections fold resolve/reopen in log order, so a
+    /// reopen after a resolve wins and a later resolve closes it again.
+    fn reopen_comment(&self, bootstrapper: &Arc<Bootstrapper>, room_id: &RoomId, thread_id: &str) {
+        let emit_err = |msg: String| {
+            (self.update_tx)(ReviewUpdate::Error {
+                room_id: Some(room_id.clone()),
+                code: "ATTN_REOPEN_COMMENT".to_string(),
+                message: msg,
+            });
+        };
+
+        let reopened_by = match bootstrapper
+            .config()
+            .identity_dir()
+            .and_then(|dir| crate::review::bootstrap::load_or_create_identity_in(&dir))
+        {
+            Ok(identity) => identity.typed_participant_id(),
+            Err(e) => return emit_err(format!("load identity: {e}")),
+        };
+
+        let body = crate::review::model::ReviewEventBody::CommentReopened {
+            thread_id: thread_id.to_string(),
+            reopened_by,
+        };
+        let send = bootstrapper.send_event_sync(room_id, body, unix_now_ms_for_manager());
+        self.emit_event_outcome(room_id.clone(), send);
+
+        tracing::info!(
+            "reopened comment thread {} (room={})",
+            thread_id,
+            room_id.as_str()
+        );
+    }
+
     /// Owner/reviewer manually re-anchors a stale comment or suggestion to a
     /// range they selected in the editor. We:
     ///   1. Look up the original event to recover its real `file_id` (the
@@ -3722,6 +3772,7 @@ fn review_command_name(cmd: &ReviewCommand) -> &'static str {
         ReviewCommand::ResolveAnchor { .. } => "ResolveAnchor",
         ReviewCommand::ReportHtmlAnchorResolution { .. } => "ReportHtmlAnchorResolution",
         ReviewCommand::ResolveComment { .. } => "ResolveComment",
+        ReviewCommand::ReopenComment { .. } => "ReopenComment",
         ReviewCommand::SendCollab { .. } => "SendCollab",
         ReviewCommand::PublishSnapshot { .. } => "PublishSnapshot",
         ReviewCommand::ReannounceIdentity => "ReannounceIdentity",
@@ -3902,6 +3953,10 @@ fn stub_update_for(cmd: &ReviewCommand) -> ReviewUpdate {
             room_id: room_id.clone(),
             status: "Pending resolve-comment — no bootstrap attached".to_string(),
         },
+        ReviewCommand::ReopenComment { room_id, .. } => ReviewUpdate::RoomStatusChanged {
+            room_id: room_id.clone(),
+            status: "Pending reopen-comment — no bootstrap attached".to_string(),
+        },
         // PublishSnapshot goes through the real bootstrap path in `submit`
         // when one is attached. Without a bootstrapper (smoke tests) it's a
         // no-op — surface a benign status so the dispatch contract stays
@@ -4569,6 +4624,7 @@ fn review_event_body_name(body: &crate::review::model::ReviewEventBody) -> &'sta
         ReviewEventBody::SnapshotSuperseded { .. } => "snapshot_superseded",
         ReviewEventBody::CommentCreated { .. } => "comment_created",
         ReviewEventBody::CommentResolved { .. } => "comment_resolved",
+        ReviewEventBody::CommentReopened { .. } => "comment_reopened",
         ReviewEventBody::SuggestionCreated { .. } => "suggestion_created",
         ReviewEventBody::SuggestionAccepted { .. } => "suggestion_accepted",
         ReviewEventBody::SuggestionRejected { .. } => "suggestion_rejected",

src/review/model.rs

@@ -1386,6 +1386,22 @@ pub enum ReviewEventBody {
         thread_id: String,
         resolved_by: ParticipantId,
     },
+    /// Reopen a resolved thread (attn-bb6t.4). Deliberately its own variant
+    /// rather than a `resolved: bool` on `CommentResolved`: the log is
+    /// append-only and every existing receiver already reads
+    /// `CommentResolved` as "this thread is closed", so flipping a field
+    /// would have changed the meaning of events already on disk. Projections
+    /// must therefore fold resolve/reopen in log order — last writer wins,
+    /// not "any resolve anywhere".
+    ///
+    /// Receivers older than this variant reject the event (the enum is
+    /// externally tagged and unknown tags fail to deserialize), so a reopen
+    /// in a mixed-version room is invisible to them and the thread stays
+    /// resolved on their side. Same compatibility family as attn-mz25.
+    CommentReopened {
+        thread_id: String,
+        reopened_by: ParticipantId,
+    },
     SuggestionCreated {
         suggestion_id: String,
         anchor: Anchor,

src/review/transport/inbound.rs

@@ -727,6 +727,16 @@ fn authorize_event(
         {
             Ok(())
         }
+        // Reopening carries exactly the resolve authority (attn-bb6t.4): a
+        // non-agent participant, acting as themselves. Anything narrower —
+        // "only the resolver may reopen" — would strand a thread whose
+        // resolver has left the room.
+        ReviewEventBody::CommentReopened { reopened_by, .. }
+            if registered.kind != ParticipantKind::Agent
+                && reopened_by == &event.meta.author_id =>
+        {
+            Ok(())
+        }
         ReviewEventBody::PresenceUpdated {
             participant_id,
             device_id,
@@ -1188,6 +1198,47 @@ mod tests {
         assert_eq!(store.iter_events(&room_id).expect("events").count(), 0);
     }
 
+    #[tokio::test]
+    async fn reviewer_can_import_self_attributed_comment_reopened() {
+        let (pipeline, store, signer, room_id, _tmp) = fresh_pipeline_with_signer();
+        let envelope = mint_event_envelope_with_body(
+            pipeline.event_key,
+            signer,
+            &room_id,
+            ReviewEventBody::CommentReopened {
+                thread_id: "thread-1".to_string(),
+                reopened_by: id::<ParticipantId>("p-author-01"),
+            },
+        );
+        pipeline
+            .import_event_envelope(&room_id, &envelope)
+            .await
+            .expect("self-attributed reopen must be accepted");
+        assert_eq!(store.iter_events(&room_id).expect("events").count(), 1);
+    }
+
+    #[tokio::test]
+    async fn comment_reopened_on_someone_elses_behalf_is_refused() {
+        let (pipeline, store, signer, room_id, _tmp) = fresh_pipeline_with_signer();
+        let envelope = mint_event_envelope_with_body(
+            pipeline.event_key,
+            signer,
+            &room_id,
+            ReviewEventBody::CommentReopened {
+                thread_id: "thread-1".to_string(),
+                // Not the envelope's author: reopening in another
+                // participant's name is exactly what the guard exists for.
+                reopened_by: id::<ParticipantId>("p-someone-else"),
+            },
+        );
+        let error = pipeline
+            .import_event_envelope(&room_id, &envelope)
+            .await
+            .expect_err("reopen attributed to another participant must be refused");
+        assert!(matches!(error, InboundError::UnauthorizedEvent));
+        assert_eq!(store.iter_events(&room_id).expect("events").count(), 0);
+    }
+
     #[tokio::test]
     async fn reviewer_cannot_import_owner_only_snapshot_event() {
         let (pipeline, store, signer, room_id, _tmp) = fresh_pipeline_with_signer();