Fix Share-for-review so the owner gets a working invite URL

scripts/dev-collab.sh

@@ -36,14 +36,22 @@ cd "$PROJECT_DIR"
 
 : "${ATTN_RELAY_URL:=http://localhost:8787}"
 : "${FIXTURE_PATH:=tests/fixtures/basic.md}"
+# Reviewer opens a visually distinct fixture so the two identical windows can be
+# told apart: share from the owner window (this fixture); the reviewer window
+# switches to it on join.
+: "${REVIEWER_FIXTURE_PATH:=tests/fixtures/typography.md}"
 : "${ATTN_BIN:=$PROJECT_DIR/target/debug/attn}"
 
 # Pin the per-instance ATTN_HOMEs the task issue asks for, then forward
 # them to scripts/lib/dual-instance.sh via its override knobs.
 export ATTN_DUAL_OWNER="/tmp/attn-collab-owner"
 export ATTN_DUAL_REVIEWER="/tmp/attn-collab-reviewer"
 export ATTN_DUAL_FIXTURE="$PROJECT_DIR/$FIXTURE_PATH"
+export ATTN_DUAL_REVIEWER_FIXTURE="$PROJECT_DIR/$REVIEWER_FIXTURE_PATH"
 export ATTN_BIN
+# Export the relay URL so the daemons booted by start_dual inherit it; without
+# this they fall back to the scaffold stub and Share never reaches the relay.
+export ATTN_RELAY_URL
 
 # Relay process state.
 RELAY_PID=""
@@ -141,7 +149,8 @@ join_reviewer() {
         return 0
     fi
 
-    log "Click [Share] in the owner window, copy the invite URL, then paste it here."
+    log "Click [Share] in the OWNER window — the one showing '$FIXTURE_PATH'."
+    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
@@ -189,11 +198,18 @@ trap cleanup EXIT INT TERM
 
 start_relay
 
+# Warn if the two windows would be indistinguishable (e.g. FIXTURE_PATH was
+# overridden to the reviewer's default) — the whole point is to tell them apart
+# for the Share step.
+if [ "$REVIEWER_FIXTURE_PATH" = "$FIXTURE_PATH" ]; then
+    log "WARNING: owner and reviewer fixtures are identical ($FIXTURE_PATH) — the two windows will look the same. Set REVIEWER_FIXTURE_PATH to a different file to tell them apart."
+fi
+
 log "Booting owner + reviewer daemons (ATTN_HOME isolation)"
 start_dual
 wait_for_dual 'h1' 20000
-log "Owner window opened (fixture: $FIXTURE_PATH, ATTN_HOME=$ATTN_DUAL_OWNER)"
-log "Reviewer window opened (ATTN_HOME=$ATTN_DUAL_REVIEWER) — idle until join"
+log "Owner window opened — shows '$FIXTURE_PATH'; SHARE FROM THIS WINDOW. ATTN_HOME=$ATTN_DUAL_OWNER"
+log "Reviewer window opened — shows '$REVIEWER_FIXTURE_PATH' until it joins, then switches to the owner's doc. ATTN_HOME=$ATTN_DUAL_REVIEWER"
 
 join_reviewer
 

scripts/lib/dual-instance.sh

@@ -44,6 +44,10 @@ __attn_dual_lib_dir() {
 : "${ATTN_DUAL_REPO:=$(__attn_dual_lib_dir)}"
 : "${ATTN_BIN:=$ATTN_DUAL_REPO/target/debug/attn}"
 : "${ATTN_DUAL_FIXTURE:=$ATTN_DUAL_REPO/tests/fixtures/review/scenario-comment-survives-edit.md}"
+# The reviewer can open a DIFFERENT fixture than the owner so the two windows
+# are visually distinguishable until the reviewer joins (then it switches to the
+# owner's shared doc). Resolved at start_dual time (NOT here) so a caller that
+# sets ATTN_DUAL_FIXTURE *after* sourcing this lib still gets it as the default.
 
 # Stashed PIDs. Empty until start_dual sets them; cleared by stop_dual.
 ATTN_DUAL_OWNER_PID=""
@@ -116,20 +120,31 @@ attn_reviewer() {
 start_dual() {
     __attn_dual_require_bin || return 1
 
+    # Resolve the reviewer fixture at call time so a caller that sets
+    # ATTN_DUAL_FIXTURE after sourcing this lib still gets it as the default.
+    local reviewer_fixture="${ATTN_DUAL_REVIEWER_FIXTURE:-$ATTN_DUAL_FIXTURE}"
+
     if [ ! -f "$ATTN_DUAL_FIXTURE" ]; then
         echo "dual-instance: fixture missing at $ATTN_DUAL_FIXTURE" >&2
         return 1
     fi
+    if [ ! -f "$reviewer_fixture" ]; then
+        echo "dual-instance: reviewer fixture missing at $reviewer_fixture" >&2
+        return 1
+    fi
 
     # Wipe any leftover state from a previous (possibly crashed) run.
     rm -rf "$ATTN_DUAL_OWNER" "$ATTN_DUAL_REVIEWER"
     mkdir -p "$ATTN_DUAL_OWNER" "$ATTN_DUAL_REVIEWER"
 
-    ATTN_HOME="$ATTN_DUAL_OWNER" "$ATTN_BIN" --no-fork "$ATTN_DUAL_FIXTURE" \
+    # Forward ATTN_RELAY_URL (if set) so the daemons attach the real relay
+    # instead of the scaffold stub — Share/Join/collab flows need it. Empty is
+    # treated as "unset" by the daemon, so this is safe when no relay is wanted.
+    ATTN_HOME="$ATTN_DUAL_OWNER" ATTN_RELAY_URL="${ATTN_RELAY_URL:-}" "$ATTN_BIN" --no-fork "$ATTN_DUAL_FIXTURE" \
         >"$ATTN_DUAL_OWNER/daemon.stdout.log" 2>"$ATTN_DUAL_OWNER/daemon.stderr.log" &
     ATTN_DUAL_OWNER_PID=$!
 
-    ATTN_HOME="$ATTN_DUAL_REVIEWER" "$ATTN_BIN" --no-fork "$ATTN_DUAL_FIXTURE" \
+    ATTN_HOME="$ATTN_DUAL_REVIEWER" ATTN_RELAY_URL="${ATTN_RELAY_URL:-}" "$ATTN_BIN" --no-fork "$reviewer_fixture" \
         >"$ATTN_DUAL_REVIEWER/daemon.stdout.log" 2>"$ATTN_DUAL_REVIEWER/daemon.stderr.log" &
     ATTN_DUAL_REVIEWER_PID=$!
 }

src/review/bootstrap.rs

@@ -659,6 +659,11 @@ impl BootstrapConfig {
 pub struct ShareOutcome {
     pub room_id: RoomId,
     pub invite: String,
+    /// Absolute path (file or folder) the owner shared, exactly as the frontend
+    /// passed it to `reviewShare` (`path.to_string_lossy()`). The Share dialog
+    /// matches this against the active target to recognise its own room (see
+    /// `shareTargetMatches`), so it must be the verbatim path string.
+    pub owner_display_path: String,
     /// `true` if the room was newly created, `false` if `Share` was a no-op
     /// because the bindings file already had an entry pointing at a live room.
     pub newly_created: bool,
@@ -953,6 +958,7 @@ impl Bootstrapper {
         Ok(ShareOutcome {
             room_id,
             invite,
+            owner_display_path: path.to_string_lossy().to_string(),
             newly_created: true,
             owner_signing_key: identity.public_signing_key.clone(),
             mode: room_mode_wire(policy.mode),
@@ -999,6 +1005,7 @@ impl Bootstrapper {
             return Ok(Some(ShareOutcome {
                 room_id,
                 invite,
+                owner_display_path: path.to_string_lossy().to_string(),
                 newly_created: false,
                 owner_signing_key: identity.public_signing_key.clone(),
                 mode: room_mode_wire(policy_mode),
@@ -2612,11 +2619,16 @@ mod tests {
             .await;
 
         let (_doc_tmp, path) = temp_markdown_file("x.md", "# X\n");
+        let expected_display_path = path.to_string_lossy().to_string();
         let outcome = boot
             .share(path, RoomMode::Async, None)
             .await
             .expect("share");
         assert!(outcome.invite.starts_with("attn://review/"));
+        assert_eq!(
+            outcome.owner_display_path, expected_display_path,
+            "ShareOutcome must carry the exact path the owner shared so the dialog recognises its own room",
+        );
         assert!(outcome.invite.contains("#key="));
         // The invite must round-trip through `parse_invite`.
         let parsed = parse_invite(&outcome.invite).expect("parse");

src/review/manager.rs

@@ -155,6 +155,10 @@ pub enum ReviewUpdate {
     ShareReady {
         room_id: RoomId,
         invite_url: String,
+        /// Absolute path the owner shared, so the dialog can recognise its own
+        /// room without relying on a frontend-captured intent. Serialised as
+        /// `ownerDisplayPath`.
+        owner_display_path: String,
         owner_signing_key: String,
         mode: String,
         expires_at: u64,
@@ -1346,6 +1350,7 @@ impl ReviewManager {
                 (self.update_tx)(ReviewUpdate::ShareReady {
                     room_id: outcome.room_id,
                     invite_url: outcome.invite,
+                    owner_display_path: outcome.owner_display_path,
                     owner_signing_key: outcome.owner_signing_key,
                     mode: outcome.mode,
                     expires_at: outcome.expires_at,

web/src/App.svelte

@@ -96,6 +96,7 @@
     isReviewerView,
     collabRoleFor,
     collabSeedReady,
+    shareTargetMatches,
   } from './lib/review/room-ui';
   import {
     requestReviewDecorationsRebuild,
@@ -275,16 +276,16 @@
   // invite; when it's a NEW target (e.g. sharing a folder while a single file is
   // shared), the dialog mints a fresh room so the owner "switches over" to it.
   let shareTargetIsCurrent = $derived.by(() => {
-    const roomId = reviewStore.currentShare?.roomId;
-    if (!roomId) return false;
-    const target = (shareTargetPath ?? activePath ?? '').replace(/\/+$/, '');
-    if (target.length === 0) return false;
-    return reviewStore.snapshots.some((s) => {
-      if (s.roomId !== roomId) return false;
-      const p = s.ownerDisplayPath;
-      if (!p) return false;
-      return p === target || p.startsWith(`${target}/`);
-    });
+    const share = reviewStore.currentShare;
+    if (!share?.roomId) return false;
+    const target = shareTargetPath ?? activePath ?? '';
+    return shareTargetMatches(share.ownerDisplayPath, target);
+  });
+  // Reset the share target when the dialog closes so a stale path can't feed
+  // shareTargetIsCurrent on the next open; openShareDialogForPath always sets it
+  // fresh before reopening, so clearing here is safe.
+  $effect(() => {
+    if (!shareDialogOpen) shareTargetPath = null;
   });
   const loadedMtimeByPath = new Map<string, number>();
   // Reactive map of disk mtimes for HTML files, keyed by path. Bumping an
@@ -1966,6 +1967,7 @@
         reviewStore.applyShareReady({
           roomId: payload.roomId,
           inviteUrl: payload.inviteUrl,
+          ownerDisplayPath: payload.ownerDisplayPath,
           ownerSigningKey: payload.ownerSigningKey,
           mode: payload.mode,
           expiresAt: payload.expiresAt,

web/src/lib/ShareDialog.svelte

@@ -141,9 +141,13 @@
     void autoMint();
   });
 
-  // Transition minting → ready when the daemon's ShareReady IPC lands.
+  // Transition → ready when the daemon's ShareReady IPC lands. Recovers from a
+  // timed-out mint too: a slow relay can answer AFTER the 15s timeout already
+  // flipped phase to 'error', and the invite is still valid — so surface it
+  // rather than leaving a spurious error on screen. A genuine daemon failure
+  // never populates inviteUrl, so this can't paper over a real error.
   $effect(() => {
-    if (phase === 'minting' && inviteUrl.length > 0) {
+    if ((phase === 'minting' || phase === 'error') && inviteUrl.length > 0) {
       if (mintTimeout) {
         clearTimeout(mintTimeout);
         mintTimeout = null;

web/src/lib/review/room-ui.test.ts

@@ -9,6 +9,7 @@ import {
   collabSeedReady,
   isReviewerView,
   roomDisplayName,
+  shareTargetMatches,
   shouldActivateRoomStatus,
   shouldAutoSelectOnlyRoom,
   shouldForgetRoomStatus,
@@ -228,6 +229,66 @@ defineCase('roomDisplayName: null when no snapshots for the room yet', () => {
   assert(roomDisplayName(snaps, 'r1' as RoomId) === null, 'no snapshots → null so the caller falls back to the short id');
 });
 
+// --- shareTargetMatches (owner-share path gate; replaces snapshot scan) -----
+
+defineCase('shareTargetMatches: single shared file matches itself', () => {
+  assert(
+    shareTargetMatches('/Users/me/plan.md', '/Users/me/plan.md') === true,
+    'the exact shared file must be recognized as current',
+  );
+});
+
+defineCase('shareTargetMatches: shared folder matches the folder itself', () => {
+  assert(
+    shareTargetMatches('/Users/me/planning', '/Users/me/planning') === true,
+    'the shared folder path must match itself',
+  );
+});
+
+defineCase('shareTargetMatches: child file under a shared folder matches', () => {
+  assert(
+    shareTargetMatches('/Users/me/planning', '/Users/me/planning/a.md') === true,
+    'a markdown file beneath the shared folder must be recognized as current',
+  );
+  assert(
+    shareTargetMatches('/Users/me/planning', '/Users/me/planning/sub/c.md') === true,
+    'a nested child of the shared folder must also match',
+  );
+});
+
+defineCase('shareTargetMatches: trailing slashes are tolerated on both sides', () => {
+  assert(
+    shareTargetMatches('/Users/me/planning/', '/Users/me/planning/a.md') === true,
+    'a folder share stored with a trailing slash must still match its children',
+  );
+  assert(
+    shareTargetMatches('/Users/me/plan.md', '/Users/me/plan.md/') === true,
+    'a stray trailing slash on the target must not break an exact match',
+  );
+});
+
+defineCase('shareTargetMatches: a different unshared file does NOT match', () => {
+  assert(
+    shareTargetMatches('/Users/me/plan.md', '/Users/me/other.md') === false,
+    'an unrelated file must not be treated as the current share',
+  );
+  assert(
+    shareTargetMatches('/Users/me/planning', '/Users/me/planningX/a.md') === false,
+    'a sibling whose name merely shares a prefix must not match (segment boundary required)',
+  );
+  assert(
+    shareTargetMatches('/Users/me/planning', '/Users/me/planning-notes.md') === false,
+    'a non-boundary prefix collision must not match',
+  );
+});
+
+defineCase('shareTargetMatches: empty / null inputs never match', () => {
+  assert(shareTargetMatches(null, '/Users/me/plan.md') === false, 'no active share → no match');
+  assert(shareTargetMatches('/Users/me/plan.md', null) === false, 'no target selected → no match');
+  assert(shareTargetMatches('', '') === false, 'both empty → no match');
+  assert(shareTargetMatches(undefined, undefined) === false, 'undefined inputs are safe → no match');
+});
+
 let failed = 0;
 for (const run of cases) {
   const result = run();

web/src/lib/review/room-ui.ts

@@ -122,3 +122,27 @@ export function roomDisplayName(
   const prefix = dir ? `${baseName(dir)}/ ` : '';
   return `${prefix}(${paths.length} files)`;
 }
+
+/**
+ * Does `target` fall under the path a room was shared for?
+ *
+ * The owner shares either a single file or a whole folder. A folder share is
+ * recognized for the folder itself AND for any markdown file beneath it, so
+ * opening the Share dialog on a child of a shared folder re-shows the existing
+ * invite instead of minting a fresh room. A different, unshared file (including
+ * a sibling whose name merely shares a prefix) does NOT match — the prefix test
+ * is path-segment aware (`P + '/'`).
+ *
+ * Both sides are trailing-slash-normalized so a folder share recorded as
+ * '/p/dir' or '/p/dir/' both match the child '/p/dir/child.md'. Empty/nullish
+ * inputs never match (no active share / no target selected).
+ */
+export function shareTargetMatches(
+  sharePath: string | null | undefined,
+  target: string | null | undefined,
+): boolean {
+  const p = (sharePath ?? '').replace(/\/+$/, '');
+  const t = (target ?? '').replace(/\/+$/, '');
+  if (p.length === 0 || t.length === 0) return false;
+  return t === p || t.startsWith(`${p}/`);
+}

web/src/lib/review/store.svelte.ts

@@ -81,6 +81,7 @@ export interface ReviewRoomSummary {
     roomId: RoomId;
     inviteUrl: string;
     ownerSigningKey: string;
+    ownerDisplayPath: string;
     mode: 'live' | 'async' | 'hybrid';
     expiresAt: number;
   };
@@ -163,6 +164,7 @@ export class ReviewStore {
     roomId: RoomId;
     inviteUrl: string;
     ownerSigningKey: string;
+    ownerDisplayPath: string;
     mode: 'live' | 'async' | 'hybrid';
     expiresAt: number;
   } | null>(null);
@@ -514,6 +516,7 @@ export class ReviewStore {
   applyShareReady(payload: {
     roomId: RoomId;
     inviteUrl: string;
+    ownerDisplayPath: string;
     ownerSigningKey: string;
     mode: 'live' | 'async' | 'hybrid';
     expiresAt: number;
@@ -522,10 +525,14 @@ export class ReviewStore {
     const nextDismissed = new Set(this.dismissedRoomIds);
     nextDismissed.delete(payload.roomId);
     this.dismissedRoomIds = nextDismissed;
+    // `ownerDisplayPath` is the path the daemon actually shared, carried on the
+    // ShareReady payload — so the dialog's shareTargetMatches gate works for any
+    // share (GUI or `attn review share`) without a frontend-captured intent.
     const share = {
       roomId: payload.roomId,
       inviteUrl: payload.inviteUrl,
       ownerSigningKey: payload.ownerSigningKey,
+      ownerDisplayPath: payload.ownerDisplayPath,
       mode: payload.mode,
       expiresAt: payload.expiresAt,
     };

web/src/lib/types.ts

@@ -875,6 +875,9 @@ export interface ReviewShareReady {
   kind: 'share_ready';
   roomId: RoomId;
   inviteUrl: string;
+  /** Absolute path the owner shared (file or folder); the dialog matches it
+   *  against the active target to recognise its own room. */
+  ownerDisplayPath: string;
   /** Base64url Ed25519 public signing key; fingerprint is sha256(this). */
   ownerSigningKey: string;
   mode: 'live' | 'async' | 'hybrid';