Fix dev:collab reviewer flow: room replay on resume, invite validation, harness repair

.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"}}

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*

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" ]; }

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"
 
 # ===================================================================

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 <garbage>`
+/// 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/<roomId>#key=<secret> — \
+             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");

src/ipc.rs

@@ -457,10 +457,16 @@ pub fn handle_message(body: &str, state: &Arc<Mutex<AppState>>, 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}"),
                 }
             }

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);