Fix dev:collab reviewer flow: room replay on resume, invite validation, harness repair#9
Merged
Merged
Conversation
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 <noreply@anthropic.com>
…n-ry9)
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 <noreply@anthropic.com>
…aemon `attn review join <garbage>` 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/<roomId>#key=<secret>) and exits non-zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…o active rooms
- 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 <noreply@anthropic.com>
… hairline - The live-collab caret chip kept showing the construction-time label (the git default) after a NamePrompt rename — the prompt fires AFTER collab starts. CollabController.setSelfLabel updates the label and re-broadcasts the last caret position so peers update immediately; an App effect feeds it userProfile.effectiveName. (Comment cards were already fixed by the ParticipantJoined re-announce; this is the caret pipeline, which is collab-presence, not review events.) - Wheel gestures over the comments rail now forward to the document viewport — the rail deliberately never scrolls itself (attn-23m), so the pointer crossing it used to dead-stop reading flow. - The rail gets its top border back at the header divider's hairline weight (border/40); the earlier complaint was the heavier /60 line stacking with the header border. Verified live: wheel event on the rail scrolls the viewport 1:1; border-top 1px. svelte-check 0 errors; 33/33 unit files; review E2E 41/41. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rail bottom - The rail's top-left corner is rounded (rounded-tl-lg; overflow-hidden clips the header/content to it). - New fitBottom pass in margin-layout: the down-pass never places a card above its anchor, so a card whose anchored text sat near the viewport bottom extended past the rail and was cut off. fitBottom walks placements bottom-up and lifts any card whose ANCHOR is on screen fully into view — above its anchor if needed, Google-Docs style — cascading the lift so cards never overlap, and flagging lifted cards `offset` so the connector line shows the displacement. Anchors below the fold keep tracking off-screen with their text (attn-23m). Applied to both the expanded cards and the collapsed gutter chips. - Four new margin-layout unit tests; the E2E scroll-tracking probe now centers the card before sampling (strict 1:1 only holds outside the fit bands) with a settle pause (the margin repositions asynchronously on scroll), plus a new assertion that a bottom-anchored card is lifted fully into view. 43 PASS / 0 FAIL. Verified live: border-radius ~10px; a card anchored at the document tail sits fully visible at max scroll (bottom 660 vs clip line 712). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… corners The author/kind/state accent was a 3px border-left against the card's 6px corner radius, which renders pointy color overshoot past the rounded corners (user-reported offset). The accent is now an inset box-shadow strip driven by a --rmc-accent custom property (inline author color; kind/state CSS fallbacks) — inset shadows clip to the rounded padding box, so the strip's ends follow the curve. Uniform 1px border all around; padding-left 13px keeps content at the same x. Verified live: border-left 1px, inset strip present, padding 13px, corners clean. svelte-check 0 errors, 33/33 unit files, E2E 43/43. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root-caused and fixed the "reviewer never enters edit mode" reports around
task dev:collab. Three independent bugs, none introduced by the recent UI branch (verified by empirical bisect — fresh-state joins pass at every commit including origin/main):1. Persisted-session replay gap (attn-6dd) — the real long-standing bug. On any session after the first over persisted state, the reviewer daemon resumes its mailbox at the stored cursor, the relay never re-delivers the snapshot, and nothing replayed the on-disk room state to the webview — the reviewer hung on "Waiting for the shared document…" forever. Fixes:
ReviewManager::replay_room_to_webview: replays persisted events (snapshots rehydrated from the blob store) through the sameEventImportedlane the live pipeline uses; called on resume and on join.resume_known_roomsemits role-accurate lifecycles (Livefor locally-shared rooms,Joinedotherwise) instead of the passiveResumed, so the reviewer's window actually re-activates the room.room.jsonwith an empty record.2. Garbage invites accepted silently — the Share dialog's primary copyable is the
npx attnmd review join '…'one-liner; pasting it into dev-collab's invite prompt sent the whole string to the daemon, which failed silently while the CLI printed "join request sent" and the script claimed success. Fixes:review joinvalidates the invite client-side with the daemon's ownparse_invite; garbage exits 1 with the expectedattn://review/…shape.dev-collab.shextracts the invite URL from whatever is pasted (bare, quoted, or npx-wrapped).3. Editorial E2E silently broken since May 28 (attn-ry9) — the suite's raw
postMessagecalls lack the IPC capability token added inba19671, so every privileged call was rejected and 11 assertions cascade-failed on every commit since (which is why none of this was caught). Fixes: daemon-routed CLI join + a debug-build-only token bridge (window.__attn_ipc_token__, gated oncfg!(debug_assertions)via the init payload — release builds never expose it) for the remaining sites.Verification
scripts/test-editorial-e2e.sh: 18 / 18 (was 7 / 11-failed)🤖 Generated with Claude Code