From 6e8f97ac2978d169d482c6700f2a7bca70de7e51 Mon Sep 17 00:00:00 2001 From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:45:13 -0500 Subject: [PATCH 1/9] Replay persisted room state to the webview on resume/re-join (attn-6dd) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On any second session over persisted state, the WS mailbox reconnects at its stored cursor, so the relay never re-delivers session-1 envelopes — the on-disk store had everything but nothing pushed it to the webview, leaving a restarted reviewer on "Waiting for the shared document…" forever. - ReviewManager::replay_room_to_webview: re-emit every persisted event through the same ReviewUpdate::EventImported push the live inbound pipeline uses, in events.jsonl order, with SnapshotCreated plaintext rehydrated from the local blob store (rehydrate_snapshot_event), so snapshots ride the reviewEvent lane exactly like a fresh import. Safe against double-delivery: the frontend dedups by eventId/snapshotId. Called from both resume_known_rooms and the join path. - resume_known_rooms now emits a role-accurate lifecycle ("Live" for rooms we shared, "Joined" for rooms we joined) instead of the passive "Resumed", so the frontend activates the room and a restarted reviewer flips back into the shared-doc view; owners never flip (role gate, attn-0wa). - join_with_identity step 5 no longer clobbers room.json on re-join: the fresh empty ReviewRoom is only written when none exists, so accumulated documents/snapshots/event_heads survive a repeat join. Tests: replay unit test (manager), re-join preservation test (bootstrap); verified end-to-end with a persisted-second-session probe (owner shares, reviewer joins, both daemons killed and rebooted on the same ATTN_HOMEs — reviewer rehydrates the shared doc without a new join/share). Co-Authored-By: Claude Fable 5 --- src/review/bootstrap.rs | 108 ++++++++++++-- src/review/manager.rs | 222 +++++++++++++++++++++++++++-- web/src/lib/review/store.svelte.ts | 5 +- 3 files changed, 308 insertions(+), 27 deletions(-) diff --git a/src/review/bootstrap.rs b/src/review/bootstrap.rs index 8a531bb..42ccee1 100644 --- a/src/review/bootstrap.rs +++ b/src/review/bootstrap.rs @@ -1276,19 +1276,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 +3055,76 @@ 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 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..ef95859 100644 --- a/src/review/manager.rs +++ b/src/review/manager.rs @@ -1396,9 +1396,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 +1411,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 +1988,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 → @@ -3196,6 +3280,118 @@ 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/lib/review/store.svelte.ts b/web/src/lib/review/store.svelte.ts index b316297..1af5935 100644 --- a/web/src/lib/review/store.svelte.ts +++ b/web/src/lib/review/store.svelte.ts @@ -463,7 +463,10 @@ export class ReviewStore { /** * Apply a transport/connection status payload pushed by Rust. Joined/Live - * statuses activate the room; Resumed only populates the switcher. + * statuses activate the room; anything else only populates the switcher. + * Daemon-resumed rooms arrive role-accurately as Live (we shared it) or + * Joined (we joined it) so a restarted reviewer re-enters the shared-doc + * view (attn-6dd); owners never flip regardless (role gate in room-ui). */ applyStatus(status: ReviewStatus): void { const roomId = status.roomId; From f45b4737b9124a6124d84a897cb11e3bcf94b787 Mon Sep 17 00:00:00 2001 From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:45:31 -0500 Subject: [PATCH 2/9] Repair test-editorial-e2e.sh after the IPC capability-token gate (attn-ry9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite drove four privileged IPC messages via raw window.ipc.postMessage, all rejected since the capability-token gate landed ("rejected privileged IPC 'review_join' without a valid token"): - The join now goes through the daemon-routed CLI (`attn review join`), which rides the unix socket straight into the ReviewManager — same path dev-collab uses, no token needed. - The remaining three sites (reply, resolve, stop) exercise the exact wire messages their buttons send, so they keep the raw postMessage form and now pass the session token. Debug builds expose it to the automation bridge: the daemon marks the init payload with debugBuild = cfg!(debug_assertions), and App.svelte then mirrors the token to window.__attn_ipc_token__. Release builds never set the flag, so the token stays confined to the init payload there; sandboxed HtmlViewer iframes have their own window and never see it. Suite: 18 passed, 0 failed (was 14/4 with the CLI join alone, 7 before). Co-Authored-By: Claude Fable 5 --- scripts/test-editorial-e2e.sh | 13 +++++++++---- src/main.rs | 6 ++++++ web/src/App.svelte | 8 ++++++++ web/src/lib/types.ts | 6 ++++++ 4 files changed, 29 insertions(+), 4 deletions(-) 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/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/web/src/App.svelte b/web/src/App.svelte index d5dad67..7e235d6 100644 --- a/web/src/App.svelte +++ b/web/src/App.svelte @@ -1684,6 +1684,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__; diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index 452a320..0b69172 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -311,6 +311,12 @@ export interface InitPayload { * so scripts in a sandboxed HTML file cannot drive the app. @see src/ipc.rs */ ipcToken?: string; + /** + * True only when the daemon is a debug build (`cfg!(debug_assertions)`). + * Gates exposing the IPC token to the automation bridge + * (`window.__attn_ipc_token__`) for E2E suites; never set in release. + */ + debugBuild?: boolean; } /** From ace07353bdce4ac22b7b3f61247d570bc27f0422 Mon Sep 17 00:00:00 2001 From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:45:52 -0500 Subject: [PATCH 3/9] Validate review-join invites client-side before handing them to the daemon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `attn review join ` printed "join request sent to the running attn daemon" and exited 0 — the daemon parses the invite asynchronously and a malformed one (e.g. the ShareDialog's full `npx attnmd review join 'attn://…'` one-liner pasted as the invite) failed silently while the CLI claimed success. The daemon-routed join now runs the same parse_invite the daemon uses before sending; on failure it prints the expected invite shape (attn://review/#key=) and exits non-zero. Co-Authored-By: Claude Fable 5 --- src/cli_review.rs | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) 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"); From a92b41e9970769838fd9d89683f1025338a0b5ad Mon Sep 17 00:00:00 2001 From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:55:50 -0500 Subject: [PATCH 4/9] dev-collab: extract the invite URL from whatever gets pasted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Share dialog's primary copyable is the full `npx attnmd review join 'attn://…'` one-liner, but the script's prompt passed the pasted text to `review join` verbatim — the daemon failed to parse it while the script still claimed "Reviewer joined". Extract the attn://review/… URL from the paste (bare URL, quoted, or npx-wrapped all work) and error clearly when none is present. Pairs with the new client-side invite validation in the CLI, which makes the script's success message truthful. Co-Authored-By: Claude Fable 5 --- scripts/dev-collab.sh | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) 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* From f529435c782462d1049b76af982686d7a0fbfe0a Mon Sep 17 00:00:00 2001 From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:23:22 -0500 Subject: [PATCH 5/9] Rail edge polish, theme focus rings, display-name changes propagate to active rooms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rail: dropped the aside border-t (it stacked with the rail header's border-b into one thick line); cards/chips now clip 8px above the rail's bottom edge (overflow wrapper + mb-2) — the bottom counterpart of the top breathing-room clamp. - Textareas (reply, comment, suggestion fields) show the theme's border-ring + ring-ring/50 focus treatment instead of the native blue WebKit ring, mirroring the shared Input component. Plain :focus, not :focus-visible — WebKit doesn't reliably match the latter on textareas. - Renaming yourself now reaches rooms you're already in: the ParticipantJoined announce was frozen at share/join time, and the onboarding NamePrompt fires AFTER a room is entered, so a name typed there ("Reader") never replaced the git default on existing comments. ReviewSetDisplayName now also submits a ReannounceIdentity manager command that re-announces the identity (kind derived from the local share binding) into every active room and drains each outbox immediately; frontends key names by participantId, last write wins. announce_owner generalized to announce_participant; new unit test decrypts the re-announce envelope and asserts the fresh name + kind. Verified: 423 cargo review tests, svelte-check 0 errors, 33/33 web unit files, review E2E 41/41, editorial relay E2E 18/18. Live checks: rail border-top 0px, bottom clip gap 8px, compiled :focus rule present (window focus can't be held by the probe environment; the ring shows under real focus, same condition as the old native ring). Co-Authored-By: Claude Fable 5 --- .beads/interactions.jsonl | 2 + src/ipc.rs | 12 +- src/review/bootstrap.rs | 152 ++++++++++++++++++++++++-- src/review/manager.rs | 48 ++++++++ web/src/App.svelte | 12 +- web/src/lib/CommentComposer.svelte | 2 +- web/src/lib/ReviewMarginCard.svelte | 12 ++ web/src/lib/SuggestionComposer.svelte | 6 +- 8 files changed, 227 insertions(+), 19 deletions(-) diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl index 51379fd..ac573d4 100644 --- a/.beads/interactions.jsonl +++ b/.beads/interactions.jsonl @@ -198,3 +198,5 @@ {"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"}} 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/review/bootstrap.rs b/src/review/bootstrap.rs index 42ccee1..b8e78d5 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(), @@ -809,10 +832,7 @@ impl Bootstrapper { client: DeviceClient::AttnNative, created_at: now_ms, }; - let joined_body = ReviewEventBody::ParticipantJoined { - participant: owner_participant, - device: owner_device, - }; + let joined_body = ReviewEventBody::ParticipantJoined { participant, device }; let joined_envelope = assemble_event_envelope(AssembleInput { event_key: *room_keys.event_key.as_bytes(), signing_key: identity.signing_key()?, @@ -828,16 +848,44 @@ impl Bootstrapper { client_nonce: None, }) .map_err(|e| { - BootstrapError::Crypto(format!("assemble owner ParticipantJoined envelope: {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}")) + 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, @@ -3125,6 +3173,90 @@ mod tests { ); } + #[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 ef95859..0201bbc 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,43 @@ 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; @@ -2586,6 +2627,7 @@ fn review_command_name(cmd: &ReviewCommand) -> &'static str { ReviewCommand::ResolveComment { .. } => "ResolveComment", ReviewCommand::SendCollab { .. } => "SendCollab", ReviewCommand::PublishSnapshot { .. } => "PublishSnapshot", + ReviewCommand::ReannounceIdentity => "ReannounceIdentity", } } @@ -2722,6 +2764,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(), + }, } } diff --git a/web/src/App.svelte b/web/src/App.svelte index 7e235d6..89143d8 100644 --- a/web/src/App.svelte +++ b/web/src/App.svelte @@ -2652,8 +2652,11 @@ 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. --> +