diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl index 51379fd..7d77ead 100644 --- a/.beads/interactions.jsonl +++ b/.beads/interactions.jsonl @@ -198,3 +198,7 @@ {"id":"int-c027a4a5","kind":"field_change","created_at":"2026-06-11T02:17:08.736047Z","actor":"Angus Bezzina","issue_id":"attn-23m","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} {"id":"int-6e5885d9","kind":"field_change","created_at":"2026-06-11T13:35:27.549825Z","actor":"Angus Bezzina","issue_id":"attn-z46","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} {"id":"int-c105ce23","kind":"field_change","created_at":"2026-06-11T14:20:18.718761Z","actor":"Angus Bezzina","issue_id":"attn-2aj","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} +{"id":"int-4b35f623","kind":"field_change","created_at":"2026-06-11T16:46:11.954096Z","actor":"Angus Bezzina","issue_id":"attn-6dd","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} +{"id":"int-4e8feff2","kind":"field_change","created_at":"2026-06-11T16:46:12.384671Z","actor":"Angus Bezzina","issue_id":"attn-ry9","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} +{"id":"int-44eff8f8","kind":"field_change","created_at":"2026-06-11T17:23:31.040962Z","actor":"Angus Bezzina","issue_id":"attn-k1n","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} +{"id":"int-fd6076ed","kind":"field_change","created_at":"2026-06-11T18:16:13.990038Z","actor":"Angus Bezzina","issue_id":"attn-bbg","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} diff --git a/scripts/dev-collab.sh b/scripts/dev-collab.sh index 04035b1..092556a 100755 --- a/scripts/dev-collab.sh +++ b/scripts/dev-collab.sh @@ -153,12 +153,24 @@ join_reviewer() { log "(The reviewer window shows '$REVIEWER_FIXTURE_PATH' until it joins.) Copy the invite, then paste it here." log "(Empty line cancels and leaves the daemons running.)" printf 'Paste invite > ' - IFS= read -r invite || true - if [ -z "$invite" ]; then + IFS= read -r pasted || true + if [ -z "$pasted" ]; then log "No invite supplied — daemons remain up. Ctrl+C to stop." return 0 fi + # The share dialog offers two copyable things: the bare attn:// URL and + # the full `npx attnmd review join 'attn://…'` one-liner. Accept either + # (plus stray quotes/whitespace) by extracting the invite URL from + # whatever was pasted — passing the npx command through verbatim used to + # send garbage to the daemon while this script still claimed success. + invite=$(printf '%s' "$pasted" | grep -oE "attn://review/[^'\"[:space:]]+" | head -1) + if [ -z "$invite" ]; then + err "no attn://review/… invite found in the pasted text — copy either the Direct link or the npx command from the Share dialog" + log "Daemons remain up. Ctrl+C to stop." + return 0 + fi + log "Reviewer joining (windowed daemon)..." # Route the join to the already-running reviewer DAEMON via its ATTN_HOME # socket — deliberately NOT `--as-agent`, which forks a separate *headless* diff --git a/scripts/test-editorial-e2e.sh b/scripts/test-editorial-e2e.sh index 64b5d25..6063c7a 100755 --- a/scripts/test-editorial-e2e.sh +++ b/scripts/test-editorial-e2e.sh @@ -96,7 +96,10 @@ rv --eval "window.__attn_user_profile__.save('$RVNAME');'x'" >/dev/null 2>&1 poll 5000 sh -c "grep -q 'Riley Reviewer' '$RV_HOME/identity.json'" && ok "onboarding: reviewer name persisted to identity" || bad "onboarding: reviewer name not persisted" log "reviewer joins" -rv --eval "window.ipc&&window.ipc.postMessage(JSON.stringify({type:'review_join',invite:'$INVITE'}));'x'" >/dev/null 2>&1 +# Daemon-routed CLI join (same path dev-collab uses). A raw postMessage join +# is rejected by the IPC capability-token gate; the CLI rides the unix socket +# straight into the ReviewManager, no token needed. +rv review join "$INVITE" >/dev/null 2>&1 || bad "review join CLI exited non-zero" pm_ready(){ [ -n "$("$1" --eval "window.__attnPmView?'y':''" 2>/dev/null | tr -d '\"')" ]; } poll 30000 pm_ready rv && ok "reviewer editor live" || { bad "reviewer editor never live"; exit 1; } if poll 20000 has rv '[data-slot=shared-doc-banner]'; then ok "reviewer shows shared-doc banner"; else bad "reviewer never entered shared-doc view"; fi @@ -136,8 +139,10 @@ case "$rvauthor" in esac # attn-1rm — reply via the real IPC the button sends; verify grouping on owner. +# Raw postMessage needs the session capability token — debug builds expose it +# to the automation bridge as window.__attn_ipc_token__ (App.svelte). log "reviewer replies ($REPLY)" -rv --eval "(function(){var s=window.__attn_review_store__;var t=s.threadsForCurrentFile[0];if(!t||t.rootEvent.body.type!=='comment_created')return 'no-thread';window.ipc.postMessage(JSON.stringify({type:'review_create_comment',roomId:s.currentRoomId,anchor:t.rootEvent.body.anchor,body:'$REPLY',parentThreadId:t.id}));return 'sent'})()" >/dev/null 2>&1 +rv --eval "(function(){var s=window.__attn_review_store__;var t=s.threadsForCurrentFile[0];if(!t||t.rootEvent.body.type!=='comment_created')return 'no-thread';window.ipc.postMessage(JSON.stringify({type:'review_create_comment',roomId:s.currentRoomId,anchor:t.rootEvent.body.anchor,body:'$REPLY',parentThreadId:t.id,token:window.__attn_ipc_token__}));return 'sent'})()" >/dev/null 2>&1 owner_reply(){ grep -rqa "$REPLY" "$OWNER_HOME/reviews" 2>/dev/null; } if poll 20000 owner_reply; then ok "attn-1rm: reply imported by owner"; else bad "attn-1rm: reply not imported"; fi threads=$(grep -rhoa '"threadId":"[^"]*"' "$OWNER_HOME/reviews" 2>/dev/null | sort -u | wc -l | tr -d ' ') @@ -146,7 +151,7 @@ cc=$(grep -rhoa '"type":"comment_created"' "$OWNER_HOME/reviews" 2>/dev/null | w # attn-zhr — resolve via the real IPC the button sends; verify event on owner. log "reviewer resolves the thread" -rv --eval "(function(){var s=window.__attn_review_store__;var t=s.threadsForCurrentFile[0];if(!t)return 'no-thread';window.ipc.postMessage(JSON.stringify({type:'review_resolve_comment',roomId:s.currentRoomId,threadId:t.id}));return 'sent'})()" >/dev/null 2>&1 +rv --eval "(function(){var s=window.__attn_review_store__;var t=s.threadsForCurrentFile[0];if(!t)return 'no-thread';window.ipc.postMessage(JSON.stringify({type:'review_resolve_comment',roomId:s.currentRoomId,threadId:t.id,token:window.__attn_ipc_token__}));return 'sent'})()" >/dev/null 2>&1 owner_resolved(){ grep -rqa '"type":"comment_resolved"' "$OWNER_HOME/reviews" 2>/dev/null; } if poll 20000 owner_resolved; then ok "attn-zhr: CommentResolved imported by owner"; else bad "attn-zhr: no comment_resolved event"; fi @@ -158,7 +163,7 @@ log "attn-tqq: room controls (switch list + leave)" # switcher RENDERS and drive leave via the exact IPC the button sends # (review_stop). The daemon Stop emits "Stopped" -> forgetRoom -> back to local. if poll 6000 has rv '.room-menu-trigger'; then ok "attn-tqq: room switcher present (ReviewBar dropdown lists rooms)"; else bad "attn-tqq: no room switcher"; fi -rv --eval "window.ipc.postMessage(JSON.stringify({type:'review_stop',roomId:window.__attn_review_store__.currentRoomId}));'sent'" >/dev/null 2>&1 +rv --eval "window.ipc.postMessage(JSON.stringify({type:'review_stop',roomId:window.__attn_review_store__.currentRoomId,token:window.__attn_ipc_token__}));'sent'" >/dev/null 2>&1 # Authoritative "left the room" signal: currentRoomId cleared (the shared-doc # banner follows reactively). Allow time for the daemon's transport teardown. room_cleared(){ [ "$(rv --eval "String(window.__attn_review_store__.currentRoomId)" 2>/dev/null | tr -d '"')" = "null" ]; } diff --git a/scripts/test-review-e2e.sh b/scripts/test-review-e2e.sh index 831990c..38f652d 100755 --- a/scripts/test-review-e2e.sh +++ b/scripts/test-review-e2e.sh @@ -506,17 +506,41 @@ screenshot "08-avatar-expanded" echo "" echo "--- Cards track their anchors while the document scrolls (attn-23m) ---" -# Scroll the EDITOR viewport and assert the card's on-screen top moves by -# the same amount (opposite sign). Tolerance ±4px for rounding/collision. +# Position the card mid-container first (strict 1:1 tracking only holds +# OUTSIDE the rail's top/bottom fit bands — fitBottom intentionally lifts +# cards near the bottom edge), then scroll by a small delta and assert the +# card's on-screen top moves the same amount. Tolerance ±4px. +# Step 1: center the card in the margin container (the margin repositions +# asynchronously on scroll, so measure only after a settle pause). +center_js=$(cat <<'EOF' +(() => { + const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]'); + const vp = document.querySelector('.attn-content-viewport [data-slot="scroll-area-viewport"]'); + const margin = document.querySelector('[data-slot="review-margin"]'); + if (!card || !vp || !margin) return 'missing'; + const mr = margin.getBoundingClientRect(); + const cr = card.getBoundingClientRect(); + const shift = (cr.top + cr.height / 2) - (mr.top + mr.height / 2); + vp.scrollTop = Math.max(0, vp.scrollTop + shift); + return 'centered'; +})() +EOF +) +result=$("$ATTN" --eval "$center_js") +assert_eq "Card centered in the rail for a clean sample" "$result" '"centered"' +sleep 0.4 + +# Step 2: record the settled position, then scroll +60. scroll_js=$(cat <<'EOF' (() => { const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]'); const vp = document.querySelector('.attn-content-viewport [data-slot="scroll-area-viewport"]'); if (!card || !vp) return 'missing'; + const baseScroll = vp.scrollTop; const before = card.getBoundingClientRect().top; - vp.scrollTop = 150; - const scrolled = vp.scrollTop; - window.__attn_scroll_probe__ = { before, scrolled }; + vp.scrollTop = baseScroll + 60; + const scrolled = vp.scrollTop - baseScroll; + window.__attn_scroll_probe__ = { before, scrolled, baseScroll }; return scrolled > 0 ? 'scrolled' : 'no-scroll'; })() EOF @@ -539,7 +563,7 @@ result=$(poll_eval "$verify_js" '"tracked"') assert_eq "Card moved 1:1 with the document scroll" "$result" '"tracked"' # Scroll back and confirm it returns to its original position. -"$ATTN" --eval "document.querySelector('.attn-content-viewport [data-slot=\\\"scroll-area-viewport\\\"]').scrollTop = 0" >/dev/null +"$ATTN" --eval "(() => { const vp = document.querySelector('.attn-content-viewport [data-slot=\\\"scroll-area-viewport\\\"]'); vp.scrollTop = window.__attn_scroll_probe__.baseScroll; return vp.scrollTop; })()" >/dev/null restore_js=$(cat <<'EOF' (() => { const probe = window.__attn_scroll_probe__; @@ -553,6 +577,32 @@ EOF result=$(poll_eval "$restore_js" '"restored"') assert_eq "Card returns to its anchor when scrolled back" "$result" '"restored"' +echo "" +echo "--- Cards with on-screen anchors are never cut off at the rail bottom ---" +# Scroll so the open card's anchor sits near the viewport bottom: the +# fitBottom pass must lift the card fully into view instead of letting the +# rail's clip line cut it. +fit_js=$(cat <<'EOF' +(() => { + const vp = document.querySelector('.attn-content-viewport [data-slot="scroll-area-viewport"]'); + const margin = document.querySelector('[data-slot="review-margin"]'); + const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]'); + if (!vp || !margin || !card) return 'missing'; + const mr = margin.getBoundingClientRect(); + // Push the anchor toward the container bottom (40px above it). + const cr0 = card.getBoundingClientRect(); + vp.scrollTop = Math.max(0, vp.scrollTop + (cr0.top - (mr.bottom - 40))); + const cr = card.getBoundingClientRect(); + const mrNow = margin.getBoundingClientRect(); + return cr.bottom <= mrNow.bottom + 1 + ? 'fully-visible' + : `cut by ${Math.round(cr.bottom - mrNow.bottom)}px`; +})() +EOF +) +result=$(poll_eval "$fit_js" '"fully-visible"') +assert_eq "Bottom-anchored card lifted fully into view" "$result" '"fully-visible"' + screenshot "09-scroll-tracking" # =================================================================== diff --git a/src/cli_review.rs b/src/cli_review.rs index ac655bb..a7f2afe 100644 --- a/src/cli_review.rs +++ b/src/cli_review.rs @@ -247,6 +247,26 @@ fn run_agent(share: Option<&str>, mode: &str, relay_url_override: Option<&str>) crate::review::agent::run(share, mode, relay_url_override) } +/// Validate an invite client-side before shipping it to the daemon. +/// +/// The daemon-routed join is fire-and-forget over the unix socket: the +/// daemon parses the invite asynchronously and a malformed one fails +/// *silently* from the CLI's perspective — `attn review join ` +/// used to print "join request sent" and exit 0 (e.g. when a user pasted +/// the ShareDialog's full `npx attnmd review join 'attn://…'` one-liner as +/// the invite). Reuse the same `parse_invite` the daemon runs so the CLI +/// rejects exactly what the daemon would. +fn validate_invite_for_join(invite: &str) -> Result<()> { + crate::review::bootstrap::parse_invite(invite).map_err(|e| { + anyhow::anyhow!( + "invalid review invite ({e}).\n\ + Expected an invite URL like attn://review/#key= — \ + copy the link from the owner's Share dialog." + ) + })?; + Ok(()) +} + /// Hand the invite to the running attn daemon so it joins as its OWN device /// identity (the same device the app window presents). This keeps the CLI join /// consistent with the daemon — a no-`--as-agent` join shows up in the app. @@ -255,6 +275,7 @@ fn run_agent(share: Option<&str>, mode: &str, relay_url_override: Option<&str>) /// socket, then deliver the invite. That keeps the invite one-liner useful for /// first-time reviewers instead of requiring a separate "open attn" step. fn run_join_via_daemon(invite: &str) -> Result<()> { + validate_invite_for_join(invite)?; crate::daemon::replace_stale_daemon().context("check running attn daemon")?; match crate::daemon::send_review_join(invite) { Ok(()) => { @@ -487,6 +508,37 @@ mod tests { } } + #[test] + fn daemon_join_rejects_garbage_invites_before_sending() { + // A non-invite (here: the ShareDialog's full npx one-liner pasted as + // the invite) must fail loudly client-side instead of being shipped + // to the daemon, which parses it asynchronously and fails silently. + for garbage in [ + "npx attnmd review join 'attn://review/abc#key=AAAA'", + "not-an-invite", + "", + "attn://wrong/abc#key=AAAA", + ] { + let err = validate_invite_for_join(garbage) + .expect_err(&format!("garbage invite must be rejected: {garbage:?}")); + let msg = format!("{err:#}"); + assert!( + msg.contains("attn://review/"), + "error should show the expected invite shape, got: {msg}" + ); + } + } + + #[test] + fn daemon_join_accepts_a_well_formed_invite() { + // Build a real invite the same way the owner's Share path does, so + // this stays in lockstep with `parse_invite`. + let secret = [0x5Cu8; 32]; + let room_id = crate::review::crypto::kdf::derive_room_id(&secret); + let invite = crate::review::bootstrap::build_invite_url(&room_id, &secret); + validate_invite_for_join(&invite).expect("well-formed invite must validate"); + } + #[test] fn attn_home_target_rejects_empty_path() { let err = normalize_attn_home(Path::new("")).expect_err("empty path should fail"); diff --git a/src/ipc.rs b/src/ipc.rs index 322250b..e5d2467 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -457,10 +457,16 @@ pub fn handle_message(body: &str, state: &Arc>, proxy: &EventLoo submit_review_command(state, ReviewCommand::SendCollab { room_id, payload }); } IpcMessage::ReviewSetDisplayName { name } => { - // Direct identity write (not a ReviewManager command). The next - // Share/Join reads the updated identity. Don't log the name. + // Direct identity write, then a manager command to re-announce + // the new name into every ACTIVE room — the onboarding prompt + // fires after a room is entered, so without the re-announce a + // name typed there never reached the already-joined room and + // comments kept showing the stale identity. Don't log the name. match crate::review::bootstrap::set_display_name(&name) { - Ok(_) => tracing::info!("review display name updated"), + Ok(_) => { + tracing::info!("review display name updated"); + submit_review_command(state, ReviewCommand::ReannounceIdentity); + } Err(e) => tracing::warn!("failed to set display name: {e}"), } } diff --git a/src/main.rs b/src/main.rs index da49b25..5defaa2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -316,6 +316,12 @@ fn run_daemon(cli: Cli, path: PathBuf) -> Result<()> { "displayNameSet": review_display_name_set, }, "ipcToken": &ipc_token, + // Debug builds only: lets the frontend expose the session IPC token + // to the automation bridge (`window.__attn_ipc_token__`) so E2E + // suites driving raw `window.ipc.postMessage` via `--eval` pass the + // capability gate. Release builds never set this, so the token stays + // confined to the init payload there. See web/src/App.svelte. + "debugBuild": cfg!(debug_assertions), }) .to_string(); let page_html = build_page_html(&init_payload_json, theme); diff --git a/src/review/bootstrap.rs b/src/review/bootstrap.rs index 8a531bb..ff9f155 100644 --- a/src/review/bootstrap.rs +++ b/src/review/bootstrap.rs @@ -791,17 +791,40 @@ impl Bootstrapper { policy: &RoomPolicy, identity: &DeviceIdentity, now_ms: u64, + ) -> Result<(), BootstrapError> { + self.announce_participant( + room_id, + room_keys, + policy, + identity, + ParticipantKind::Owner, + now_ms, + ) + } + + /// Sign + enqueue a `ParticipantJoined` announce for the local identity + /// with the given kind. Shared by the owner's share-time announce and + /// `reannounce_identity` (display-name changes). Re-announcing is + /// harmless: frontends key names by participantId, last write wins. + fn announce_participant( + &self, + room_id: &RoomId, + room_keys: &RoomKeys, + policy: &RoomPolicy, + identity: &DeviceIdentity, + kind: ParticipantKind, + now_ms: u64, ) -> Result<(), BootstrapError> { let participant_id = identity.typed_participant_id(); let device_id = identity.typed_device_id(); - let owner_participant = Participant { + let participant = Participant { participant_id: participant_id.clone(), display_name: identity.effective_display_name(), - kind: ParticipantKind::Owner, + kind, public_signing_key: identity.public_signing_key.clone(), - capabilities: agent_capabilities(ParticipantKind::Owner), + capabilities: agent_capabilities(kind), }; - let owner_device = Device { + let device = Device { device_id: device_id.clone(), participant_id: participant_id.clone(), public_encryption_key: identity.public_encryption_key.clone(), @@ -810,8 +833,8 @@ impl Bootstrapper { created_at: now_ms, }; let joined_body = ReviewEventBody::ParticipantJoined { - participant: owner_participant, - device: owner_device, + participant, + device, }; let joined_envelope = assemble_event_envelope(AssembleInput { event_key: *room_keys.event_key.as_bytes(), @@ -827,17 +850,41 @@ impl Bootstrapper { kind: EnvelopeKind::Event, client_nonce: None, }) - .map_err(|e| { - BootstrapError::Crypto(format!("assemble owner ParticipantJoined envelope: {e}")) - })?; + .map_err(|e| BootstrapError::Crypto(format!("assemble ParticipantJoined envelope: {e}")))?; self.store .append_outbox(room_id, &joined_envelope) - .map_err(|e| { - BootstrapError::Store(format!("append owner ParticipantJoined outbox: {e}")) - })?; + .map_err(|e| BootstrapError::Store(format!("append ParticipantJoined outbox: {e}")))?; Ok(()) } + /// Re-announce the local identity into an existing room — the fix for + /// "I renamed myself but my comments still show the old name": the + /// original `ParticipantJoined` is emitted at share/join time with the + /// then-current name, and the onboarding NamePrompt fires AFTER a room + /// is entered, so a name typed there never reached already-active + /// rooms. Reads the room secret + policy from disk; the caller supplies + /// the participant kind (owner for locally-shared rooms, reviewer + /// otherwise). + pub fn reannounce_identity( + &self, + room_id: &RoomId, + kind: ParticipantKind, + ) -> Result<(), BootstrapError> { + let identity_dir = self.config.identity_dir()?; + let identity = load_or_create_identity_in(&identity_dir)?; + let secret = load_room_secret(self.store.root(), room_id)?; + let keys = derive_room_keys(&secret); + let now_ms = unix_now_ms(); + let policy = self + .store + .load_room(room_id) + .ok() + .flatten() + .map(|r| r.policy) + .unwrap_or_else(|| default_room_policy(now_ms)); + self.announce_participant(room_id, &keys, &policy, &identity, kind, now_ms) + } + pub async fn share( &self, path: PathBuf, @@ -1276,19 +1323,31 @@ impl Bootstrapper { // 5. Persist a local `ReviewRoom`. The reviewer view starts empty — // documents/snapshots fill in as snapshot envelopes arrive. - let review_room = ReviewRoom { - v: 2, - room_id: parsed.room_id.clone(), - created_at: now_ms, - created_by: participant_id.clone(), - policy, - documents: Default::default(), - snapshots: Default::default(), - event_heads: vec![], - }; - self.store - .save_room(&review_room) - .map_err(|e| BootstrapError::Store(format!("save_room: {e}")))?; + // Only on FIRST join though: a re-join over persisted state + // (same invite pasted twice, daemon restart + join) must not + // clobber the room.json that already accumulated + // documents/snapshots/event_heads — overwriting it with this + // empty shell orphaned the locally stored room state (attn-6dd). + let already_known = self + .store + .load_room(&parsed.room_id) + .map_err(|e| BootstrapError::Store(format!("load_room: {e}")))? + .is_some(); + if !already_known { + let review_room = ReviewRoom { + v: 2, + room_id: parsed.room_id.clone(), + created_at: now_ms, + created_by: participant_id.clone(), + policy, + documents: Default::default(), + snapshots: Default::default(), + event_heads: vec![], + }; + self.store + .save_room(&review_room) + .map_err(|e| BootstrapError::Store(format!("save_room: {e}")))?; + } save_room_secret(self.store.root(), &parsed.room_id, &parsed.room_secret)?; Ok(JoinOutcome { @@ -3043,6 +3102,153 @@ mod tests { assert_eq!(envelopes.len(), 1); } + #[tokio::test] + async fn rejoin_preserves_existing_room_state() { + // attn-6dd: step 5 of `join_with_identity` used to unconditionally + // overwrite room.json with a fresh empty `ReviewRoom`. On a RE-join + // (same invite pasted again, or a daemon restart followed by a join) + // that clobbered the documents/snapshots/event_heads the room had + // accumulated. Re-join must keep the existing room.json intact. + let server = MockServer::start().await; + let id_dir = TempDir::new().expect("id tempdir"); + let (_store_tmp, store, boot) = + make_bootstrapper(server.uri(), id_dir.path().to_path_buf()); + + let secret = [0x4Bu8; 32]; + let room_id = derive_room_id(&secret); + let invite = build_invite_url(&room_id, &secret); + + Mock::given(method("POST")) + .and(path_regex_for_room_create()) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "roomId": "x", + "createdAt": 0u64, + "expiresAt": 0u64, + "policy": {}, + "ownerSigningKeyId": "k", + "serverSeq": 0, + }))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path_regex_for_devices()) + .respond_with(ResponseTemplate::new(204)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_regex_for_devices()) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({ "devices": [] })), + ) + .mount(&server) + .await; + + // First join persists the fresh empty room. + boot.join(&invite, None).await.expect("first join"); + let mut room = store + .load_room(&room_id) + .expect("load") + .expect("room.json exists after first join"); + assert!(room.event_heads.is_empty()); + + // Simulate accumulated state between sessions. + let head: crate::review::ids::EventId = + serde_json::from_value(serde_json::Value::String("evt-head-1".into())) + .expect("EventId deserializes"); + room.event_heads = vec![head.clone()]; + store.save_room(&room).expect("save mutated room"); + + // Re-join with the same invite must NOT reset the room. + boot.join(&invite, None).await.expect("re-join"); + let room_after = store + .load_room(&room_id) + .expect("load") + .expect("room.json still exists"); + assert_eq!( + room_after.event_heads, + vec![head], + "re-join must not clobber room.json with an empty ReviewRoom" + ); + } + + #[tokio::test] + async fn reannounce_identity_emits_participant_joined_with_fresh_name() { + // The onboarding NamePrompt fires AFTER a room is entered, so the + // join-time ParticipantJoined carries the stale default name. + // `reannounce_identity` must enqueue a fresh PJ with the renamed + // identity so all windows re-resolve the author. + let server = MockServer::start().await; + let id_dir = TempDir::new().expect("id tempdir"); + let (_store_tmp, store, boot) = + make_bootstrapper(server.uri(), id_dir.path().to_path_buf()); + + let secret = [0x5Cu8; 32]; + let room_id = derive_room_id(&secret); + let invite = build_invite_url(&room_id, &secret); + + Mock::given(method("POST")) + .and(path_regex_for_room_create()) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "roomId": "x", + "createdAt": 0u64, + "expiresAt": 0u64, + "policy": {}, + "ownerSigningKeyId": "k", + "serverSeq": 0, + }))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path_regex_for_devices()) + .respond_with(ResponseTemplate::new(204)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_regex_for_devices()) + .respond_with( + ResponseTemplate::new(200).set_body_json(serde_json::json!({ "devices": [] })), + ) + .mount(&server) + .await; + + boot.join(&invite, None).await.expect("join"); + + // Rename AFTER joining — the user's exact flow. + set_display_name_in(id_dir.path(), "Reader").expect("rename"); + boot.reannounce_identity(&room_id, ParticipantKind::Reviewer) + .expect("reannounce"); + + let envelopes: Vec = store + .iter_outbox(&room_id) + .expect("iter") + .collect::>() + .expect("decode"); + // Join-time PJ plus the re-announce. + assert_eq!(envelopes.len(), 2); + let env = &envelopes[1]; + assert_eq!(env.kind, EnvelopeKind::Event); + + // The re-announce decrypts to a participant_joined carrying the + // renamed identity. + let keys = derive_room_keys(&secret); + let aad = crate::review::envelope::envelope_aad(env); + let nonce_bytes = URL_SAFE_NO_PAD + .decode(env.nonce.as_bytes()) + .expect("nonce decodes"); + let nonce: crate::review::crypto::aead::AeadNonce = + nonce_bytes.as_slice().try_into().expect("24-byte nonce"); + let ciphertext = URL_SAFE_NO_PAD + .decode(env.ciphertext.as_bytes()) + .expect("ciphertext decodes"); + let plaintext = + crate::review::crypto::aead::open(keys.event_key.as_bytes(), &nonce, &ciphertext, &aad) + .expect("event opens under eventKey"); + let event: serde_json::Value = serde_json::from_slice(&plaintext).expect("event JSON"); + assert_eq!(event["body"]["type"], "participant_joined"); + assert_eq!(event["body"]["participant"]["displayName"], "Reader"); + assert_eq!(event["body"]["participant"]["kind"], "reviewer"); + } + #[tokio::test] async fn join_with_malformed_invite_errors_without_network() { let server = MockServer::start().await; diff --git a/src/review/manager.rs b/src/review/manager.rs index 43114aa..b8e67e2 100644 --- a/src/review/manager.rs +++ b/src/review/manager.rs @@ -116,6 +116,10 @@ pub enum ReviewCommand { /// broadcast) from this webview to the room over the encrypted signal /// channel. `payload` is opaque JSON the daemon doesn't parse. SendCollab { room_id: RoomId, payload: String }, + /// The local display name changed: re-announce the identity into every + /// active room so existing comments resolve to the new name everywhere + /// (the original ParticipantJoined was frozen at share/join time). + ReannounceIdentity, } /// Updates the `ReviewManager` emits up to the tao event loop. @@ -617,6 +621,40 @@ impl ReviewManager { self.stop_rooms(room_id.clone()); return; } + (ReviewCommand::ReannounceIdentity, Some(bootstrapper), Some(runtime)) => { + // Display name changed: refresh the ParticipantJoined + // announce in every active room so existing comments resolve + // to the new name on all windows (frontends key names by + // participantId; last write wins). The onboarding NamePrompt + // fires AFTER a room is entered, so without this a name typed + // there never reached the already-joined room. Drain each + // room's outbox immediately so the rename lands without + // waiting for the next scheduled pass; a failed drain is fine + // (the envelope is durably queued). + let rooms: Vec<( + RoomId, + Arc, + )> = self + .outboxes + .lock() + .map(|m| m.iter().map(|(k, v)| (k.clone(), Arc::clone(v))).collect()) + .unwrap_or_default(); + for (room_id, outbox) in rooms { + let kind = match crate::review::bootstrap::find_path_for_room( + self.store.root(), + &room_id, + ) { + Ok(Some(_)) => crate::review::model::ParticipantKind::Owner, + _ => crate::review::model::ParticipantKind::Reviewer, + }; + if let Err(e) = bootstrapper.reannounce_identity(&room_id, kind) { + tracing::warn!("reannounce into room {} failed: {e}", room_id.as_str()); + continue; + } + let _ = runtime.block_on(outbox.process_once()); + } + return; + } (ReviewCommand::Pull { room_id }, _, _) => { self.pull_rooms(room_id.clone()); return; @@ -1396,9 +1434,9 @@ impl ReviewManager { &self, result: Result, ) { - let update = match result { + match result { Ok(outcome) => { - let room_id = outcome.room_id.clone(); + let room_id = outcome.room_id; // Start the transport runtime BEFORE emitting status so // by the time the frontend reacts to "Joined" the WS // subscriber is already listening. If init fails we @@ -1411,18 +1449,24 @@ impl ReviewManager { message: format!("could not start room transports: {err}"), }); } - ReviewUpdate::RoomStatusChanged { - room_id: outcome.room_id, + (self.update_tx)(ReviewUpdate::RoomStatusChanged { + room_id: room_id.clone(), status: "Joined".to_string(), - } + }); + // A RE-join over persisted state gets nothing re-delivered + // by the relay (the WS cursor already advanced past the + // session-1 envelopes), so replay the on-disk log. Fresh + // joins have an empty events.jsonl — a no-op (attn-6dd). + self.replay_room_to_webview(&room_id); } - Err(err) => ReviewUpdate::Error { - room_id: None, - code: error_code(&err), - message: err.to_string(), - }, - }; - (self.update_tx)(update); + Err(err) => { + (self.update_tx)(ReviewUpdate::Error { + room_id: None, + code: error_code(&err), + message: err.to_string(), + }); + } + } } /// Spawn the outbox drain + inbound WS subscriber for `room_id` onto the @@ -1982,15 +2026,93 @@ impl ReviewManager { // surfaces appear. Without this push the reviewer's UI shows // only the local file tree even though the WS subscription // is already streaming inbound envelopes. + // + // The lifecycle string is role-accurate: a room WE shared (a + // local share binding exists) resumes as the owner's "Live"; a + // room we joined resumes as "Joined". The frontend activates the + // room on both and derives the role from the string — the old + // neutral "Resumed" was passive (switcher-only) and left the + // role 'unknown', so a restarted reviewer could never flip back + // into the shared-doc view even with the snapshot replayed + // (attn-6dd). Owners don't flip either way (isReviewerView + // requires role 'reviewer'), so a resumed share never hijacks + // the owner's local file view (attn-0wa). + let is_owner = + crate::review::bootstrap::find_path_for_room(self.store.root(), &room_id) + .ok() + .flatten() + .is_some(); (self.update_tx)(ReviewUpdate::RoomStatusChanged { room_id: room_id.clone(), - status: "Resumed".to_string(), + status: if is_owner { "Live" } else { "Joined" }.to_string(), }); + // Re-feed the webview everything the room already imported in + // earlier sessions. The WS mailbox resumes at its persisted + // cursor, so the relay never re-delivers the envelopes behind + // events.jsonl — without this replay a resumed reviewer (or a + // restarted owner) renders "Waiting for the shared document…" + // forever even though the snapshot sits on disk (attn-6dd). + self.replay_room_to_webview(&room_id); resumed.push(room_id); } resumed } + /// Replay a room's persisted state to the webview using the SAME + /// `ReviewUpdate::EventImported` pushes the live inbound pipeline emits + /// on fresh import (see `forward_transport_event`), in `events.jsonl` + /// log order. + /// + /// `SnapshotCreated` events persist in wire form (`inline_snapshot: + /// None`, decision #14); `rehydrate_snapshot_event` fills the plaintext + /// from the local blob store at this IPC boundary exactly like the live + /// forwarder does, so the webview receives the shared document's + /// markdown and flips from "Waiting for the shared document…" to the + /// snapshot view. Snapshots therefore ride the `reviewEvent` lane here + /// too — that is the only snapshot delivery path the frontend store + /// consumes (`applyEvent` mirrors `snapshot_created` events into its + /// `snapshots` view). + /// + /// Safe to run before, after, or concurrently with live imports: the + /// frontend dedups events by `eventId` and snapshots by `snapshotId` + /// (`store.svelte.ts` `applyEvent`). Decode errors skip the bad line + /// instead of aborting so one corrupt entry can't hide the rest of the + /// log. + pub(crate) fn replay_room_to_webview(&self, room_id: &RoomId) { + let events = match self.store.iter_events(room_id) { + Ok(iter) => iter, + Err(err) => { + tracing::warn!("replay iter_events for {}: {err}", room_id.as_str()); + return; + } + }; + let mut replayed = 0usize; + for entry in events { + let mut event = match entry { + Ok(event) => event, + Err(err) => { + tracing::warn!( + "replay skipping undecodable event in {}: {err}", + room_id.as_str() + ); + continue; + } + }; + rehydrate_snapshot_event(&self.store, room_id, &mut event); + (self.update_tx)(ReviewUpdate::EventImported { + room_id: room_id.clone(), + event, + }); + replayed += 1; + } + if replayed > 0 { + tracing::info!( + "replayed {replayed} persisted event(s) to the webview for room={}", + room_id.as_str() + ); + } + } + /// Push a freshly-resolved anchor to the frontend. /// /// Issue attn-nnj.3.8: the callback path Rust → tao event loop → @@ -2502,6 +2624,7 @@ fn review_command_name(cmd: &ReviewCommand) -> &'static str { ReviewCommand::ResolveComment { .. } => "ResolveComment", ReviewCommand::SendCollab { .. } => "SendCollab", ReviewCommand::PublishSnapshot { .. } => "PublishSnapshot", + ReviewCommand::ReannounceIdentity => "ReannounceIdentity", } } @@ -2638,6 +2761,12 @@ fn stub_update_for(cmd: &ReviewCommand) -> ReviewUpdate { room_id: stub_room_id(), status: "Pending suggestion reject — no bootstrap attached".to_string(), }, + // ReannounceIdentity iterates active rooms in `submit`; without a + // bootstrap (smoke tests) it's a benign no-op surfaced as status. + ReviewCommand::ReannounceIdentity => ReviewUpdate::RoomStatusChanged { + room_id: stub_room_id(), + status: "Pending identity reannounce — no bootstrap attached".to_string(), + }, } } @@ -3196,6 +3325,123 @@ mod tests { )); } + // ----- replay of persisted room state on resume/re-join (attn-6dd) ---- + + #[test] + fn replay_room_to_webview_emits_persisted_events_with_rehydrated_snapshots() { + use crate::review::crypto::ids::content_hash; + use crate::review::model::{ + AnchorIndex, BlobRef, BlobStorage, CanonicalEncoding, ReviewEventBody, + SnapshotPlaintext, + }; + + let (mgr, rx, tmp) = make_manager(); + // Second handle onto the same on-disk store to seed "session 1" state. + let store = ReviewStore::open_at(tmp.path().join("reviews")).expect("open store"); + let room_id: RoomId = dummy_id("room-replay"); + + // Persist the snapshot blob the SnapshotCreated event references — + // this is what the inbound pipeline leaves behind for a reviewer + // (events.jsonl + blobs/*.bin; no SnapshotNode on the reviewer side). + let plaintext = SnapshotPlaintext { + markdown: "# replayed doc\n".to_string(), + anchor_index: AnchorIndex { + doc_hash: dummy_id("hash-1"), + canonical_encoding: CanonicalEncoding::Utf8Bytes, + line_count: 1, + blocks: vec![], + headings: vec![], + }, + }; + let blob_bytes = crate::review::crypto::canonical::to_canonical_bytes(&plaintext) + .expect("canonical snapshot"); + store + .save_snapshot_blob(&room_id, "blob-env-replay", &blob_bytes) + .expect("save blob"); + let blob_ref = BlobRef { + storage: BlobStorage::Mailbox, + blob_id: "blob-env-replay".to_string(), + byte_length: blob_bytes.len() as u64, + content_hash: content_hash(&blob_bytes), + }; + + // Seed events.jsonl in log order: snapshot first (wire form, no + // inline plaintext — decision #14), then a comment. + let mut snapshot_event = stub_review_event( + &room_id, + ReviewEventBody::SnapshotCreated { + file_id: dummy_id("file-1"), + snapshot_id: dummy_id("snap-1"), + owner_display_path: Some("/tmp/doc.md".to_string()), + parent_snapshot_id: None, + base_hash: dummy_id("hash-1"), + encrypted_blob_ref: Some(blob_ref), + inline_snapshot: None, + }, + ); + snapshot_event.meta.event_id = dummy_id("evt-snap-1"); + let mut comment_event = stub_review_event( + &room_id, + ReviewEventBody::CommentCreated { + thread_id: "thr-1".to_string(), + anchor: dummy_anchor(), + body: "persisted comment".to_string(), + }, + ); + comment_event.meta.event_id = dummy_id("evt-comment-1"); + assert!( + store + .append_event(&room_id, &snapshot_event) + .expect("append") + ); + assert!( + store + .append_event(&room_id, &comment_event) + .expect("append") + ); + + // "Session 2": replay must re-emit both events through the same + // EventImported push the live inbound pipeline uses, in log order, + // with the snapshot's plaintext rehydrated from the blob store. + mgr.replay_room_to_webview(&room_id); + + let first = rx.try_recv().expect("first replayed update"); + match first { + ReviewUpdate::EventImported { + room_id: rid, + event, + } => { + assert_eq!(rid, room_id); + assert_eq!(event.meta.event_id, dummy_id::("evt-snap-1")); + match event.body { + ReviewEventBody::SnapshotCreated { + inline_snapshot: Some(inline), + .. + } => assert_eq!(inline.markdown, "# replayed doc\n"), + other => panic!("expected rehydrated SnapshotCreated, got {other:?}"), + } + } + other => panic!("expected EventImported, got {other:?}"), + } + let second = rx.try_recv().expect("second replayed update"); + match second { + ReviewUpdate::EventImported { event, .. } => { + assert_eq!(event.meta.event_id, dummy_id::("evt-comment-1")); + assert!(matches!(event.body, ReviewEventBody::CommentCreated { .. })); + } + other => panic!("expected EventImported, got {other:?}"), + } + assert!( + rx.try_recv().is_err(), + "replay must emit exactly the persisted events" + ); + + // A room with no persisted log replays nothing (fresh join). + let empty_room: RoomId = dummy_id("room-empty"); + mgr.replay_room_to_webview(&empty_room); + assert!(rx.try_recv().is_err(), "empty room must replay nothing"); + } + /// Build a `(ReviewManager, receiver)` pair backed by an std::mpsc channel /// so tests can assert which `ReviewUpdate`s the manager emitted. fn make_manager() -> (ReviewManager, mpsc::Receiver, TempDir) { diff --git a/web/src/App.svelte b/web/src/App.svelte index d5dad67..6e75668 100644 --- a/web/src/App.svelte +++ b/web/src/App.svelte @@ -589,6 +589,19 @@ }); } + // Keep the live caret label in sync with renames: the onboarding + // NamePrompt fires AFTER a room is entered, so the label captured when + // the CollabController was constructed (the git/OS default) goes stale + // the moment the user picks a real name — peers' caret chips kept + // showing the old name (attn-k1n follow-up). The controller + // re-broadcasts the caret so the rename lands immediately. + $effect(() => { + const name = userProfile.effectiveName; + collabController?.setSelfLabel( + name || (collabRole === 'owner' ? 'Owner' : 'Reviewer'), + ); + }); + // Base (earliest) snapshot markdown for a file in the current room. This is // the v0 every authority + resync replay anchors to, so the live editor seeds // from it (NOT the latest republished snapshot) and the full step log rebases @@ -1684,6 +1697,14 @@ // Capture the IPC capability token before we clear the payload — `send()` // attaches it to privileged messages so the daemon accepts them. setIpcToken(init.ipcToken); + // DEBUG BUILDS ONLY (daemon sets `debugBuild` from cfg!(debug_assertions), + // never in release): expose the session token on the main frame so the + // automation bridge (`attn --eval`, E2E suites) can drive privileged raw + // `window.ipc.postMessage` calls past the capability gate. Sandboxed + // HtmlViewer iframes have their own `window` and still never see it. + if (init.debugBuild && init.ipcToken) { + (window as { __attn_ipc_token__?: string }).__attn_ipc_token__ = init.ipcToken; + } // Clear so we only process once (prevents $effect re-entry) delete (window as { __attn_init__?: InitPayload }).__attn_init__; @@ -2644,13 +2665,22 @@ CSS transitions in occluded windows, which left the rail stuck mid-transition at ~1px (attn-23m), and an animating width also desyncs the breadcrumb inset. --> +