diff --git a/.beads/interactions.jsonl b/.beads/interactions.jsonl index 50d89e0..51379fd 100644 --- a/.beads/interactions.jsonl +++ b/.beads/interactions.jsonl @@ -192,3 +192,9 @@ {"id":"int-1f4f8a2b","kind":"field_change","created_at":"2026-05-28T20:54:26.223302Z","actor":"Angus Bezzina","issue_id":"attn-zb5.2","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} {"id":"int-d6c93840","kind":"field_change","created_at":"2026-05-28T20:54:26.306244Z","actor":"Angus Bezzina","issue_id":"attn-zb5.3","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} {"id":"int-4cc271e7","kind":"field_change","created_at":"2026-05-28T20:54:26.385364Z","actor":"Angus Bezzina","issue_id":"attn-zb5.4","extra":{"field":"status","new_value":"closed","old_value":"open","reason":"Closed"}} +{"id":"int-fd265fab","kind":"field_change","created_at":"2026-06-10T20:42:34.962462Z","actor":"Angus Bezzina","issue_id":"attn-d7y","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} +{"id":"int-b5122651","kind":"field_change","created_at":"2026-06-10T23:24:31.985272Z","actor":"Angus Bezzina","issue_id":"attn-0sv","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} +{"id":"int-20e67379","kind":"field_change","created_at":"2026-06-11T01:04:13.463975Z","actor":"Angus Bezzina","issue_id":"attn-42y","extra":{"field":"status","new_value":"closed","old_value":"in_progress","reason":"Closed"}} +{"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"}} diff --git a/CLAUDE.md b/CLAUDE.md index 0b1739c..8c77131 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,9 +44,11 @@ This boots three processes: user provides an invite. Interactive flow: click **[Share]** in the owner window, copy the invite URL, -paste it at the script's prompt — it runs `attn review join ---as-agent reviewer` against the reviewer daemon and the second window joins -the room. Ctrl+C cleans up all three processes. +paste it at the script's prompt — it runs `attn review join ` routed +to the reviewer daemon (deliberately NOT `--as-agent`, which forks a separate +headless agent process and leaves the reviewer window idle; the daemon-routed +join flips the reviewer's own window onto the shared document). Ctrl+C cleans +up all three processes. Env overrides: diff --git a/planning/collab/ui/review-panel-design.md b/planning/collab/ui/review-panel-design.md index 7795a5f..61c418c 100644 --- a/planning/collab/ui/review-panel-design.md +++ b/planning/collab/ui/review-panel-design.md @@ -233,31 +233,47 @@ spatial-align. ## 3. Resolved State -Resolved threads do **not** disappear. They shrink to a **single-line grey -strip** in place (same y-position as the active card would have had): +> **Amended by attn-d7y** (shipped): the full-width strip became a +> content-sized **chip**, the rail width adapts, and clicking a chip +> expands a read-only card. The original strip spec is superseded by the +> behavior below; the >5 "show all resolved" pill survives unchanged. + +Resolved threads do **not** disappear. They shrink to a **28px rounded +chip** in place (same y-position as the active card would have had): ```text -│ ─── ✓ rufus · resolved 2m · §Anchor resolver ─── │ +│ (✓ rufus · resolved) │ ← content-sized pill, full rail ``` -The strip is 24px tall, uses the `--muted-foreground` color, and is dismissed -from the document-order layout queue (it can be skipped over for collision -purposes — newer active cards collapse past it). - -If there are more than 5 collapsed strips visible in the current viewport, -the design collapses them further to a "show all resolved" pill at the -**bottom of the margin** (still inside the overlay, after the last anchored -card): +When the margin holds ONLY resolved threads (no active cards, no orphan +tray, nothing expanded) the right-rail aside slims from 320px to a **48px +gutter** and the chips render icon-only — `(✓)` centered in the gutter — +so the document reclaims the width instead of facing a vacant column. The +mode derivation lives in `web/src/lib/review/rail-mode.ts` +(`closed`/`slim`/`full`), exposed as `reviewStore.railMode`, and drives +both the aside width (App.svelte) and the chip variant (ReviewMargin). + +Clicking a chip expands it **in place to the full card** in read-only +resolved state (author, quote, body, replies, `resolved` badge) with a +single `Collapse` action — no Reply/Resolve/Accept/Reject. Expanding +forces the rail to full width; Collapse (or Escape) shrinks it back. The +expanded card participates in the SAME collision pass as active cards +(one unified `layoutCards` call), so it pushes neighbors instead of +overlapping them. + +If there are more than 5 collapsed chips visible in full mode, they +collapse further to a "show all resolved" pill at the **bottom of the +margin** (still inside the overlay, after the last anchored card): ```text │ ─────────────────────────────────── │ -│ ⌃ 12 resolved · show │ ← bottom pill, click to expand all strips +│ ⌃ 12 resolved · show │ ← bottom pill, click to expand all chips │ ─────────────────────────────────── │ ``` -Clicking the pill expands all strips inline at their anchor positions. -Clicking a strip pops it back to a full card for one cycle (until next focus -change). +Clicking the pill expands all chips inline at their anchor positions. +Slim mode ignores the pill (the 48px gutter can't fit it; icon chips are +cheap to render). --- diff --git a/scripts/test-review-e2e.sh b/scripts/test-review-e2e.sh index 28c32b4..831990c 100755 --- a/scripts/test-review-e2e.sh +++ b/scripts/test-review-e2e.sh @@ -232,11 +232,12 @@ for cb in reviewStatus reviewEvent reviewSnapshot reviewAnchorResolution; do done echo "" -echo "--- Review store scaffold (from 12.10, pending) ---" -# The review store module should be importable and expose a default shape. -# Today the module does not exist — record as PEND. +echo "--- Review store scaffold ---" +# The review store singleton is exposed for E2E seeding (App.svelte wires +# `window.__attn_review_store__` on mount). Hard assertion — the adaptive +# rail suite below depends on it. result=$("$ATTN" --eval "typeof window.__attn_review_store__") -expect_eq_soft "window.__attn_review_store__ exposed" "$result" '"object"' "attn-nnj.12.10" +assert_eq "window.__attn_review_store__ exposed" "$result" '"object"' screenshot "02-shape-asserted" @@ -321,6 +322,239 @@ echo " (run scripts/test-apply-e2e.sh to verify the contract directly)" screenshot "03-apply-flow-pending" +# =================================================================== +# TEST SUITE: Collapsible rail + identity chips (attn-d7y / attn-42y) +# =================================================================== +# +# Seeds the review store directly through `window.__attn_review_store__` +# (no relay needed): one comment thread + its CommentResolved event. In a +# review room the rail is always present: collapsed to a 48px gutter +# (✓ chips for resolved, author-avatar chips for unresolved) unless +# expanded by the user/auto-open. Clicking a ✓ chip expands the rail with +# the full read-only card (no action buttons); clicking the card shrinks +# it back to a labeled chip. The ReviewBar dock carries the rail toggle. + +echo "" +echo "=== Review E2E: collapsible rail + identity chips (attn-d7y/attn-42y) ===" + +# Poll an --eval expression until it returns the expected value (the aside +# width animates over 200ms, so one-shot reads race the transition). +poll_eval() { + local expr="$1" expected="$2" attempts=30 + local result="" + while [ $attempts -gt 0 ]; do + result=$("$ATTN" --eval "$expr" 2>/dev/null || echo "") + if [ "$result" = "$expected" ]; then + echo "$result" + return 0 + fi + sleep 0.1 + attempts=$((attempts - 1)) + done + echo "$result" +} + +"$ATTN" --wait-for '.ProseMirror' --timeout 10000 >/dev/null 2>&1 || true + +seed_js=$(cat <<'EOF' +(() => { + const s = window.__attn_review_store__; + if (!s) return 'no-store'; + const anchor = { + v: 2, fileId: 'file-x', snapshotId: 'snap-x', baseHash: 'h-x', + position: { byteRange: [0, 10], lineRange: [1, 1] }, + }; + const meta = (id) => ({ + v: 2, eventId: id, roomId: 'room-x', authorId: 'p-reviewer', + deviceId: 'd-x', createdAt: Date.now(), parentEventIds: [], + snapshotId: 'snap-x', + }); + const auth = { signature: 'sig', signingKeyId: 'kid' }; + s.applyEvent({ meta: meta('e-1'), body: { type: 'comment_created', threadId: 't-1', anchor, body: 'Consider tightening this wording.' }, auth }); + s.applyEvent({ meta: meta('e-2'), body: { type: 'comment_resolved', threadId: 't-1', resolvedBy: 'p-owner' }, auth }); + s.selectRoom('room-x'); + s.setCurrentFile('file-x'); + return 'ok'; +})() +EOF +) +result=$("$ATTN" --eval "$seed_js") +assert_eq "Seeded resolved-only review thread" "$result" '"ok"' + +echo "" +echo "--- Collapsed gutter (resolved-only margin, default) ---" +result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"collapsed"') +assert_eq "Rail data-mode is collapsed (no unresolved threads)" "$result" '"collapsed"' + +result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.offsetWidth" '48') +assert_eq "Rail width is exactly the 48px gutter" "$result" "48" + +result=$("$ATTN" --query '[data-testid="review-margin-resolved-chip"]' | jq -r '.count' 2>/dev/null || echo "0") +assert_eq "One resolved chip rendered" "$result" "1" + +result=$("$ATTN" --eval "document.querySelector('[data-testid=\\\"review-margin-resolved-chip\\\"]')?.getAttribute('data-variant') ?? 'missing'") +assert_eq "Chip is icon variant in the gutter" "$result" '"icon"' + +result=$("$ATTN" --eval "parseInt(document.querySelector('[data-testid=\\\"review-margin-resolved-chip\\\"]')?.style.top ?? '-1', 10) >= 8") +assert_eq "Gutter chip respects the inner clearance (top ≥ 8)" "$result" "true" + +result=$("$ATTN" --eval "(() => { const t = document.querySelector('[data-slot=\\\"rail-toggle\\\"]'); const rail = document.querySelector('[data-slot=\\\"right-rail\\\"]'); if (!t || !rail) return 'missing'; const tr = t.getBoundingClientRect(); const rr = rail.getBoundingClientRect(); return Math.abs((tr.left + tr.width / 2) - (rr.left + rr.width / 2)) <= 2 ? 'centered' : 'off-center'; })()") +assert_eq "Toggle is horizontally centered in the collapsed gutter" "$result" '"centered"' + +screenshot "04-resolved-collapsed-gutter" + +echo "" +echo "--- Expand ✓ chip → rail expands with read-only card ---" +"$ATTN" --click '[data-testid="review-margin-resolved-chip"]' >/dev/null 2>&1 || true + +result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"expanded"') +assert_eq "Rail expands on chip click" "$result" '"expanded"' + +result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.offsetWidth" '320') +assert_eq "Rail width is exactly 320px when expanded" "$result" "320" + +result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]') !== null" 'true') +assert_eq "Resolved card rendered" "$result" "true" + +result=$("$ATTN" --eval "parseInt(document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]')?.parentElement?.style.top ?? '-1', 10) >= 8") +assert_eq "Card keeps breathing room from the rail top (top ≥ 8)" "$result" "true" + +result=$("$ATTN" --eval "(() => { const c = document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]'); if (!c) return 'no-card'; return [c.querySelectorAll('[data-action]').length === 0, c.querySelector('.rmc-avatar') !== null].join(','); })()") +assert_eq "Card is read-only (no action buttons) with an author avatar" "$result" '"true,true"' + +result=$("$ATTN" --eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]')?.textContent.includes('Consider tightening this wording.')") +assert_eq "Card shows the resolved comment body" "$result" "true" + +screenshot "05-resolved-expanded-card" + +echo "" +echo "--- Click card → shrinks back to labeled chip (rail stays expanded) ---" +"$ATTN" --click '[data-testid="review-margin-card"][data-state="resolved"]' >/dev/null 2>&1 || true + +result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"resolved\\\"]') === null" 'true') +assert_eq "Resolved card gone after card click" "$result" "true" + +result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-resolved-chip\\\"]')?.getAttribute('data-variant') ?? 'missing'" '"label"') +assert_eq "Labeled chip back in the expanded rail" "$result" '"label"' + +result=$("$ATTN" --eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode')") +assert_eq "Rail stays expanded after card collapse" "$result" '"expanded"' + +echo "" +echo "--- Mixed margin (active + resolved) → cards + labeled chip ---" +mixed_js=$(cat <<'EOF' +(() => { + const s = window.__attn_review_store__; + if (!s) return 'no-store'; + // Anchored deep in the document (not the top band): the scroll-tracking + // suite below needs this card to move 1:1 with a 150px scroll, and the + // rail-top breathing-room clamp (attn-2aj) intentionally pins cards + // whose anchors sit near the viewport top. + const anchor = { + v: 2, fileId: 'file-x', snapshotId: 'snap-x', baseHash: 'h-x', + position: { byteRange: [600, 640], lineRange: [20, 20] }, + }; + s.applyEvent({ + meta: { v: 2, eventId: 'e-3', roomId: 'room-x', authorId: 'p-reviewer', deviceId: 'd-x', createdAt: Date.now(), parentEventIds: [], snapshotId: 'snap-x' }, + body: { type: 'comment_created', threadId: 't-2', anchor, body: 'An open question.' }, + auth: { signature: 'sig', signingKeyId: 'kid' }, + }); + return 'ok'; +})() +EOF +) +result=$("$ATTN" --eval "$mixed_js") +assert_eq "Seeded an additional unresolved thread" "$result" '"ok"' + +result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"open\\\"]') !== null" 'true') +assert_eq "Active card rendered alongside the chip" "$result" "true" + +result=$("$ATTN" --eval "(() => { const c = document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"open\\\"]'); if (!c) return 'no-card'; const cs = getComputedStyle(c); return [c.querySelector('.rmc-avatar') !== null, cs.borderLeftColor.length > 0].join(','); })()") +assert_eq "Active card carries author avatar + colored border" "$result" '"true,true"' + +screenshot "06-mixed-expanded-rail" + +echo "" +echo "--- Rail-header toggle → collapsed gutter with avatar chips ---" +result=$("$ATTN" --query '[data-slot="rail-toggle"]' | jq -r '.status' 2>/dev/null || echo "not_found") +assert_eq "Rail toggle present in the rail header" "$result" "found" + +"$ATTN" --click '[data-slot="rail-toggle"]' >/dev/null 2>&1 || true + +result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"collapsed"') +assert_eq "Toggle collapses the rail" "$result" '"collapsed"' + +result=$("$ATTN" --query '[data-testid="review-margin-avatar-chip"]' | jq -r '.count' 2>/dev/null || echo "0") +assert_eq "Unresolved thread shows an author avatar chip in the gutter" "$result" "1" + +result=$("$ATTN" --query '[data-testid="review-margin-resolved-chip"][data-variant="icon"]' | jq -r '.count' 2>/dev/null || echo "0") +assert_eq "Resolved thread shows a ✓ chip in the gutter" "$result" "1" + +screenshot "07-collapsed-gutter-chips" + +echo "" +echo "--- Avatar chip click → rail expands onto the thread ---" +"$ATTN" --click '[data-testid="review-margin-avatar-chip"]' >/dev/null 2>&1 || true + +result=$(poll_eval "document.querySelector('[data-slot=\\\"right-rail\\\"]')?.getAttribute('data-mode') ?? 'missing'" '"expanded"') +assert_eq "Avatar chip expands the rail" "$result" '"expanded"' + +result=$(poll_eval "document.querySelector('[data-testid=\\\"review-margin-card\\\"][data-state=\\\"open\\\"]') !== null" 'true') +assert_eq "Thread card visible after avatar expand" "$result" "true" + +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. +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 before = card.getBoundingClientRect().top; + vp.scrollTop = 150; + const scrolled = vp.scrollTop; + window.__attn_scroll_probe__ = { before, scrolled }; + return scrolled > 0 ? 'scrolled' : 'no-scroll'; +})() +EOF +) +result=$("$ATTN" --eval "$scroll_js") +assert_eq "Editor viewport scrolled" "$result" '"scrolled"' + +verify_js=$(cat <<'EOF' +(() => { + const probe = window.__attn_scroll_probe__; + const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]'); + if (!probe || !card) return 'missing'; + const after = card.getBoundingClientRect().top; + const drift = Math.abs((probe.before - after) - probe.scrolled); + return drift <= 4 ? 'tracked' : `drifted by ${Math.round(drift)}px (before ${Math.round(probe.before)}, after ${Math.round(after)}, scrolled ${probe.scrolled})`; +})() +EOF +) +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 +restore_js=$(cat <<'EOF' +(() => { + const probe = window.__attn_scroll_probe__; + const card = document.querySelector('[data-testid="review-margin-card"][data-state="open"]'); + if (!probe || !card) return 'missing'; + const drift = Math.abs(card.getBoundingClientRect().top - probe.before); + return drift <= 4 ? 'restored' : `off by ${Math.round(drift)}px`; +})() +EOF +) +result=$(poll_eval "$restore_js" '"restored"') +assert_eq "Card returns to its anchor when scrolled back" "$result" '"restored"' + +screenshot "09-scroll-tracking" + # =================================================================== # Summary # =================================================================== diff --git a/src/review/bootstrap.rs b/src/review/bootstrap.rs index 178869c..8a531bb 100644 --- a/src/review/bootstrap.rs +++ b/src/review/bootstrap.rs @@ -47,7 +47,7 @@ use tokio::sync::RwLock; use crate::daemon::runtime_dir; use crate::review::crypto::ids::{derive_envelope_id_for_event, derive_event_id}; -use crate::review::crypto::kdf::{derive_room_id, derive_room_keys}; +use crate::review::crypto::kdf::{RoomKeys, derive_room_id, derive_room_keys}; use crate::review::crypto::pow::TokenPool; use crate::review::crypto::signing::{DeviceSigningKey, DeviceVerifyingKey, SignError, sign_event}; use crate::review::envelope::{AssembleInput, assemble_event_envelope}; @@ -777,6 +777,67 @@ impl Bootstrapper { /// records a `LocalFileBinding` whose room hasn't expired, we look up the /// room secret stashed on disk and re-emit the same invite without any /// network calls. + /// Sign + enqueue the owner's self `ParticipantJoined` announce + /// (attn-42y). Joiners emit one in `join_with_identity`, but the owner + /// historically never did — so owner-authored comments degraded to a + /// raw participant id on every window. Called on fresh mint AND on + /// re-Share: re-announcing is harmless (frontends key names by + /// participantId, last write wins), backfills rooms shared before this + /// landed, and refreshes a display name changed since the first share. + fn announce_owner( + &self, + room_id: &RoomId, + room_keys: &RoomKeys, + policy: &RoomPolicy, + identity: &DeviceIdentity, + now_ms: u64, + ) -> Result<(), BootstrapError> { + let participant_id = identity.typed_participant_id(); + let device_id = identity.typed_device_id(); + let owner_participant = Participant { + participant_id: participant_id.clone(), + display_name: identity.effective_display_name(), + kind: ParticipantKind::Owner, + public_signing_key: identity.public_signing_key.clone(), + capabilities: agent_capabilities(ParticipantKind::Owner), + }; + let owner_device = Device { + device_id: device_id.clone(), + participant_id: participant_id.clone(), + public_encryption_key: identity.public_encryption_key.clone(), + public_signing_key: identity.public_signing_key.clone(), + client: DeviceClient::AttnNative, + created_at: now_ms, + }; + let joined_body = ReviewEventBody::ParticipantJoined { + participant: owner_participant, + device: owner_device, + }; + let joined_envelope = assemble_event_envelope(AssembleInput { + event_key: *room_keys.event_key.as_bytes(), + signing_key: identity.signing_key()?, + room_id: room_id.clone(), + author_id: participant_id, + device_id, + created_at_ms: now_ms, + expires_at_ms: policy.expires_at, + parent_event_ids: vec![], + snapshot_id: None, + body: joined_body, + kind: EnvelopeKind::Event, + client_nonce: None, + }) + .map_err(|e| { + BootstrapError::Crypto(format!("assemble owner ParticipantJoined envelope: {e}")) + })?; + self.store + .append_outbox(room_id, &joined_envelope) + .map_err(|e| { + BootstrapError::Store(format!("append owner ParticipantJoined outbox: {e}")) + })?; + Ok(()) + } + pub async fn share( &self, path: PathBuf, @@ -826,6 +887,10 @@ impl Bootstrapper { Err(_) => false, }; if reestablished { + // Re-announce the owner on every re-Share: backfills rooms + // shared before the owner self-announce existed and picks up + // a display name changed since the original share. + self.announce_owner(&existing.room_id, &keys, &policy, &identity, now_ms)?; return Ok(existing); } tracing::warn!( @@ -903,6 +968,9 @@ impl Bootstrapper { .append_outbox(&room_id, &envelope) .map_err(|e| BootstrapError::Store(format!("append RoomCreated outbox: {e}")))?; + // 5b. Announce the owner identity (display name) into the room log. + self.announce_owner(&room_id, &room_keys, &policy, &identity, now_ms)?; + // 6. Persist the room state on disk. We snapshot a `ReviewRoom` with // no documents/snapshots yet — that wiring lands when SnapshotCreated // flows through ReviewManager. The path is recorded in `bindings.json` @@ -2731,36 +2799,43 @@ mod tests { assert_eq!(loaded_room.event_heads.len(), 1); assert_eq!(loaded_room.policy.mode, RoomMode::Async); - // Outbox holds the RoomCreated event, the snapshot blob, and the - // SnapshotCreated event — with the blob enqueued BEFORE the event - // that references it, so peers receive bytes-then-pointer in relay - // serverSeq order. + // Outbox holds: the RoomCreated event, the owner's self + // ParticipantJoined announce (attn-42y — carries the display name + // so owner-authored comments resolve to a real name on every + // window), the snapshot blob, and the SnapshotCreated event — with + // the blob enqueued BEFORE the event that references it, so peers + // receive bytes-then-pointer in relay serverSeq order. let envelopes: Vec = store .iter_outbox(&outcome.room_id) .expect("iter") .collect::>() .expect("decode"); - assert_eq!(envelopes.len(), 3); + assert_eq!(envelopes.len(), 4); assert_eq!(envelopes[0].kind, EnvelopeKind::Event, "RoomCreated"); assert_eq!( envelopes[1].kind, + EnvelopeKind::Event, + "owner ParticipantJoined announce" + ); + assert_eq!( + envelopes[2].kind, EnvelopeKind::SnapshotBlob, "snapshot bytes ride the snapshot_blob lane" ); - assert_eq!(envelopes[2].kind, EnvelopeKind::Event, "SnapshotCreated"); + assert_eq!(envelopes[3].kind, EnvelopeKind::Event, "SnapshotCreated"); // The blob envelope opens under the room's snapshotKey and carries // the canonical SnapshotPlaintext bytes (markdown + anchor index). let parsed = parse_invite(&outcome.invite).expect("parse invite"); let keys = derive_room_keys(&parsed.room_secret); - let aad = crate::review::envelope::envelope_aad(&envelopes[1]); + let aad = crate::review::envelope::envelope_aad(&envelopes[2]); let nonce_bytes = URL_SAFE_NO_PAD - .decode(envelopes[1].nonce.as_bytes()) + .decode(envelopes[2].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(envelopes[1].ciphertext.as_bytes()) + .decode(envelopes[2].ciphertext.as_bytes()) .expect("ciphertext decodes"); let blob_bytes = crate::review::crypto::aead::open( keys.snapshot_key.as_bytes(), @@ -2775,7 +2850,7 @@ mod tests { // Locally: blob persisted by envelopeId, SnapshotNode references it. let stored_blob = store - .load_snapshot_blob(&outcome.room_id, &envelopes[1].envelope_id) + .load_snapshot_blob(&outcome.room_id, &envelopes[2].envelope_id) .expect("load blob") .expect("owner persisted its own blob"); assert_eq!(stored_blob, blob_bytes); @@ -2791,7 +2866,7 @@ mod tests { let node_ref = node .encrypted_blob_ref .expect("SnapshotNode carries the BlobRef"); - assert_eq!(node_ref.blob_id, envelopes[1].envelope_id); + assert_eq!(node_ref.blob_id, envelopes[2].envelope_id); assert_eq!( node_ref.storage, crate::review::model::BlobStorage::Mailbox, @@ -3427,25 +3502,27 @@ mod tests { .expect("iter") .collect::>() .expect("decode"); - assert_eq!(envelopes.len(), 3); - assert_eq!(envelopes[1].kind, EnvelopeKind::SnapshotBlob); + // RoomCreated, owner ParticipantJoined announce (attn-42y), blob + // wrapper, SnapshotCreated. + assert_eq!(envelopes.len(), 4); + assert_eq!(envelopes[2].kind, EnvelopeKind::SnapshotBlob); assert!( - envelopes[1].ciphertext_bytes < 1024, + envelopes[2].ciphertext_bytes < 1024, "outbox envelope must be the small BlobRef wrapper, got {} bytes", - envelopes[1].ciphertext_bytes + envelopes[2].ciphertext_bytes ); // The wrapper opens under snapshotKey to a BlobRef pointing at R2. let parsed = parse_invite(&outcome.invite).expect("parse invite"); let keys = derive_room_keys(&parsed.room_secret); - let aad = crate::review::envelope::envelope_aad(&envelopes[1]); + let aad = crate::review::envelope::envelope_aad(&envelopes[2]); let nonce_bytes = URL_SAFE_NO_PAD - .decode(envelopes[1].nonce.as_bytes()) + .decode(envelopes[2].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(envelopes[1].ciphertext.as_bytes()) + .decode(envelopes[2].ciphertext.as_bytes()) .expect("ciphertext decodes"); let ref_bytes = crate::review::crypto::aead::open( keys.snapshot_key.as_bytes(), @@ -3457,11 +3534,11 @@ mod tests { let blob_ref: crate::review::model::BlobRef = serde_json::from_slice(&ref_bytes).expect("wrapper plaintext is a BlobRef"); assert_eq!(blob_ref.storage, crate::review::model::BlobStorage::R2); - assert_eq!(blob_ref.blob_id, envelopes[1].envelope_id); + assert_eq!(blob_ref.blob_id, envelopes[2].envelope_id); // The owner still persists the decrypted bytes + node locally. let stored_blob = store - .load_snapshot_blob(&outcome.room_id, &envelopes[1].envelope_id) + .load_snapshot_blob(&outcome.room_id, &envelopes[2].envelope_id) .expect("load blob") .expect("blob persisted"); assert_eq!(stored_blob.len() as u64, blob_ref.byte_length); diff --git a/web/src/App.svelte b/web/src/App.svelte index 567ea68..d5dad67 100644 --- a/web/src/App.svelte +++ b/web/src/App.svelte @@ -64,6 +64,8 @@ import ShareDialog from './lib/ShareDialog.svelte'; import NamePrompt from './lib/NamePrompt.svelte'; import { userProfile } from './lib/review/profile.svelte'; + import PanelRightClose from '@lucide/svelte/icons/panel-right-close'; + import PanelRightOpen from '@lucide/svelte/icons/panel-right-open'; import Users from '@lucide/svelte/icons/users'; import CommentComposer from './lib/CommentComposer.svelte'; import SuggestionComposer from './lib/SuggestionComposer.svelte'; @@ -87,6 +89,7 @@ loadMarkdownFromPath, markdownSourceUrl, } from './lib/markdown-layer'; + import { RAIL_WIDTH_PX } from './lib/review/rail-mode'; import { reviewStore } from './lib/review/store.svelte'; import ReviewMargin from './lib/ReviewMargin.svelte'; import ReviewFileNav from './lib/ReviewFileNav.svelte'; @@ -484,15 +487,19 @@ } }); - // Auto-open the review rail the first time the current file has feedback (a - // comment or suggestion). Without this the rail stays collapsed and a + // Auto-expand the review rail the first time the current file has + // UNRESOLVED feedback (attn-42y: resolved-only history defaults to the + // collapsed gutter). Without this the rail stays collapsed and a // reviewer's notes are invisible until someone happens to press Cmd+J — so - // incoming review work silently disappears. Open only ONCE so a deliberate - // Cmd+J close stays closed. - let reviewRailAutoOpened = $state(false); + // incoming review work silently disappears. One-shot PER ROOM so a + // deliberate collapse (toggle / Cmd+J) stays collapsed, but a second room + // joined later in the session still opens by default on its feedback. + let reviewRailAutoOpenedRoom = $state(null); $effect(() => { - if (reviewStore.threadsForCurrentFile.length > 0 && !reviewRailAutoOpened) { - reviewRailAutoOpened = true; + const roomId = reviewStore.currentRoomId; + if (roomId === null) return; + if (reviewStore.marginActiveThreadCount > 0 && reviewRailAutoOpenedRoom !== roomId) { + reviewRailAutoOpenedRoom = roomId; if (!reviewStore.panelOpen) reviewStore.panelOpen = true; } }); @@ -828,6 +835,39 @@ suggestionComposer = null; } + /** + * Submit-only reset (attn-2aj): after a comment/suggestion is CREATED, + * collapse the editor selection so the floating Comment|Suggest toolbar + * doesn't resurrect over the just-annotated text the instant the + * composer unmounts. Collapsing the PM selection is the load-bearing + * part — clearing `toolbarSelection` alone gets re-derived from the + * still-live selection on the next scroll/selectionchange. Cancel paths + * deliberately do NOT call this, so Escape keeps the selection for a + * retry or a switch to the other composer. + */ + function collapseComposeSelection(): void { + toolbarSelection = null; + const view = pmViewForReview; + if (!view) return; + // A mid-compose file switch re-creates the editor (collabEpoch bump); + // only collapse the view the composer was opened against — the old + // selection died with the old view, and the new file's selection is + // not ours to touch (a stray dispatch would also broadcast a caret + // move to peers). + const composerView = commentComposer?.view ?? suggestionComposer?.view; + if (composerView !== undefined && view !== composerView) return; + try { + view.dispatch( + view.state.tr.setSelection( + TextSelection.create(view.state.doc, view.state.selection.to), + ), + ); + } catch { + // The view can be mid-teardown on a file/tab switch; the selection + // dies with it, which is exactly the outcome we wanted anyway. + } + } + // Floating selection toolbar (attn-bit): the discoverable surface for the // otherwise keyboard-only comment/suggest actions. We track the live editor // selection and expose {from,to} when — and only when — composing is @@ -2349,7 +2389,9 @@ avoidWindowControls={!hasSidebar} fixed={!hasSidebar} topOffsetPx={34} - rightInsetPx={showReviewChrome && !reviewStore.panelOpen ? 328 : 16} + rightInsetPx={showReviewChrome && reviewStore.railMode !== 'expanded' + ? 328 - (hasSidebar ? RAIL_WIDTH_PX[reviewStore.railMode] : 0) + : 16} onNavigate={(dir) => openPath(dir, inferFileTypeFromTree(dir))} onShare={showBreadcrumbShare ? openShareDialog : undefined} shareEnabled={showBreadcrumbShare} @@ -2360,25 +2402,6 @@
{/if} - {#if isReviewerViewingSnapshot} - -
-
- {/if} - {/snippet} +{#snippet sharedDocBanner()} + {#if isReviewerViewingSnapshot} + +
+
+ {/if} +{/snippet} + {#snippet rightRailPlaceholder()} + {@render sharedDocBanner()} +
+ {@render reviewChrome()} +
+ {@render mainContent()} +
+ + + +
{:else} @@ -2663,6 +2751,7 @@ anchorContext={commentComposer.anchorContext} roomId={commentComposer.roomId} onClose={closeCommentComposer} + onSubmitted={collapseComposeSelection} /> {/if} @@ -2674,6 +2763,7 @@ anchorContext={suggestionComposer.anchorContext} roomId={suggestionComposer.roomId} onClose={closeSuggestionComposer} + onSubmit={() => collapseComposeSelection()} /> {/if} openPath(path, detectFileType(path))} /> - + + diff --git a/web/src/app.css b/web/src/app.css index 9ab2ffb..d9aa70e 100644 --- a/web/src/app.css +++ b/web/src/app.css @@ -82,10 +82,13 @@ --review-card-comment-accent: oklch(0.62 0.13 82); --review-card-suggestion-accent: oklch(0.58 0.15 150); - /* Peer avatar accents — owner uses primary, reviewer cool, agent distinct violet. */ - --peer-avatar-bg-owner: oklch(0.62 0.14 32); - --peer-avatar-bg-reviewer: oklch(0.58 0.11 235); - --peer-avatar-bg-agent: oklch(0.60 0.13 295); + /* Peer avatar accents — owner uses primary, reviewer cool, agent distinct + violet. Lightness tuned so white monogram text clears WCAG AA 4.5:1 + (these tokens back small-text chips: PeerStrip, margin avatar chips, + card avatars — attn-42y). */ + --peer-avatar-bg-owner: oklch(0.58 0.14 32); + --peer-avatar-bg-reviewer: oklch(0.56 0.11 235); + --peer-avatar-bg-agent: oklch(0.57 0.13 295); /* Stale anchor — dimmed/desaturated foreground. */ --stale-anchor-fg: oklch(0.55 0.008 55 / 65%); @@ -167,10 +170,11 @@ --review-card-comment-accent: oklch(0.78 0.13 86); --review-card-suggestion-accent: oklch(0.74 0.16 150); - /* Peer avatar accents. */ - --peer-avatar-bg-owner: oklch(0.62 0.12 32); - --peer-avatar-bg-reviewer: oklch(0.62 0.10 230); - --peer-avatar-bg-agent: oklch(0.62 0.12 295); + /* Peer avatar accents. Lightness tuned for white text at WCAG AA 4.5:1 + (see the light-block note). */ + --peer-avatar-bg-owner: oklch(0.58 0.12 32); + --peer-avatar-bg-reviewer: oklch(0.56 0.10 230); + --peer-avatar-bg-agent: oklch(0.57 0.12 295); /* Stale anchor. */ --stale-anchor-fg: oklch(0.60 0.010 250 / 60%); @@ -938,35 +942,25 @@ border-radius: 8px; } -/* ============================================ - * Dialog opacity override (WKWebView animation fix) - * - * `tw-animate-css`'s `animate-in fade-in-0` keyframe leaves - * dialog contents at opacity:0 in this WKWebView build when the - * dialog's child component mounts $effects on the same tick the - * dialog opens. Forcing opacity to 1 once data-state=open removes - * the regression without changing behavior in other dialog - * surfaces — the slide / zoom transforms still run. - * - * Track: when the bits-ui dialog dependency upgrades, retest - * by removing this rule. - * ============================================ */ -[data-slot="share-dialog"][data-state="open"], -[data-slot="dialog-overlay"][data-state="open"] { - opacity: 1; -} - /* ============================================ * Dialog EXIT-animation override (WKWebView unmount fix) * - * Same root cause, close path: `tw-animate-css`'s `animate-out fade-out-0` - * keyframe runs but never fires `animationend` in this WKWebView build, so - * bits-ui (which waits for the exit animation before unmounting) leaves the - * dialog overlay + content MOUNTED FOREVER after close — a stuck full-screen - * `z-50 pointer-events:auto` overlay (and a focus-trap) that blocks the rest of + * `tw-animate-css`'s `animate-out fade-out-0` keyframe runs but never fires + * `animationend` in this WKWebView build, so bits-ui (which waits for the + * exit animation before unmounting) leaves the dialog overlay + content + * MOUNTED FOREVER after close — a stuck full-screen `z-50 + * pointer-events:auto` overlay (and a focus-trap) that blocks the rest of * the app: the comment composer can't be typed in, the owner can't reach the - * composer after sharing, etc. Removing the exit animation makes bits-ui see no - * animation (animationName: none) and unmount immediately on close. + * composer after sharing, etc. Removing the exit animation makes bits-ui see + * no animation (animationName: none) and unmount immediately on close. + * + * The plain Dialog surfaces (share dialog, name prompt, shortcuts) no longer + * use tw-animate keyframes at all — they fade in via a CSS transition + + * @starting-style in `components/ui/dialog/dialog-content.svelte` (a sibling + * entry-side WKWebView stall left dialogs stuck at opacity 0; the old + * `opacity: 1` override for it was removed with the keyframes). This rule + * still matters for the OTHER role=dialog surfaces that keep animate-out + * classes (sheet, command palette). * * Track: retest by removing this rule when the bits-ui dependency upgrades. * ============================================ */ diff --git a/web/src/lib/CommentComposer.svelte b/web/src/lib/CommentComposer.svelte index 91b2de1..4f40e82 100644 --- a/web/src/lib/CommentComposer.svelte +++ b/web/src/lib/CommentComposer.svelte @@ -9,6 +9,7 @@
+ {:else if t} + + {/if} + {/each} + {:else} {#if orphanThreads.length > 0}
activateThread(t)} onReject={() => dismissLocally(t.id)} @@ -624,7 +876,7 @@ class="review-margin-connectors" data-testid="review-margin-connectors" aria-hidden="true" - width="320" + width="100%" height="100%" > {#each connectorLines as line (line.id)} @@ -641,10 +893,11 @@ {/if} - + {#each visiblePlacements as p (p.id)} - {@const t = anchoredThreads.find((th) => th.id === p.id)} - {#if t} + {@const t = threadById.get(p.id)} + {#if t && !t.resolved}
activateThread(t)} onReject={() => dismissLocally(t.id)} onResolve={() => resolveThread(t.id)} - onReply={(body) => replyToThread(t, body)} + onReply={(body) => replyToThread(t, body)} pendingDismiss={locallyDismissed.has(t.id)} />
+ {:else if t && t.id === expandedResolvedId} + +
+ { void collapseResolved(t); }} + /> +
+ {:else if t} + + {/if} {/each} - - {#if resolvedExpanded || resolvedPlacements.length <= COLLAPSED_RESOLVED_THRESHOLD} - {#each resolvedPlacements as p (p.id)} - {@const t = resolvedThreads.find((th) => th.id === p.id)} - {#if t} - - {/if} - {/each} - {/if} - {#if showResolvedPill} {/if} @@ -740,33 +1017,37 @@
{/if} + {/if}