From a66f461ee55b1c80fb99791ec46cc1e302a04015 Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Mon, 8 Jun 2026 22:03:15 -0500
Subject: [PATCH 1/5] Fix owner Share dialog timing out instead of showing the
 invite URL

Clicking Share on a markdown file succeeded on the backend (relay
round-trip + ShareReady with a real invite URL) but the dialog timed out
after 15s with "Couldn't reach the review relay" and never showed the URL.

Root cause: commit e1bf35d gated the invite URL on `shareTargetIsCurrent`,
which scanned `reviewStore.snapshots` for the shared path. That store is
populated for reviewers (who receive a snapshot_created event) but never
for the owner's own fresh share -- the daemon drops snapshot envelopes in
forward_transport_event and ShareReady carries no path. So the gate was
always false for the owner, blanking the URL until the 15s mint timeout.

Fix (frontend-only, narrow blast radius): thread the path the frontend
already knows onto the share record. ShareDialog.onStart records the
in-flight path via reviewStore.noteShareIntent() when reviewShare fires;
applyShareReady stamps it as currentShare.ownerDisplayPath (persisted on
the room.share record so reselect/rehydration keeps it); shareTargetIsCurrent
now matches against that path via a new pure shareTargetMatches() in
room-ui.ts. A failed/timed-out mint releases the intent (clearShareIntent /
onAbort + applyError) so a stale path can't bleed into a later share.

Preserves single-file, folder, child-of-folder, and unshared-file
semantics. No daemon change.

Tests: 6 new shareTargetMatches cases in room-ui.test.ts (tsx defineCase
harness; the project has no vitest).

Deferred follow-up: emit the owner's own snapshot to the frontend at share
time so reviewStore.snapshots is populated for the owner -- the
architecturally complete daemon fix that would also repair the file-tree
"shared" marker and owner roomDisplayName (both already broken for the
owner before this gate, out of scope here).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 web/src/App.svelte                 | 17 ++++-----
 web/src/lib/ShareDialog.svelte     | 13 +++++++
 web/src/lib/review/room-ui.test.ts | 61 ++++++++++++++++++++++++++++++
 web/src/lib/review/room-ui.ts      | 24 ++++++++++++
 web/src/lib/review/store.svelte.ts | 52 +++++++++++++++++++++++++
 5 files changed, 157 insertions(+), 10 deletions(-)

diff --git a/web/src/App.svelte b/web/src/App.svelte
index 3b18138..db8789b 100644
--- a/web/src/App.svelte
+++ b/web/src/App.svelte
@@ -96,6 +96,7 @@
     isReviewerView,
     collabRoleFor,
     collabSeedReady,
+    shareTargetMatches,
   } from './lib/review/room-ui';
   import {
     requestReviewDecorationsRebuild,
@@ -275,16 +276,10 @@
   // 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);
   });
   const loadedMtimeByPath = new Map<string, number>();
   // Reactive map of disk mtimes for HTML files, keyed by path. Bumping an
@@ -2625,6 +2620,8 @@
   ownerSigningKey={shareTargetIsCurrent ? (reviewStore.currentShare?.ownerSigningKey ?? '') : ''}
   existingRoomId={shareTargetIsCurrent ? (reviewStore.currentShare?.roomId ?? null) : null}
   shareErrorMessage={reviewStore.lastError?.message ?? ''}
+  onStart={(params) => reviewStore.noteShareIntent(params.filePath)}
+  onAbort={() => reviewStore.clearShareIntent()}
   onClearError={() => reviewStore.clearLastError()}
 />
 <NamePrompt
diff --git a/web/src/lib/ShareDialog.svelte b/web/src/lib/ShareDialog.svelte
index f019390..2ea6393 100644
--- a/web/src/lib/ShareDialog.svelte
+++ b/web/src/lib/ShareDialog.svelte
@@ -55,6 +55,9 @@
     shareErrorMessage?: string;
     writeToClipboard?: (text: string) => Promise<void>;
     onStart?: (params: ShareStartParams) => void;
+    /** Fired when the mint fails or times out, so the owner can release any
+     *  captured share-intent (see `reviewStore.clearShareIntent`). */
+    onAbort?: () => void;
     onClearError?: () => void;
   }
 
@@ -75,6 +78,7 @@
     shareErrorMessage = '',
     writeToClipboard,
     onStart,
+    onAbort,
     onClearError,
   }: Props = $props();
 
@@ -173,6 +177,15 @@
     phase = 'error';
   });
 
+  // Release the captured share-intent whenever a mint ends in error (relay
+  // error OR the 15s relay-silent timeout, which emits no daemon error). Keeps
+  // a stale path from bleeding into a later, unrelated share (e.g. a CLI
+  // `attn review share` of a different file). The ready path already clears it
+  // via `applyShareReady`.
+  $effect(() => {
+    if (phase === 'error') onAbort?.();
+  });
+
   // Recompute the fingerprint whenever the owner key changes.
   $effect(() => {
     let cancelled = false;
diff --git a/web/src/lib/review/room-ui.test.ts b/web/src/lib/review/room-ui.test.ts
index d12fdcd..6966259 100644
--- a/web/src/lib/review/room-ui.test.ts
+++ b/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();
diff --git a/web/src/lib/review/room-ui.ts b/web/src/lib/review/room-ui.ts
index e78a324..33b5c69 100644
--- a/web/src/lib/review/room-ui.ts
+++ b/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}/`);
+}
diff --git a/web/src/lib/review/store.svelte.ts b/web/src/lib/review/store.svelte.ts
index 1fd280c..2729283 100644
--- a/web/src/lib/review/store.svelte.ts
+++ b/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,10 +164,41 @@ export class ReviewStore {
     roomId: RoomId;
     inviteUrl: string;
     ownerSigningKey: string;
+    ownerDisplayPath: string;
     mode: 'live' | 'async' | 'hybrid';
     expiresAt: number;
   } | null>(null);
 
+  /**
+   * Path (file or folder) the owner just fired `reviewShare` for, captured the
+   * instant the IPC is sent and consumed by the async `applyShareReady`
+   * callback. `ReviewUpdate::ShareReady` (src/review/manager.rs) carries no
+   * path, and the owner never reliably receives their own snapshot back as a
+   * frontend event before the dialog times out, so this is the only reliable
+   * source for the shared path on the owner side. Cleared once `applyShareReady`
+   * consumes it.
+   */
+  pendingShareTargetPath = $state<string | null>(null);
+
+  /**
+   * Record the path the owner is about to share, just before the `reviewShare`
+   * IPC fires. `applyShareReady` reads this to stamp the resulting
+   * `currentShare`/room `share` record with its `ownerDisplayPath`.
+   */
+  noteShareIntent(path: string): void {
+    this.pendingShareTargetPath = path;
+  }
+
+  /**
+   * Release a captured but unconsumed share-intent. Called when a share mint
+   * fails or times out (the relay-silent timeout produces no `applyError`, so
+   * the dialog signals this directly via its `onAbort`) so the path can't be
+   * mis-stamped onto a later, unrelated `ShareReady`.
+   */
+  clearShareIntent(): void {
+    this.pendingShareTargetPath = null;
+  }
+
   /**
    * Append-only buffer of imported review events. The thread selectors
    * below reconstruct typed `Thread[]` from this.
@@ -418,6 +450,10 @@ export class ReviewStore {
       ...error,
       updatedAt: Date.now(),
     };
+    // A share that errors out must release any captured share-intent so a
+    // stale path can't be mis-stamped onto a later, unrelated ShareReady
+    // (e.g. a subsequent CLI `attn review share` of a different file).
+    this.pendingShareTargetPath = null;
   }
 
   clearLastError(): void {
@@ -522,10 +558,26 @@ export class ReviewStore {
     const nextDismissed = new Set(this.dismissedRoomIds);
     nextDismissed.delete(payload.roomId);
     this.dismissedRoomIds = nextDismissed;
+    // The path the owner just shared, captured at `reviewShare`-fire time via
+    // `noteShareIntent`. Fall back to the room's previously-recorded path
+    // (idempotent re-share / reconnect re-emits ShareReady without a fresh
+    // noteShareIntent). NOTE: a CLI-initiated `attn review share <path>`
+    // (src/cli_review.rs) reaches here without ever calling noteShareIntent and,
+    // on a first-ever share, has no prior room record either — so it falls
+    // through to '' and the dialog will re-mint rather than re-show that invite.
+    // Benign (re-share is idempotent on the Rust side) and not a regression; to
+    // fully close it, thread the path through ReviewUpdate::ShareReady on the
+    // Rust side and stamp it here instead of relying on pendingShareTargetPath.
+    const ownerDisplayPath =
+      this.pendingShareTargetPath ??
+      this.rooms[payload.roomId]?.share?.ownerDisplayPath ??
+      '';
+    this.pendingShareTargetPath = null;
     const share = {
       roomId: payload.roomId,
       inviteUrl: payload.inviteUrl,
       ownerSigningKey: payload.ownerSigningKey,
+      ownerDisplayPath,
       mode: payload.mode,
       expiresAt: payload.expiresAt,
     };

From 2330835f8693b6e9cd55fd87b4c1a477b337784a Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Tue, 9 Jun 2026 10:48:19 -0500
Subject: [PATCH 2/5] Forward ATTN_RELAY_URL to collab daemons in the
 dual-instance harness

`scripts/dev-collab.sh` set ATTN_RELAY_URL but never exported it, and
`start_dual` (scripts/lib/dual-instance.sh) launched the owner/reviewer
daemons with only ATTN_HOME prefixed. So the daemons started with no relay
configured and fell back to the scaffold stub -- Share never reached the
relay and the dialog timed out, making the collab flow impossible to test
locally even though the relay was healthy.

Forward ATTN_RELAY_URL (defaulting to empty, which the daemon treats as
"unset") on both daemon launches in start_dual, and export it from
dev-collab.sh. Empty stays safe for callers that don't want a relay; when
set, the daemons now attach the real relay.

Verified end to end: `task dev:collab` owner now logs `review bootstrap
attached (relay=http://localhost:8787)`, Share publishes a snapshot and the
dialog shows the invite, and the reviewer joins over ws://localhost:8787.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 scripts/dev-collab.sh        | 3 +++
 scripts/lib/dual-instance.sh | 7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/dev-collab.sh b/scripts/dev-collab.sh
index 5873f60..ea46723 100755
--- a/scripts/dev-collab.sh
+++ b/scripts/dev-collab.sh
@@ -44,6 +44,9 @@ 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_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=""
diff --git a/scripts/lib/dual-instance.sh b/scripts/lib/dual-instance.sh
index be780ad..d399cd9 100644
--- a/scripts/lib/dual-instance.sh
+++ b/scripts/lib/dual-instance.sh
@@ -125,11 +125,14 @@ start_dual() {
     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 "$ATTN_DUAL_FIXTURE" \
         >"$ATTN_DUAL_REVIEWER/daemon.stdout.log" 2>"$ATTN_DUAL_REVIEWER/daemon.stderr.log" &
     ATTN_DUAL_REVIEWER_PID=$!
 }

From 5431f4dc2a4eca84b9dbc2f0816cdf7ff8a240ff Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Tue, 9 Jun 2026 13:59:31 -0500
Subject: [PATCH 3/5] =?UTF-8?q?dev:collab=20=E2=80=94=20open=20reviewer=20?=
 =?UTF-8?q?on=20a=20distinct=20fixture=20so=20the=20windows=20are=20tellab?=
 =?UTF-8?q?le=20apart?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The owner and reviewer windows both opened the same fixture and were
pixel-identical, so it was easy to click Share on the wrong one. Since the
join always routes to the reviewer daemon, sharing from the reviewer window
loops the invite back to itself (joins its own room, "No peers") while the
owner window sits idle — looking like collab silently failed.

Open the reviewer on a different fixture (default typography.md) via a new
ATTN_DUAL_REVIEWER_FIXTURE knob (defaults to the owner's fixture for
backward compat). Now the owner window shows basic.md / "Project Status" and
the reviewer shows typography.md until it joins, at which point it switches
to the owner's doc — making both "which window do I share from" and "did the
join work" visually obvious. Terminal hints updated to say SHARE FROM the
owner window.

Verified: owner h1 "Project Status" vs reviewer h1 "Heading Level 1"; after
owner Share + reviewer join, reviewer h1 -> "Project Status" and both review
bars show the other peer (owner "…Live R", reviewer "Live O").

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 scripts/dev-collab.sh        | 12 +++++++++---
 scripts/lib/dual-instance.sh | 10 +++++++++-
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/scripts/dev-collab.sh b/scripts/dev-collab.sh
index ea46723..c49567e 100755
--- a/scripts/dev-collab.sh
+++ b/scripts/dev-collab.sh
@@ -36,6 +36,10 @@ 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
@@ -43,6 +47,7 @@ cd "$PROJECT_DIR"
 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.
@@ -144,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
@@ -195,8 +201,8 @@ start_relay
 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
 
diff --git a/scripts/lib/dual-instance.sh b/scripts/lib/dual-instance.sh
index d399cd9..e97f524 100644
--- a/scripts/lib/dual-instance.sh
+++ b/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). Defaults to the owner's fixture for backward compat.
+: "${ATTN_DUAL_REVIEWER_FIXTURE:=$ATTN_DUAL_FIXTURE}"
 
 # Stashed PIDs. Empty until start_dual sets them; cleared by stop_dual.
 ATTN_DUAL_OWNER_PID=""
@@ -120,6 +124,10 @@ start_dual() {
         echo "dual-instance: fixture missing at $ATTN_DUAL_FIXTURE" >&2
         return 1
     fi
+    if [ ! -f "$ATTN_DUAL_REVIEWER_FIXTURE" ]; then
+        echo "dual-instance: reviewer fixture missing at $ATTN_DUAL_REVIEWER_FIXTURE" >&2
+        return 1
+    fi
 
     # Wipe any leftover state from a previous (possibly crashed) run.
     rm -rf "$ATTN_DUAL_OWNER" "$ATTN_DUAL_REVIEWER"
@@ -132,7 +140,7 @@ start_dual() {
         >"$ATTN_DUAL_OWNER/daemon.stdout.log" 2>"$ATTN_DUAL_OWNER/daemon.stderr.log" &
     ATTN_DUAL_OWNER_PID=$!
 
-    ATTN_HOME="$ATTN_DUAL_REVIEWER" ATTN_RELAY_URL="${ATTN_RELAY_URL:-}" "$ATTN_BIN" --no-fork "$ATTN_DUAL_FIXTURE" \
+    ATTN_HOME="$ATTN_DUAL_REVIEWER" ATTN_RELAY_URL="${ATTN_RELAY_URL:-}" "$ATTN_BIN" --no-fork "$ATTN_DUAL_REVIEWER_FIXTURE" \
         >"$ATTN_DUAL_REVIEWER/daemon.stdout.log" 2>"$ATTN_DUAL_REVIEWER/daemon.stderr.log" &
     ATTN_DUAL_REVIEWER_PID=$!
 }

From b9ceac5387c365dc9bb11fda97cd82ad3c281e8b Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Tue, 9 Jun 2026 15:13:11 -0500
Subject: [PATCH 4/5] Address code-review findings on the share-intent +
 harness changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From the xhigh review of this branch:

#1 (ShareReady after the 15s timeout): a slow relay could answer after the
mint timeout already flipped the dialog to 'error' and the onAbort effect had
cleared the share-intent, so a *successful* share showed a spurious error and
"Try again" minted a duplicate room. Fixed two ways: the ready transition now
also recovers from the 'error' state when a valid inviteUrl arrives, and the
over-eager onAbort clear is removed (a genuine daemon failure never populates
inviteUrl, so recovery can't mask a real error).

#5/#8 (onAbort effect): the `$effect(() => { if (phase==='error') onAbort?.() })`
lacked the `if (!open) return` guard its siblings have and re-fired on every
parent re-render (fresh closure dep); it also double-cleared the intent
alongside applyError. Removed the effect, the onAbort prop, and the now-dead
clearShareIntent() store method.

#3 (applyError scope): applyError cleared pendingShareTargetPath for ANY review
error, so an unrelated error on another live owner room during the ~15s mint
window discarded a valid in-flight share intent. Now it clears only on a
non-room-scoped error (room_id: None — what a failed share emits).

#6 (dual-instance reviewer fixture): ATTN_DUAL_REVIEWER_FIXTURE was defaulted
at source time, so a caller that set ATTN_DUAL_FIXTURE after sourcing got a
reviewer fixture pinned to the old default. Resolved at start_dual call time
via a local instead.

Verified: web tests 31/0, svelte-check 0 errors, vite build OK, bash -n OK,
and a share happy-path smoke against the relay still reaches the invite URL.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 scripts/lib/dual-instance.sh       | 14 +++++++++-----
 web/src/App.svelte                 |  1 -
 web/src/lib/ShareDialog.svelte     | 21 ++++++---------------
 web/src/lib/review/store.svelte.ts | 21 +++++++--------------
 4 files changed, 22 insertions(+), 35 deletions(-)

diff --git a/scripts/lib/dual-instance.sh b/scripts/lib/dual-instance.sh
index e97f524..47837dd 100644
--- a/scripts/lib/dual-instance.sh
+++ b/scripts/lib/dual-instance.sh
@@ -46,8 +46,8 @@ __attn_dual_lib_dir() {
 : "${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). Defaults to the owner's fixture for backward compat.
-: "${ATTN_DUAL_REVIEWER_FIXTURE:=$ATTN_DUAL_FIXTURE}"
+# 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=""
@@ -120,12 +120,16 @@ 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 "$ATTN_DUAL_REVIEWER_FIXTURE" ]; then
-        echo "dual-instance: reviewer fixture missing at $ATTN_DUAL_REVIEWER_FIXTURE" >&2
+    if [ ! -f "$reviewer_fixture" ]; then
+        echo "dual-instance: reviewer fixture missing at $reviewer_fixture" >&2
         return 1
     fi
 
@@ -140,7 +144,7 @@ start_dual() {
         >"$ATTN_DUAL_OWNER/daemon.stdout.log" 2>"$ATTN_DUAL_OWNER/daemon.stderr.log" &
     ATTN_DUAL_OWNER_PID=$!
 
-    ATTN_HOME="$ATTN_DUAL_REVIEWER" ATTN_RELAY_URL="${ATTN_RELAY_URL:-}" "$ATTN_BIN" --no-fork "$ATTN_DUAL_REVIEWER_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=$!
 }
diff --git a/web/src/App.svelte b/web/src/App.svelte
index db8789b..479cd50 100644
--- a/web/src/App.svelte
+++ b/web/src/App.svelte
@@ -2621,7 +2621,6 @@
   existingRoomId={shareTargetIsCurrent ? (reviewStore.currentShare?.roomId ?? null) : null}
   shareErrorMessage={reviewStore.lastError?.message ?? ''}
   onStart={(params) => reviewStore.noteShareIntent(params.filePath)}
-  onAbort={() => reviewStore.clearShareIntent()}
   onClearError={() => reviewStore.clearLastError()}
 />
 <NamePrompt
diff --git a/web/src/lib/ShareDialog.svelte b/web/src/lib/ShareDialog.svelte
index 2ea6393..7c8bdc0 100644
--- a/web/src/lib/ShareDialog.svelte
+++ b/web/src/lib/ShareDialog.svelte
@@ -55,9 +55,6 @@
     shareErrorMessage?: string;
     writeToClipboard?: (text: string) => Promise<void>;
     onStart?: (params: ShareStartParams) => void;
-    /** Fired when the mint fails or times out, so the owner can release any
-     *  captured share-intent (see `reviewStore.clearShareIntent`). */
-    onAbort?: () => void;
     onClearError?: () => void;
   }
 
@@ -78,7 +75,6 @@
     shareErrorMessage = '',
     writeToClipboard,
     onStart,
-    onAbort,
     onClearError,
   }: Props = $props();
 
@@ -145,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;
@@ -177,15 +177,6 @@
     phase = 'error';
   });
 
-  // Release the captured share-intent whenever a mint ends in error (relay
-  // error OR the 15s relay-silent timeout, which emits no daemon error). Keeps
-  // a stale path from bleeding into a later, unrelated share (e.g. a CLI
-  // `attn review share` of a different file). The ready path already clears it
-  // via `applyShareReady`.
-  $effect(() => {
-    if (phase === 'error') onAbort?.();
-  });
-
   // Recompute the fingerprint whenever the owner key changes.
   $effect(() => {
     let cancelled = false;
diff --git a/web/src/lib/review/store.svelte.ts b/web/src/lib/review/store.svelte.ts
index 2729283..39217fb 100644
--- a/web/src/lib/review/store.svelte.ts
+++ b/web/src/lib/review/store.svelte.ts
@@ -189,16 +189,6 @@ export class ReviewStore {
     this.pendingShareTargetPath = path;
   }
 
-  /**
-   * Release a captured but unconsumed share-intent. Called when a share mint
-   * fails or times out (the relay-silent timeout produces no `applyError`, so
-   * the dialog signals this directly via its `onAbort`) so the path can't be
-   * mis-stamped onto a later, unrelated `ShareReady`.
-   */
-  clearShareIntent(): void {
-    this.pendingShareTargetPath = null;
-  }
-
   /**
    * Append-only buffer of imported review events. The thread selectors
    * below reconstruct typed `Thread[]` from this.
@@ -450,10 +440,13 @@ export class ReviewStore {
       ...error,
       updatedAt: Date.now(),
     };
-    // A share that errors out must release any captured share-intent so a
-    // stale path can't be mis-stamped onto a later, unrelated ShareReady
-    // (e.g. a subsequent CLI `attn review share` of a different file).
-    this.pendingShareTargetPath = null;
+    // A failed share emits a non-room-scoped error (room_id: None from
+    // emit_share_outcome's Err branch), so release the captured share-intent
+    // only for those. A room-scoped error on some OTHER live room must NOT
+    // discard an in-flight share intent for a different file.
+    if (error.roomId == null) {
+      this.pendingShareTargetPath = null;
+    }
   }
 
   clearLastError(): void {

From 38409db5a29fae8354a61492189540338fb88fe2 Mon Sep 17 00:00:00 2001
From: Angus Bezzina <37071175+angusbezzina@users.noreply.github.com>
Date: Tue, 9 Jun 2026 17:50:41 -0500
Subject: [PATCH 5/5] Thread owner_display_path through ShareReady; remove the
 frontend intent seam
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The durable fix for review finding #9 (and the #2/#4/#7 family that stemmed
from it). The owner's shared path is now carried on the ShareReady payload
straight from the daemon, instead of being reconstructed on the frontend via a
captured-intent slot.

Rust: ShareOutcome and ReviewUpdate::ShareReady gain owner_display_path
(= path.to_string_lossy(), the verbatim path the frontend passed to
reviewShare). Populated at both share() outcome sites (fresh mint + re-share)
and forwarded by emit_share_outcome. New assertion in
share_emits_invite_with_expected_shape locks the round-trip.

Frontend: applyShareReady stamps currentShare.ownerDisplayPath from
payload.ownerDisplayPath. Removed pendingShareTargetPath, noteShareIntent, the
ShareDialog onStart wiring, and applyError's intent-clear — the whole seam is
gone. This dissolves the earlier findings it caused:
  - #2 un-keyed cross-stamp (no global slot anymore),
  - #4 / #7 CLI `attn review share` yielding ownerDisplayPath='' (the path now
    comes from the daemon for CLI and GUI shares alike),
  - the noteShareIntent-timing footgun.

Also resolves two smaller findings:
  - #11: App resets shareTargetPath when the share dialog closes so a stale
    path can't feed shareTargetIsCurrent on the next open.
  - sweep: dev:collab warns when the owner and reviewer fixtures collide
    (windows would be indistinguishable).

This change is genuinely testable where the seam was not: the path logic now
lives in Rust (cargo test), not the un-unit-testable runes store.

Verified: cargo review tests 413/0 (incl. the new path assertion), web tests
31/0, svelte-check 0 errors, vite build OK, bash -n OK, and a share happy-path
smoke against the relay shows the invite (gate matches the daemon-supplied
path).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 scripts/dev-collab.sh              |  7 +++++
 src/review/bootstrap.rs            | 12 ++++++++
 src/review/manager.rs              |  5 ++++
 web/src/App.svelte                 |  8 ++++-
 web/src/lib/review/store.svelte.ts | 48 ++++--------------------------
 web/src/lib/types.ts               |  3 ++
 6 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/scripts/dev-collab.sh b/scripts/dev-collab.sh
index c49567e..04035b1 100755
--- a/scripts/dev-collab.sh
+++ b/scripts/dev-collab.sh
@@ -198,6 +198,13 @@ 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
diff --git a/src/review/bootstrap.rs b/src/review/bootstrap.rs
index 3400b0b..de26023 100644
--- a/src/review/bootstrap.rs
+++ b/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");
diff --git a/src/review/manager.rs b/src/review/manager.rs
index 6e4e23f..45afe5f 100644
--- a/src/review/manager.rs
+++ b/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,
diff --git a/web/src/App.svelte b/web/src/App.svelte
index 479cd50..567ea68 100644
--- a/web/src/App.svelte
+++ b/web/src/App.svelte
@@ -281,6 +281,12 @@
     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
   // entry cache-busts the HtmlViewer iframe URL so on-disk edits live-reload.
@@ -1961,6 +1967,7 @@
         reviewStore.applyShareReady({
           roomId: payload.roomId,
           inviteUrl: payload.inviteUrl,
+          ownerDisplayPath: payload.ownerDisplayPath,
           ownerSigningKey: payload.ownerSigningKey,
           mode: payload.mode,
           expiresAt: payload.expiresAt,
@@ -2620,7 +2627,6 @@
   ownerSigningKey={shareTargetIsCurrent ? (reviewStore.currentShare?.ownerSigningKey ?? '') : ''}
   existingRoomId={shareTargetIsCurrent ? (reviewStore.currentShare?.roomId ?? null) : null}
   shareErrorMessage={reviewStore.lastError?.message ?? ''}
-  onStart={(params) => reviewStore.noteShareIntent(params.filePath)}
   onClearError={() => reviewStore.clearLastError()}
 />
 <NamePrompt
diff --git a/web/src/lib/review/store.svelte.ts b/web/src/lib/review/store.svelte.ts
index 39217fb..7a3f361 100644
--- a/web/src/lib/review/store.svelte.ts
+++ b/web/src/lib/review/store.svelte.ts
@@ -169,26 +169,6 @@ export class ReviewStore {
     expiresAt: number;
   } | null>(null);
 
-  /**
-   * Path (file or folder) the owner just fired `reviewShare` for, captured the
-   * instant the IPC is sent and consumed by the async `applyShareReady`
-   * callback. `ReviewUpdate::ShareReady` (src/review/manager.rs) carries no
-   * path, and the owner never reliably receives their own snapshot back as a
-   * frontend event before the dialog times out, so this is the only reliable
-   * source for the shared path on the owner side. Cleared once `applyShareReady`
-   * consumes it.
-   */
-  pendingShareTargetPath = $state<string | null>(null);
-
-  /**
-   * Record the path the owner is about to share, just before the `reviewShare`
-   * IPC fires. `applyShareReady` reads this to stamp the resulting
-   * `currentShare`/room `share` record with its `ownerDisplayPath`.
-   */
-  noteShareIntent(path: string): void {
-    this.pendingShareTargetPath = path;
-  }
-
   /**
    * Append-only buffer of imported review events. The thread selectors
    * below reconstruct typed `Thread[]` from this.
@@ -440,13 +420,6 @@ export class ReviewStore {
       ...error,
       updatedAt: Date.now(),
     };
-    // A failed share emits a non-room-scoped error (room_id: None from
-    // emit_share_outcome's Err branch), so release the captured share-intent
-    // only for those. A room-scoped error on some OTHER live room must NOT
-    // discard an in-flight share intent for a different file.
-    if (error.roomId == null) {
-      this.pendingShareTargetPath = null;
-    }
   }
 
   clearLastError(): void {
@@ -543,6 +516,7 @@ export class ReviewStore {
   applyShareReady(payload: {
     roomId: RoomId;
     inviteUrl: string;
+    ownerDisplayPath: string;
     ownerSigningKey: string;
     mode: 'live' | 'async' | 'hybrid';
     expiresAt: number;
@@ -551,26 +525,14 @@ export class ReviewStore {
     const nextDismissed = new Set(this.dismissedRoomIds);
     nextDismissed.delete(payload.roomId);
     this.dismissedRoomIds = nextDismissed;
-    // The path the owner just shared, captured at `reviewShare`-fire time via
-    // `noteShareIntent`. Fall back to the room's previously-recorded path
-    // (idempotent re-share / reconnect re-emits ShareReady without a fresh
-    // noteShareIntent). NOTE: a CLI-initiated `attn review share <path>`
-    // (src/cli_review.rs) reaches here without ever calling noteShareIntent and,
-    // on a first-ever share, has no prior room record either — so it falls
-    // through to '' and the dialog will re-mint rather than re-show that invite.
-    // Benign (re-share is idempotent on the Rust side) and not a regression; to
-    // fully close it, thread the path through ReviewUpdate::ShareReady on the
-    // Rust side and stamp it here instead of relying on pendingShareTargetPath.
-    const ownerDisplayPath =
-      this.pendingShareTargetPath ??
-      this.rooms[payload.roomId]?.share?.ownerDisplayPath ??
-      '';
-    this.pendingShareTargetPath = null;
+    // `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,
+      ownerDisplayPath: payload.ownerDisplayPath,
       mode: payload.mode,
       expiresAt: payload.expiresAt,
     };
diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts
index 8588d7a..452a320 100644
--- a/web/src/lib/types.ts
+++ b/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';