Images render for the author who wrote them and the reviewer who receives them

scripts/dev-collab.sh

@@ -86,6 +86,14 @@ require_fixture() {
     fi
 }
 
+# The port `ATTN_RELAY_URL` points at. Everything below needs it as a number,
+# and defaulting to 8787 keeps a URL without an explicit port working.
+relay_port() {
+    local port
+    port=$(printf '%s' "$ATTN_RELAY_URL" | sed -E 's#^[a-z]+://[^/:]+:?([0-9]*).*#\1#')
+    printf '%s' "${port:-8787}"
+}
+
 # Start Miniflare via the relay package. Installs deps on first run.
 # Waits for /health to return 200 before returning.
 start_relay() {
@@ -94,7 +102,26 @@ start_relay() {
         (cd "$PROJECT_DIR/relay" && npm ci) >/dev/null
     fi
 
-    log "Starting Miniflare relay (wrangler dev --local --port 8787)"
+    # Refuse to start on a port someone else already holds (attn-1kvp). The
+    # health poll below cannot tell our relay from a stranger's — a foreign
+    # process answers /health identically — and wrangler SURVIVES its own
+    # EADDRINUSE, so `kill -0 $RELAY_PID` stays true while the runtime that
+    # failed to bind sits there dead. The harness then runs both daemons
+    # against a relay holding unrelated Durable Object state, and the user
+    # sees a reviewer that never opens the shared doc, a 409 on device
+    # registration, and a missing <room>.secret — three symptoms that point
+    # nowhere near a port clash. Fail here, where the cause is still legible.
+    local port
+    port=$(relay_port)
+    local holder
+    holder=$(lsof -ti "tcp:$port" -sTCP:LISTEN 2>/dev/null | head -1 || true)
+    if [ -n "$holder" ]; then
+        err "port $port is already in use by PID $holder ($(ps -p "$holder" -o comm= 2>/dev/null || echo 'unknown'))"
+        err "another relay or dev server is running. Stop it first:  kill $holder"
+        return 1
+    fi
+
+    log "Starting Miniflare relay (wrangler dev --local --port $port)"
     (
         cd "$PROJECT_DIR/relay"
         exec npm run dev
@@ -110,6 +137,14 @@ start_relay() {
             tail -30 "$RELAY_LOG" >&2 || true
             return 1
         fi
+        # wrangler keeps running after a failed bind, so the liveness check
+        # above never fires for the one failure that matters. The log is the
+        # only place it is stated.
+        if grep -q "Address already in use" "$RELAY_LOG" 2>/dev/null; then
+            err "relay could not bind port $port — another process took it"
+            tail -10 "$RELAY_LOG" >&2 || true
+            return 1
+        fi
         if curl -fsS "$ATTN_RELAY_URL/health" >/dev/null 2>&1; then
             log "Relay listening on $ATTN_RELAY_URL (health OK)"
             return 0
@@ -152,37 +187,48 @@ join_reviewer() {
     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 pasted || true
-    if [ -z "$pasted" ]; then
-        log "No invite supplied — daemons remain up. Ctrl+C to stop."
-        return 0
-    fi
 
-    # The share dialog offers two copyable things: the bare attn:// URL and
-    # the full `npx attnmd review join 'attn://…'` one-liner. Accept either
-    # (plus stray quotes/whitespace) by extracting the invite URL from
-    # whatever was pasted — passing the npx command through verbatim used to
-    # send garbage to the daemon while this script still claimed success.
-    invite=$(printf '%s' "$pasted" | grep -oE "attn://review/[^'\"[:space:]]+" | head -1)
-    if [ -z "$invite" ]; then
-        err "no attn://review/… invite found in the pasted text — copy either the Direct link or the npx command from the Share dialog"
-        log "Daemons remain up. Ctrl+C to stop."
-        return 0
-    fi
+    # Loop until a join succeeds or the user cancels (attn-0cnt). A single
+    # prompt meant any first-attempt failure — a truncated paste, a relay
+    # hiccup, an invite the daemon declined — could only be retried by tearing
+    # the whole harness down and starting over, since by then the script had
+    # already fallen through to "Ctrl+C to stop". The daemons are still up and
+    # the owner can share again, so the only thing missing was somewhere to
+    # put the next invite.
+    while true; do
+        printf 'Paste invite > '
+        IFS= read -r pasted || true
+        if [ -z "$pasted" ]; then
+            log "No invite supplied — daemons remain up. Ctrl+C to stop."
+            return 0
+        fi
 
-    log "Reviewer joining (windowed daemon)..."
-    # Route the join to the already-running reviewer DAEMON via its ATTN_HOME
-    # socket — deliberately NOT `--as-agent`, which forks a separate *headless*
-    # agent process (no window, no UI) and leaves the reviewer window idle.
-    # The daemon-routed join makes the reviewer's own window switch to the
-    # shared document, which is the experience a human reviewer expects.
-    if ATTN_HOME="$ATTN_DUAL_REVIEWER" ATTN_RELAY_URL="$ATTN_RELAY_URL" \
-        "$ATTN_BIN" review join "$invite"; then
-        log "Reviewer joined — both windows are now collaborating."
-    else
-        err "reviewer join failed — see daemon logs under $ATTN_DUAL_REVIEWER/"
-    fi
+        # The share dialog offers two copyable things: the bare attn:// URL and
+        # the full `npx attnmd review join 'attn://…'` one-liner. Accept either
+        # (plus stray quotes/whitespace) by extracting the invite URL from
+        # whatever was pasted — passing the npx command through verbatim used to
+        # send garbage to the daemon while this script still claimed success.
+        invite=$(printf '%s' "$pasted" | grep -oE "attn://review/[^'\"[:space:]]+" | head -1)
+        if [ -z "$invite" ]; then
+            err "no attn://review/… invite found in the pasted text — copy either the Direct link or the npx command from the Share dialog"
+            continue
+        fi
+
+        log "Reviewer joining (windowed daemon)..."
+        # Route the join to the already-running reviewer DAEMON via its ATTN_HOME
+        # socket — deliberately NOT `--as-agent`, which forks a separate *headless*
+        # agent process (no window, no UI) and leaves the reviewer window idle.
+        # The daemon-routed join makes the reviewer's own window switch to the
+        # shared document, which is the experience a human reviewer expects.
+        if ATTN_HOME="$ATTN_DUAL_REVIEWER" ATTN_RELAY_URL="$ATTN_RELAY_URL" \
+            "$ATTN_BIN" review join "$invite"; then
+            log "Reviewer joined — both windows are now collaborating."
+            return 0
+        fi
+
+        err "reviewer join failed — see $ATTN_DUAL_REVIEWER/attn.log"
+        err "share again from the owner window and paste the new invite, or press Enter to give up."
+    done
 }
 
 # Guard against double-fire: SIGINT + EXIT would otherwise both invoke

scripts/test-e2e.sh

@@ -266,6 +266,90 @@ result=$("$ATTN" --query '[class*="breadcrumb"], nav[aria-label]' | jq -r '.elem
 assert_contains "Breadcrumb shows nested path" "$result" "child.md"
 screenshot "06-nested-file"
 
+# ===================================================================
+# TEST SUITE 3: Relative image resolution (attn-cgev)
+# ===================================================================
+
+echo ""
+echo "=== Test Suite 3: Relative Images (images.md) ==="
+
+# Poll a synchronous eval until it stops returning `null`/empty. `--eval` hands
+# back whatever the expression evaluates to, JSON-encoded, and does NOT await a
+# Promise — so waiting has to happen out here, not in the page.
+poll_eval() {
+    local js="$1"
+    local tries=0
+    local out=""
+    while [ "$tries" -lt 40 ]; do
+        out=$("$ATTN" --eval "$js" 2>/dev/null || echo "")
+        case "$out" in
+            ''|null|'""'|false|0) ;;
+            *) echo "$out"; return 0 ;;
+        esac
+        sleep 0.1
+        tries=$((tries + 1))
+    done
+    echo "$out"
+}
+
+start_daemon "$FIXTURES/images.md"
+
+# Single-file mode, so directory ordering in tests/fixtures/ is irrelevant here.
+"$ATTN" --wait-for '.attn-doc .md-image img' --timeout 5000 >/dev/null 2>&1
+
+result=$("$ATTN" --query '.attn-doc .md-image img' | jq -r '.count' 2>/dev/null || echo "0")
+assert_truthy "Image nodes rendered" "$result"
+
+# The whole bug: a relative src used to reach the DOM verbatim and 404 against
+# the app origin. It must now address the file through the attn:// handler.
+result=$("$ATTN" --query '.attn-doc .md-image img' | jq -r '.elements[0].attributes.src' 2>/dev/null || echo "")
+assert_contains "Relative src resolved through attn://" "$result" "attn://localhost/"
+assert_contains "Resolved against the markdown file's own directory" "$result" "tests/fixtures/diagram.png"
+
+# The authored src is what gets serialized back to disk, so it must survive.
+# Read through --query rather than --eval: the webview JSON-escapes '/' in a
+# returned string, which turns every path assertion into a slash-counting exercise.
+result=$("$ATTN" --query '.attn-doc .md-image' | jq -r '.elements[0].attributes["data-src"]' 2>/dev/null || echo "")
+assert_eq "Authored src preserved on the node" "$result" "./diagram.png"
+
+# naturalWidth is not a DOM attribute, and decode is asynchronous.
+result=$(poll_eval "Array.from(document.querySelectorAll('.attn-doc .md-image img')).filter((img) => img.complete && img.naturalWidth > 0).length")
+assert_truthy "Local image bytes actually decoded (naturalWidth > 0)" "$result"
+
+# Every local src in the fixture except the deliberate miss should decode: two
+# PNGs (sibling, bare, subdirectory) plus the SVG, which carries explicit
+# width/height so WebKit reports an intrinsic size for it.
+result=$(poll_eval "(() => { const n = Array.from(document.querySelectorAll('.attn-doc .md-image img')).filter((img) => img.complete && img.naturalWidth > 0).length; return n >= 4 ? n : null; })()")
+assert_eq "All four local assets decoded" "$result" "4"
+
+# A missing file gets the document's own placeholder, not the platform glyph.
+# Anchored on the deliberate miss by its authored src, NOT on "the first broken
+# image": the remote https src in this fixture also fails (there is no network
+# in the E2E environment), and which of the two reports `error` first is a race
+# between a local 404 and a DNS timeout.
+GONE='.attn-doc .md-image[data-src="./gone.png"]'
+result=$(poll_eval "document.querySelector('$GONE[data-broken]')?.textContent")
+assert_contains "Missing image shows the alt text" "$result" "A diagram that moved"
+assert_contains "Missing image names the file" "$result" "gone.png"
+assert_contains "Missing image is labelled, not left blank" "$result" "Image didn’t load"
+
+# The card is announced as one thing, not three loose runs: the eyebrow, alt and
+# filename are aria-hidden and the wrapper carries a single composed label.
+result=$(poll_eval "document.querySelector('$GONE .md-image-fallback')?.getAttribute('aria-label')")
+assert_contains "Missing image is announced as a single image role" "$result" "A diagram that moved"
+
+# The selection ring and the drag handle both land on the NodeView's own
+# element, so the wrapper has to hug the picture rather than span the measure.
+result=$(poll_eval "(() => { const w = document.querySelector('.attn-doc .md-image[data-loaded]'); if (!w) return null; const img = w.querySelector('img'); return Math.abs(w.getBoundingClientRect().width - img.getBoundingClientRect().width) < 1 ? 'hugs' : 'spans'; })()")
+# `--eval` hands back a JSON-encoded string, hence the contains form.
+assert_contains "Image wrapper hugs the image, not the measure" "$result" "hugs"
+
+# A remote src has no business being rewritten.
+result=$("$ATTN" --query '.attn-doc .md-image[data-src^="https:"] img' | jq -r '.elements[0].attributes.src' 2>/dev/null || echo "")
+assert_eq "Absolute URL passes through untouched" "$result" "https://example.com/pixel.png"
+
+screenshot "07-relative-images"
+
 # ===================================================================
 # Summary
 # ===================================================================

src/cli_review.rs

@@ -491,6 +491,12 @@ fn validate_invite_for_join(invite: &str) -> Result<()> {
     Ok(())
 }
 
+/// How long the CLI waits for a join to complete before giving up on the
+/// answer. Generous enough for a cold relay handshake, short enough that a
+/// wedged network does not pin a terminal open. The daemon keeps trying past
+/// this point — only the CLI stops waiting.
+const JOIN_WAIT_TIMEOUT: Duration = Duration::from_secs(30);
+
 /// Hand the invite to the running attn daemon so it joins as its OWN device
 /// identity (the same device the app window presents). This keeps the CLI join
 /// consistent with the daemon — a no-`--as-agent` join shows up in the app.
@@ -501,31 +507,38 @@ fn validate_invite_for_join(invite: &str) -> Result<()> {
 fn run_join_via_daemon(invite: &str) -> Result<()> {
     validate_invite_for_join(invite)?;
     crate::daemon::replace_stale_daemon().context("check running attn daemon")?;
-    match crate::daemon::send_review_join(invite) {
-        Ok(()) => {
-            println!("join request sent to the running attn daemon");
-            println!("  invite: {invite}");
-            return Ok(());
+    // Wait for the join to actually run rather than for the socket write to
+    // succeed (attn-q8gs). The old fire-and-forget path printed "join request
+    // sent" and exited 0 for joins the daemon went on to reject, which left
+    // the reviewer window on its old document with nothing in the terminal
+    // suggesting anything was wrong — the failure was only ever visible in
+    // the daemon's own log.
+    match crate::daemon::send_review_join_wait(invite, Some(JOIN_WAIT_TIMEOUT)) {
+        Ok(room_id) => {
+            println!("joined review room {room_id}");
+            Ok(())
         }
         Err(_err) if crate::daemon::send_info().is_err() => {
+            // No daemon at all — that error is "no daemon running", not a
+            // failed join. Start one on the current directory and retry, which
+            // keeps the invite one-liner useful for a first-time reviewer.
             start_app_for_join()?;
             wait_for_daemon(Duration::from_secs(8))?;
-            crate::daemon::send_review_join(invite).map_err(|join_err| {
-                anyhow::anyhow!(
-                    "started attn, but could not send the review invite ({join_err}).\n\
-                     Invite: {invite}"
-                )
-            })?;
-        }
-        Err(err) => {
-            return Err(anyhow::anyhow!(
-                "could not send the review invite to the running attn daemon ({err})."
-            ));
+            let room_id = crate::daemon::send_review_join_wait(invite, Some(JOIN_WAIT_TIMEOUT))
+                .map_err(|join_err| {
+                    anyhow::anyhow!(
+                        "started attn, but the review join failed ({join_err}).\n\
+                         Invite: {invite}"
+                    )
+                })?;
+            println!("joined review room {room_id}");
+            Ok(())
         }
+        Err(err) => Err(anyhow::anyhow!(
+            "review join failed ({err}).\n\
+             Invite: {invite}"
+        )),
     }
-    println!("join request sent to the running attn daemon");
-    println!("  invite: {invite}");
-    Ok(())
 }
 
 fn start_app_for_join() -> Result<()> {