From 8f055f71bf07e17be67923965f07bc7ec44d1259 Mon Sep 17 00:00:00 2001
From: Avi <avi@aiolabs.com>
Date: Wed, 23 Sep 2026 13:02:19 -0500
Subject: [PATCH] fix(nip46): spec-correct connect params, full-handshake
 [NIP46] trace, visible handshake errors

---
 frontend/src/screens/SignerModeScreen.tsx   |  20 ++
 frontend/src/test/SignerModeScreen.test.tsx |  72 +++++
 frontend/src/test/fakeBackend.ts            |  46 +++
 src/signer/nip46_client.rs                  | 334 +++++++++++++++++---
 tests/nip46_e2e.rs                          | 253 ++++++++++++++-
 5 files changed, 673 insertions(+), 52 deletions(-)
 create mode 100644 frontend/src/test/SignerModeScreen.test.tsx

diff --git a/frontend/src/screens/SignerModeScreen.tsx b/frontend/src/screens/SignerModeScreen.tsx
index 1385bf0..2bdd00f 100644
--- a/frontend/src/screens/SignerModeScreen.tsx
+++ b/frontend/src/screens/SignerModeScreen.tsx
@@ -592,6 +592,11 @@ export function SignerModeScreen() {
                     Waiting for the signer to scan… the connection appears automatically once
                     approved. The code expires after a few minutes.
                   </p>
+                  {nip46StatusState?.error && (
+                    <Alert tone="error" title="Pairing failed">
+                      {nip46StatusState.error}
+                    </Alert>
+                  )}
                   {pairError && <ErrorText>{pairError}</ErrorText>}
                   <div className="settings-inline" style={{ justifyContent: 'center' }}>
                     <Button variant="ghost" onClick={() => void handlePairCancel()}>
@@ -636,6 +641,21 @@ export function SignerModeScreen() {
                   </div>
                   {error && <ErrorText>{error}</ErrorText>}
                   {pairError && <ErrorText>{pairError}</ErrorText>}
+                  {/* A failed handshake must never look like an idle form:
+                      surface the backend's error so a timeout is visible. */}
+                  {nip46StatusState?.error && (
+                    <Alert tone="error" title="Connection failed">
+                      {nip46StatusState.error}
+                    </Alert>
+                  )}
+                  {/* A sent-but-unapproved connection request is in flight:
+                      say so instead of showing a blank form. */}
+                  {!nip46StatusState?.error && (nip46StatusState?.relays?.length ?? 0) > 0 && (
+                    <p className="hint">
+                      Connection request sent — approve it in Amber. This updates automatically; it
+                      can take up to a couple of minutes on a slow network.
+                    </p>
+                  )}
                   <div className="settings-inline">
                     {mode === 'nip46_client' && (
                       <Button
diff --git a/frontend/src/test/SignerModeScreen.test.tsx b/frontend/src/test/SignerModeScreen.test.tsx
new file mode 100644
index 0000000..a4a94f4
--- /dev/null
+++ b/frontend/src/test/SignerModeScreen.test.tsx
@@ -0,0 +1,72 @@
+import { screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { beforeEach, expect, vi } from 'vitest';
+import { SignerModeScreen } from '../screens/SignerModeScreen';
+import { renderWithApp } from './render';
+import { createFakeBackend, installFakeBackend } from './fakeBackend';
+import { makeState } from './apiMock';
+
+vi.mock('qrcode', () => ({
+  default: { toDataURL: vi.fn(async () => 'data:image/png;base64,QR') },
+}));
+
+function installNip46Backend() {
+  const backend = createFakeBackend(makeState({ signer_mode: 'nip46_client' }));
+  installFakeBackend(backend);
+  return backend;
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe('SignerModeScreen handshake states', () => {
+  it('shows the QR waiting hint while a pairing is in flight', async () => {
+    const backend = installNip46Backend();
+    const user = userEvent.setup();
+    renderWithApp(<SignerModeScreen />);
+
+    await screen.findByRole('button', { name: /Show QR/i });
+    await user.click(screen.getByRole('button', { name: /Show QR/i }));
+
+    expect(await screen.findByText(/Waiting for the signer to scan/i)).toBeInTheDocument();
+    expect(backend.requests.some((r) => r.method === 'nip46_pair_start')).toBe(true);
+  });
+
+  it('shows a connecting hint after a paste-URI connect is sent but unapproved', async () => {
+    const backend = installNip46Backend();
+    backend.setNip46({
+      type: 'nip46',
+      connected: false,
+      relays: ['wss://relay.test'],
+      connected_relays: ['wss://relay.test'],
+      pending_approvals: [],
+    });
+    renderWithApp(<SignerModeScreen />);
+
+    expect(await screen.findByText(/Connection request sent/i)).toBeInTheDocument();
+  });
+
+  it('surfaces a failed handshake as a visible error, not a silent idle form', async () => {
+    const backend = installNip46Backend();
+    backend.setNip46({
+      type: 'nip46',
+      connected: false,
+      relays: ['wss://relay.test'],
+      connected_relays: [],
+      error:
+        'The signer would not reveal its public key (timeout). Keep Amber open in the foreground with network access and try again.',
+      pending_approvals: [],
+    });
+    renderWithApp(<SignerModeScreen />);
+
+    expect(await screen.findByText('Connection failed')).toBeInTheDocument();
+    expect(
+      await screen.findByText(/The signer would not reveal its public key/i),
+    ).toBeInTheDocument();
+    // The failure must poll through the same status channel the screen reads.
+    await waitFor(() =>
+      expect(backend.requests.some((r) => r.method === 'nip46_status')).toBe(true),
+    );
+  });
+});
diff --git a/frontend/src/test/fakeBackend.ts b/frontend/src/test/fakeBackend.ts
index d0ee3cc..1cab35c 100644
--- a/frontend/src/test/fakeBackend.ts
+++ b/frontend/src/test/fakeBackend.ts
@@ -2,6 +2,7 @@ import type {
   AppState,
   BackendResponse,
   FeedItem,
+  Nip46SignerStatus,
   ProfileSummary,
   PublishReport,
   RelayTestResult,
@@ -42,6 +43,9 @@ export interface FakeBackend {
   /** Current NIP-46 signer status. */
   signer: SignerStatus;
   setSigner: (next: SignerStatus) => void;
+  /** NIP-46 client handshake status backing the nip46_* methods. */
+  nip46: Nip46SignerStatus;
+  setNip46: (next: Nip46SignerStatus) => void;
   /** Standing "always allow" grants returned by signer_grants_list. */
   signerGrants: SignerGrant[];
   /** Notes returned by `feed_get`. */
@@ -109,6 +113,16 @@ export function createFakeBackend(initial?: AppState): FakeBackend {
     setSigner(next) {
       backend.signer = next;
     },
+    nip46: {
+      type: 'nip46',
+      connected: false,
+      relays: [],
+      connected_relays: [],
+      pending_approvals: [],
+    },
+    setNip46(next) {
+      backend.nip46 = next;
+    },
     signerGrants: [],
     feedItems: [
       {
@@ -181,6 +195,38 @@ export function createFakeBackend(initial?: AppState): FakeBackend {
       case 'get_state':
         return state;
 
+      // NIP-46 client handshake surface used by SignerModeScreen. The fake
+      // keeps a Nip46SignerStatus-shaped object so handshake-state tests
+      // (pairing URI, connecting relays, failure errors) run without relays.
+      case 'nip46_status':
+        return backend.nip46;
+      case 'nip46_pair_start': {
+        const next = {
+          ...backend.nip46,
+          pairing_uri: `nostrconnect://deadbeef?relay=${encodeURIComponent('wss://relay.test')}&secret=fake`,
+        };
+        backend.setNip46(next);
+        return next;
+      }
+      case 'nip46_connect': {
+        const next = { ...backend.nip46, relays: ['wss://relay.test'] };
+        backend.setNip46(next);
+        return next;
+      }
+      case 'nip46_disconnect': {
+        const next: Nip46SignerStatus = {
+          type: 'nip46',
+          connected: false,
+          relays: [],
+          connected_relays: [],
+          pending_approvals: [],
+        };
+        backend.setNip46(next);
+        return next;
+      }
+      case 'nip46_approve':
+        return backend.nip46;
+
       case 'create_profile': {
         const label = String(params.label ?? '');
         const profile: ProfileSummary = {
diff --git a/src/signer/nip46_client.rs b/src/signer/nip46_client.rs
index dd5e404..ec4c3e1 100644
--- a/src/signer/nip46_client.rs
+++ b/src/signer/nip46_client.rs
@@ -267,32 +267,40 @@ impl Nip46ClientSigner {
     /// Connect to a NIP-46 signer using a nostrconnect:// URI.
     pub async fn connect(&self, uri: &str, label: String) -> Result<Nip46Status, AppError> {
         let parsed = Self::parse_connect_uri(uri)?;
-
-        // For NIP-46 Client the identity lives on the external signer, so local
-        // profile is optional. Use ephemeral keys for the session, preferring
-        // the local vault profile if available and unlocked.
-        let (keys, active_npub) = {
-            let app = self.app.lock().await;
-            if let Some(npub) = app.vault.active_profile.clone() {
-                if !app.is_locked() {
-                    let vault_key = app.vault_key().copied();
-                    if let Ok(secret_hex) =
-                        profiles::resolve_secret_key(&app.vault, &npub, vault_key.as_ref())
-                    {
-                        if let Ok(secret_key) = profiles::parse_secret_key(&secret_hex) {
-                            (Keys::new(secret_key), Some(npub))
-                        } else {
-                            (Keys::generate(), Some(npub))
-                        }
-                    } else {
-                        (Keys::generate(), Some(npub))
-                    }
-                } else {
-                    (Keys::generate(), Some(npub))
-                }
+        eprintln!(
+            "[NIP46] parsed connect URI: peer={} relays={:?} secret={} perms={}",
+            parsed.peer.to_hex(),
+            parsed
+                .relays
+                .iter()
+                .map(|r| r.to_string())
+                .collect::<Vec<_>>(),
+            if parsed.secret.is_some() {
+                "present"
             } else {
-                (Keys::generate(), None)
-            }
+                "absent"
+            },
+            if parsed.permissions.is_some() {
+                "present"
+            } else {
+                "absent"
+            },
+        );
+
+        // Per NIP-46 ("client-keypair ... largely disposable ... delete it on
+        // logout") the client keypair is ALWAYS ephemeral: it identifies this
+        // session on the wire and must never be confused with the user's
+        // actual identity (learned later via `get_public_key`) or with a
+        // local vault key. Reusing a vault key here would also link the
+        // throwaway session to the long-term identity.
+        let keys = Keys::generate();
+        eprintln!(
+            "[NIP46] generated client keypair: client pubkey={}",
+            keys.public_key().to_hex()
+        );
+        let active_npub = {
+            let app = self.app.lock().await;
+            app.vault.active_profile.clone()
         };
 
         // Check for permission broadening against stored connections for
@@ -523,6 +531,21 @@ impl Nip46ClientSigner {
             secret.clone(),
         )
         .to_string();
+        // NOTE: the URI carries the connection secret — log the shape only,
+        // never the secret itself.
+        eprintln!(
+            "[NIP46] generated client keypair: client pubkey={}",
+            keys.public_key().to_hex()
+        );
+        eprintln!(
+            "[NIP46] connection secret: present ({} hex chars)",
+            secret.len()
+        );
+        eprintln!(
+            "[NIP46] nostrconnect URI generated ({} chars, {} relays); waiting for Amber response",
+            uri.len(),
+            relays.len()
+        );
 
         {
             let mut inner = self.inner.lock().await;
@@ -579,6 +602,7 @@ impl Nip46ClientSigner {
             .authenticator(SignerAuthenticator::new(keys.clone()))
             .build();
         for url in &relays {
+            eprintln!("[NIP46] relay connecting: {url}");
             client
                 .add_relay(url.to_string())
                 .await
@@ -599,6 +623,9 @@ impl Nip46ClientSigner {
             }
             tokio::time::sleep(Duration::from_millis(250)).await
         };
+        for url in &connected {
+            eprintln!("[NIP46] relay connected: {url}");
+        }
         if connected.is_empty() {
             return Err("None of the relays answered".to_string());
         }
@@ -607,6 +634,10 @@ impl Nip46ClientSigner {
         // signer replies to after scanning the QR. The author is unknown
         // until the first event lands.
         let our_pk = keys.public_key();
+        eprintln!(
+            "[NIP46] subscription created: kind=24133 #p={} (pairing listen)",
+            our_pk.to_hex()
+        );
         let filter = Filter::new()
             .kind(Kind::NostrConnect)
             .tag(Tag::public_key(our_pk));
@@ -642,6 +673,11 @@ impl Nip46ClientSigner {
                 &event.pubkey.to_hex()[..16],
                 event.content.len()
             );
+            eprintln!(
+                "[NIP46] received kind 24133: author={} tags={:?}",
+                event.pubkey.to_hex(),
+                event.tags
+            );
             // Local-only debug capture (never leaves this machine): keep the
             // full event so a failed handshake can be dissected offline.
             // Loopback (e2e harness) pairings are excluded so test traffic
@@ -677,7 +713,14 @@ impl Nip46ClientSigner {
                 continue;
             };
             let plain = match nip44_decrypt(&conv, &event.content) {
-                Ok(p) => p,
+                Ok(p) => {
+                    eprintln!(
+                        "[NIP46] decrypting response from {}: OK ({} plaintext bytes)",
+                        event.pubkey.to_hex(),
+                        p.len()
+                    );
+                    p
+                }
                 Err(e) => {
                     // Surface the REAL failure (HMAC vs wrong key vs invalid
                     // padding): a malformed NIP-44 frame looks identical to a
@@ -715,7 +758,9 @@ impl Nip46ClientSigner {
                     return Err(format!("The signer rejected the connection: {msg}"));
                 }
                 let result = shaped["result"].as_str().unwrap_or("");
+                // Never log the secret values themselves — only the verdict.
                 if result != secret && result != "ack" {
+                    eprintln!("[NIP46] secret validation: FAIL (echo did not match)");
                     eprintln!(
                         "[nip46 pairing] connect response echoed the wrong secret — ignoring"
                     );
@@ -725,6 +770,12 @@ impl Nip46ClientSigner {
                     continue;
                 }
                 eprintln!("[nip46 pairing] connect response accepted (secret echo verified)");
+                eprintln!("[NIP46] secret validation: PASS");
+                eprintln!(
+                    "[NIP46] remote signer pubkey (response author): {}",
+                    event.pubkey.to_hex()
+                );
+                eprintln!("[NIP46] connection established (pairing leg)");
                 if live_capture {
                     pairing_trace("connect response accepted (secret echo verified)");
                 }
@@ -744,6 +795,10 @@ impl Nip46ClientSigner {
             };
             if request.method != "connect" {
                 // Anything before the handshake is premature — ignore.
+                eprintln!(
+                    "[NIP46] decrypted method: {} (pre-handshake — ignored)",
+                    request.method
+                );
                 eprintln!("[nip46 pairing] pre-handshake '{}' ignored", request.method);
                 if live_capture {
                     pairing_trace(&format!("pre-handshake '{}' ignored", request.method));
@@ -765,6 +820,11 @@ impl Nip46ClientSigner {
                 return Err(format!("Could not answer the connect request: {e}"));
             }
             eprintln!("[nip46 pairing] connect answered; awaiting identity handshake");
+            eprintln!(
+                "[NIP46] decrypted method: connect (legacy request shape) from {}",
+                event.pubkey.to_hex()
+            );
+            eprintln!("[NIP46] connection established (pairing leg)");
             if live_capture {
                 pairing_trace("connect answered; awaiting identity handshake");
             }
@@ -906,11 +966,19 @@ impl Nip46ClientSigner {
         String,
     > {
         let filter = Filter::new().kind(Kind::NostrConnect).author(peer);
+        eprintln!(
+            "[NIP46] subscription created: kind=24133 author={} (signer replies)",
+            peer.to_hex()
+        );
         let notifications = client.notifications();
         let subscription = client
             .subscribe(filter)
             .await
             .map_err(|e| format!("Could not subscribe: {e}"))?;
+        eprintln!(
+            "[NIP46] subscription registered: id={} — listening before any publish",
+            subscription.id()
+        );
         Ok((notifications, subscription))
     }
 
@@ -1021,9 +1089,17 @@ impl Nip46ClientSigner {
     fn fail(&self, message: impl Into<String>) {
         let message = message.into();
         eprintln!("[nip46] session failed: {message}");
+        eprintln!("[NIP46] session failed: {message}");
         pairing_trace(&format!("session failed: {message}"));
         if let Ok(mut inner) = self.inner.try_lock() {
-            inner.phase = Nip46Phase::Error(message);
+            // First failure wins: the demux loop exits with a generic
+            // "Connect handshake failed" AFTER the handshake task already
+            // recorded the specific cause — overwriting it would hide the
+            // useful error ("signer would not reveal its public key") behind
+            // a generic one in the UI.
+            if !matches!(inner.phase, Nip46Phase::Error(_)) {
+                inner.phase = Nip46Phase::Error(message);
+            }
             inner.task = None;
             inner.client = None;
             inner.conversation_key = None;
@@ -1166,14 +1242,46 @@ impl Nip46ClientSigner {
             return Err(SigningError::Network { detail });
         }
 
+        eprintln!(
+            "[NIP46] requesting {}: id={} timeout={}s",
+            method,
+            &id[..8.min(id.len())],
+            timeout.as_secs()
+        );
         match tokio::time::timeout(timeout, receiver).await {
-            Ok(Ok(result)) => result.map_err(|detail| SigningError::Internal { detail }),
+            Ok(Ok(result)) => {
+                match &result {
+                    Ok(_) => eprintln!(
+                        "[NIP46] received {} response: id={}",
+                        method,
+                        &id[..8.min(id.len())]
+                    ),
+                    Err(detail) => eprintln!(
+                        "[NIP46] received {} error response: id={} detail={}",
+                        method,
+                        &id[..8.min(id.len())],
+                        detail
+                    ),
+                }
+                result.map_err(|detail| SigningError::Internal { detail })
+            }
             Ok(Err(_)) => {
                 // The waiter was dropped (disconnect/fail) while awaiting.
+                eprintln!(
+                    "[NIP46] {} id={}: waiter dropped (disconnect/fail)",
+                    method,
+                    &id[..8.min(id.len())]
+                );
                 Err(SigningError::NotConnected)
             }
             Err(_) => {
                 // Abandon the waiter so a late response finds nothing.
+                eprintln!(
+                    "[NIP46] {} id={}: TIMED OUT after {}s with no response",
+                    method,
+                    &id[..8.min(id.len())],
+                    timeout.as_secs()
+                );
                 self.inner.lock().await.remote_pending.remove(&id);
                 Err(SigningError::Timeout)
             }
@@ -1194,11 +1302,17 @@ impl Nip46ClientSigner {
         };
 
         // Connect to relays
+        eprintln!(
+            "[NIP46] generated client keypair already in session; peer={} relays={:?}",
+            uri.peer.to_hex(),
+            uri.relays.iter().map(|r| r.to_string()).collect::<Vec<_>>(),
+        );
         let client = Client::builder()
             .authenticator(SignerAuthenticator::new(keys.clone()))
             .build();
 
         for url in &uri.relays {
+            eprintln!("[NIP46] relay connecting: {url}");
             client
                 .add_relay(url.to_string())
                 .await
@@ -1220,6 +1334,9 @@ impl Nip46ClientSigner {
             }
             tokio::time::sleep(Duration::from_millis(250)).await
         };
+        for url in &connected {
+            eprintln!("[NIP46] relay connected: {url}");
+        }
 
         if connected.is_empty() {
             return Err("None of the relays answered".to_string());
@@ -1297,13 +1414,35 @@ impl Nip46ClientSigner {
                 };
 
             let event = *incoming;
-            if event.kind != Kind::NostrConnect || event.pubkey != uri.peer {
+            // Log EVERY inbound event on this subscription: a silent `continue`
+            // here once hid live handshake traffic, so each drop names itself.
+            if event.kind != Kind::NostrConnect {
+                eprintln!(
+                    "[NIP46] demux: ignoring kind {} from {} (want kind 24133)",
+                    u16::from(event.kind),
+                    event.pubkey.to_hex()
+                );
+                continue;
+            }
+            if event.pubkey != uri.peer {
+                eprintln!(
+                    "[NIP46] demux: ignoring 24133 from {} (want author {})",
+                    event.pubkey.to_hex(),
+                    uri.peer.to_hex()
+                );
                 continue;
             }
 
             let plaintext = match nip44_decrypt(&conversation, &event.content) {
                 Ok(p) => p,
-                Err(_) => continue,
+                Err(e) => {
+                    eprintln!(
+                        "[NIP46] demux: payload from {} did not decrypt: {}",
+                        event.pubkey.to_hex(),
+                        e.message()
+                    );
+                    continue;
+                }
             };
 
             // A payload carrying `result`/`error` is a response to a request
@@ -1315,7 +1454,13 @@ impl Nip46ClientSigner {
                 serde_json::from_str(&plaintext).unwrap_or(serde_json::Value::Null);
             if shaped.get("result").is_some() || shaped.get("error").is_some() {
                 let id = shaped.get("id").and_then(|v| v.as_str()).unwrap_or("");
-                let outcome = if shaped.get("error").is_some() && !shaped["error"].is_null() {
+                let is_error = shaped.get("error").is_some() && !shaped["error"].is_null();
+                eprintln!(
+                    "[NIP46] received response: id={} {} — routing to waiter",
+                    id,
+                    if is_error { "error" } else { "result" }
+                );
+                let outcome = if is_error {
                     Err(shaped["error"]
                         .as_str()
                         .unwrap_or("The signer reported an error.")
@@ -1326,9 +1471,14 @@ impl Nip46ClientSigner {
                 let waiter = self.inner.lock().await.remote_pending.remove(id);
                 match waiter {
                     Some(pending) => {
+                        eprintln!("[NIP46] waiter found for id={} — delivering", id);
                         let _ = pending.sender.send(outcome);
                     }
                     None => {
+                        eprintln!(
+                            "[NIP46] stale/duplicate response: no waiter for id={} — dropped",
+                            id
+                        );
                         eprintln!("Ignoring NIP-46 response with no waiting request: {id}");
                     }
                 }
@@ -1337,8 +1487,16 @@ impl Nip46ClientSigner {
 
             let request: RawRequest = match serde_json::from_str(&plaintext) {
                 Ok(r) => r,
-                Err(_) => continue,
+                Err(e) => {
+                    // Unreachable with the lenient parser, but named if hit.
+                    eprintln!("[NIP46] demux: request parse failed: {e}");
+                    continue;
+                }
             };
+            eprintln!(
+                "[NIP46] demux: incoming request method={} id={}",
+                request.method, request.id
+            );
 
             let response = if self.requires_approval(&request.method) {
                 self.gated_response(&keys, &request).await
@@ -1604,6 +1762,19 @@ impl Nip46ClientSigner {
         }
     }
 
+    /// Build the `connect` RPC params per NIP-46: the first param is the
+    /// REMOTE signer's pubkey (the URI authority), followed by the optional
+    /// connection secret. The client's own pubkey is already the event
+    /// author — sending it as params[0] is a spec violation that strict
+    /// signers (Amber) reject with silence, stalling the handshake.
+    fn connect_params(peer: PublicKey, secret: Option<&str>) -> Vec<String> {
+        let mut params = vec![peer.to_hex()];
+        if let Some(secret) = secret {
+            params.push(secret.to_string());
+        }
+        params
+    }
+
     /// The connect handshake, run as its own task alongside the demux loop.
     ///
     /// 1. Send `connect` (our client pubkey + the URI secret, resolved
@@ -1654,24 +1825,27 @@ impl Nip46ClientSigner {
             }
         };
 
-        let mut params = vec![{
-            let inner = self.inner.lock().await;
-            inner
-                .keys
-                .as_ref()
-                .ok_or("No session keys")?
-                .public_key()
-                .to_hex()
-        }];
-        if let Some(secret) = &secret {
-            params.push(secret.clone());
-        }
+        let params = Self::connect_params(peer, secret.as_deref());
+        eprintln!(
+            "[NIP46] requesting connect: peer={} params=[remote_pubkey, {}] — awaiting signer ack",
+            peer.to_hex(),
+            if secret.is_some() {
+                "secret:present"
+            } else {
+                "secret:absent"
+            },
+        );
 
         // Await the ack (may wait on a human approving in Amber).
         let ack = self
             .send_rpc("connect", params, HANDSHAKE_TIMEOUT, false)
             .await
-            .map_err(|e| format!("The signer refused the connection: {e}"))?;
+            .map_err(|e| {
+                format!(
+                    "The signer did not approve the connection ({e}). Keep Amber open in the foreground and try again."
+                )
+            })?;
+        eprintln!("[NIP46] connect ack received");
 
         // Anti-spoofing: per NIP-46, a signer answering a nostrconnect://
         // (secret-carrying) handshake must echo the secret back. A mismatch
@@ -1679,11 +1853,13 @@ impl Nip46ClientSigner {
         if let Some(expected) = &expected_secret {
             let trimmed = ack.trim();
             if trimmed != expected.as_str() {
+                eprintln!("[NIP46] secret validation: FAIL (connect ack echo did not match)");
                 return Err(
                     "The signer did not echo the connection secret; refusing the connection."
                         .to_string(),
                 );
             }
+            eprintln!("[NIP46] secret validation: PASS (connect ack echo matched)");
         }
 
         self.adopt_identity(peer).await
@@ -1724,17 +1900,49 @@ impl Nip46ClientSigner {
         // the answer never comes. A retry lands while the signer is
         // listening. Non-timeout failures (refusal, bad payload) still fail
         // fast.
+        eprintln!("[NIP46] requesting get_public_key (identity resolution)");
         let mut attempt = 0u32;
         let identity = loop {
             attempt += 1;
+            // Fail fast with a USEFUL error when no relay is even connected:
+            // waiting out a 30s timeout here would hide a dead transport.
+            let connected_relays: Vec<String> = {
+                let inner = self.inner.lock().await;
+                match inner.client.as_ref() {
+                    Some(client) => client
+                        .relays()
+                        .all()
+                        .await
+                        .into_iter()
+                        .filter(|(_, relay)| relay.status().is_connected())
+                        .map(|(url, _)| url.to_string())
+                        .collect(),
+                    None => Vec::new(),
+                }
+            };
+            if connected_relays.is_empty() {
+                return Err(
+                    "Lost the relay connection while waiting for the signer. Reconnect and try again."
+                        .to_string(),
+                );
+            }
+            eprintln!(
+                "[NIP46] get_public_key attempt {attempt}: {} relay(s) connected",
+                connected_relays.len()
+            );
             match self
                 .send_rpc("get_public_key", vec![], REQUEST_TIMEOUT, false)
                 .await
             {
-                Ok(identity) => break identity,
+                Ok(identity) => {
+                    eprintln!("[NIP46] received get_public_key response");
+                    break identity;
+                }
                 Err(e) => {
                     if !matches!(e, SigningError::Timeout) || attempt >= 4 {
-                        return Err(format!("The signer would not reveal its public key: {e}"));
+                        return Err(format!(
+                            "The signer would not reveal its public key ({e}). Keep Amber open in the foreground with network access and try again."
+                        ));
                     }
                     if live_relays(&connection.relays) {
                         pairing_trace(&format!(
@@ -1755,6 +1963,8 @@ impl Nip46ClientSigner {
         let identity_npub = identity
             .to_bech32()
             .map_err(|e| format!("Could not encode identity npub: {e}"))?;
+        eprintln!("[NIP46] user pubkey: {identity_npub}");
+        eprintln!("[NIP46] updating account state: storing remote profile");
 
         // NOTE: kind-0 metadata (display name / picture / nip05) is fetched
         // AFTER the session flips to Connected, as a background enrichment
@@ -1826,6 +2036,7 @@ impl Nip46ClientSigner {
             ));
         }
         drop(inner);
+        eprintln!("[NIP46] UI connection state updated: Connected ({identity_npub})");
 
         // Background enrichment (post-Connected by design): look up the
         // identity's kind-0 metadata so the profile row carries the real
@@ -2219,6 +2430,35 @@ fn percent_decode(raw: &str) -> Option<String> {
 #[cfg(test)]
 mod raw_request_tests {
     use super::RawRequest;
+    use nostr_sdk::prelude::Keys;
+
+    #[test]
+    fn connect_params_carry_remote_pubkey_first_per_spec() {
+        // NIP-46 ("connect | [<remote-signer-pubkey>, <optional_secret>, …]")
+        // names the REMOTE signer in params[0]. The client key is already the
+        // event author; sending it as params[0] stalled pasted connections
+        // against strict signers with zero feedback.
+        let peer = Keys::generate().public_key();
+        let params = super::Nip46ClientSigner::connect_params(peer, Some("s3cr3t"));
+        assert_eq!(params, vec![peer.to_hex(), "s3cr3t".to_string()]);
+        let params = super::Nip46ClientSigner::connect_params(peer, None);
+        assert_eq!(params, vec![peer.to_hex()]);
+    }
+
+    #[test]
+    fn connect_params_round_trip_through_library_codec() {
+        // Our outbound `connect` must parse under rust-nostr's own
+        // NostrConnectRequest::Connect codec — the same codec strict signers
+        // validate against.
+        use nostr::nips::nip46::{NostrConnectMethod, NostrConnectRequest};
+        let peer = Keys::generate().public_key();
+        let params = super::Nip46ClientSigner::connect_params(peer, Some("s3cr3t"));
+        let req = NostrConnectRequest::from_message(NostrConnectMethod::Connect, params)
+            .expect("library must accept our connect params");
+        assert_eq!(req.method(), NostrConnectMethod::Connect);
+        assert_eq!(req.params()[0], peer.to_hex());
+        assert_eq!(req.params()[1], "s3cr3t");
+    }
 
     #[test]
     fn parses_amber_style_connect_with_object_params() {
diff --git a/tests/nip46_e2e.rs b/tests/nip46_e2e.rs
index a937613..cffd012 100644
--- a/tests/nip46_e2e.rs
+++ b/tests/nip46_e2e.rs
@@ -335,12 +335,18 @@ async fn run_fake_amber(relay_url: String, comms: Keys, identity: Keys, approval
 // ---------------------------------------------------------------------------
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+/// Serializes the e2e tests against each other (see the whole-body `_vault_guard`
+/// below). `await_holding_lock` is allowed here deliberately: the guard is a
+/// test-only serialization lock, never nested, never shared with production
+/// code — holding it across awaits is the entire point.
+#[allow(clippy::await_holding_lock)]
 async fn nip46_client_handshake_and_sign_against_fake_amber() {
     // Isolated vault so the test never touches the real user vault.
-    // XDG_DATA_HOME is process-global and both tests in this binary set it,
-    // so vault setup + App::load are serialized.
+    // XDG_DATA_HOME is process-global and every test in this binary sets it,
+    // so the lock is held for the WHOLE test: data_dir() re-reads the env on
+    // every save, and a setup-only guard lets parallel tests cross-write.
+    let _vault_guard = VAULT_ENV_LOCK.lock().unwrap();
     let app = {
-        let _guard = VAULT_ENV_LOCK.lock().unwrap();
         let tmp = std::env::temp_dir().join(format!("keynectr-e2e-{}", std::process::id()));
         // data_dir() is $XDG_DATA_HOME/keynectr — the isolation vault must
         // live there. Writing it one level too shallow left the app finding
@@ -478,6 +484,230 @@ async fn nip46_client_handshake_and_sign_against_fake_amber() {
     let _ = PublicKey::from_hex; // keep import used across cfg variations
 }
 
+// ---------------------------------------------------------------------------
+// Strict Amber for the bunker:// (paste-URI) flow: validates the `connect`
+// request against NIP-46 instead of acking anything. params[0] MUST be the
+// remote signer's pubkey (the URI authority) — the client's own pubkey there
+// is a spec violation that real signers answer with silence, stalling the
+// handshake with zero feedback. This test FAILS on the old param order and
+// passes on the fixed one.
+// ---------------------------------------------------------------------------
+
+/// Run Amber in strict mode: enforce the NIP-46 `connect` shape, then behave
+/// like the lenient fake (delayed approval ack, real identity, remote sign).
+async fn run_strict_amber(
+    relay_url: String,
+    comms: Keys,
+    identity: Keys,
+    approval_delay: Duration,
+) {
+    let (mut ws, _) = tokio_tungstenite::connect_async(&relay_url)
+        .await
+        .expect("strict amber connect");
+    ws.send(Message::Text(
+        json!(["REQ", "strict-amber", {"kinds": [24133]}])
+            .to_string()
+            .into(),
+    ))
+    .await
+    .unwrap();
+
+    let comms_pub = comms.public_key();
+    let comms_hex = comms_pub.to_hex();
+
+    while let Some(Ok(msg)) = ws.next().await {
+        let Message::Text(text) = msg else { continue };
+        let Ok(arr) = serde_json::from_str::<Vec<Value>>(&text) else {
+            continue;
+        };
+        if arr.first().and_then(|v| v.as_str()) != Some("EVENT") {
+            continue;
+        }
+        let Some(ev) = arr
+            .get(2)
+            .and_then(|v| v.as_object())
+            .and_then(|o| Event::from_json(serde_json::to_string(o).ok()?.as_bytes()).ok())
+        else {
+            continue;
+        };
+        if ev.pubkey == comms_pub {
+            continue;
+        }
+        let Ok(conversation) = ConversationKey::derive(comms.secret_key(), &ev.pubkey) else {
+            continue;
+        };
+        let Some(plain) = nip44_dec(&conversation, &ev.content) else {
+            continue;
+        };
+        let Ok(req) = serde_json::from_str::<Value>(&plain) else {
+            continue;
+        };
+        let Some(method) = req.get("method").and_then(|m| m.as_str()) else {
+            continue;
+        };
+        let id = req
+            .get("id")
+            .and_then(|v| v.as_str())
+            .unwrap_or("")
+            .to_string();
+
+        let response: Value = match method {
+            "connect" => {
+                let first_param = req
+                    .get("params")
+                    .and_then(|p| p.get(0))
+                    .and_then(|v| v.as_str())
+                    .unwrap_or("");
+                if first_param != comms_hex {
+                    json!({"id": id, "error": "connect params must start with the remote signer pubkey"})
+                } else {
+                    tokio::time::sleep(approval_delay).await;
+                    json!({"id": id, "result": "ack"})
+                }
+            }
+            "get_public_key" => json!({"id": id, "result": identity.public_key().to_hex()}),
+            "sign_event" => {
+                let unsigned_json = req["params"].get(0).and_then(|v| v.as_str());
+                match unsigned_json.and_then(|s| serde_json::from_str::<Value>(s).ok()) {
+                    Some(mut v) => {
+                        if v.get("pubkey").is_none() {
+                            v["pubkey"] = json!(identity.public_key().to_hex());
+                        }
+                        match serde_json::from_value::<UnsignedEvent>(v)
+                            .ok()
+                            .and_then(|u| identity.sign_event(u).ok())
+                        {
+                            Some(signed) => {
+                                json!({"id": id, "result": signed.as_json()})
+                            }
+                            None => json!({"id": id, "error": "sign failed"}),
+                        }
+                    }
+                    None => json!({"id": id, "error": "bad params"}),
+                }
+            }
+            other => json!({"id": id, "error": format!("unsupported: {other}")}),
+        };
+
+        let content = nip44_enc(&conversation, &response.to_string());
+        let out = EventBuilder::new(Kind::NostrConnect, content)
+            .tags([Tag::parse(["p", ev.pubkey.to_hex().as_str()]).unwrap()])
+            .finalize(&comms)
+            .unwrap();
+        ws.send(Message::Text(
+            json!([
+                "EVENT",
+                serde_json::from_str::<Value>(&out.as_json()).unwrap()
+            ])
+            .to_string()
+            .into(),
+        ))
+        .await
+        .unwrap();
+    }
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+/// Serializes the e2e tests against each other (see the whole-body `_vault_guard`
+/// below). `await_holding_lock` is allowed here deliberately: the guard is a
+/// test-only serialization lock, never nested, never shared with production
+/// code — holding it across awaits is the entire point.
+#[allow(clippy::await_holding_lock)]
+async fn nip46_bunker_connect_params_match_spec_against_strict_amber() {
+    // Whole-body env lock: data_dir() re-reads XDG_DATA_HOME on every save.
+    let _vault_guard = VAULT_ENV_LOCK.lock().unwrap();
+    let app = {
+        let tmp = std::env::temp_dir().join(format!("keynectr-e2e-strict-{}", std::process::id()));
+        let app_dir = tmp.join("keynectr");
+        std::fs::create_dir_all(&app_dir).unwrap();
+        std::fs::write(
+            app_dir.join("profiles_vault.json"),
+            serde_json::to_string(&Vault::empty()).unwrap(),
+        )
+        .unwrap();
+        std::env::set_var("XDG_DATA_HOME", &tmp);
+        let app = std::sync::Arc::new(Mutex::new(App::load().expect("load app")));
+        assert!(
+            app.try_lock().unwrap().vault.profiles.is_empty(),
+            "e2e vault isolation failed: a non-empty vault was loaded"
+        );
+        app
+    };
+
+    // `comms` is the per-connection key in the bunker:// URI; `identity` is
+    // the REAL signing identity, never in the URI.
+    let comms = Keys::generate();
+    let identity = Keys::generate();
+
+    let relay_url = start_relay().await;
+    tokio::spawn(run_strict_amber(
+        relay_url.clone(),
+        comms.clone(),
+        identity.clone(),
+        Duration::from_millis(200),
+    ));
+
+    let signer = Nip46ClientSigner::new(app.clone());
+
+    // No secret in the URI: the strict signer must still ack a well-formed
+    // connect whose params[0] is its own pubkey.
+    let uri = format!(
+        "bunker://{}?relay={}",
+        comms.public_key().to_hex(),
+        relay_url
+    );
+    let status = signer
+        .connect(&uri, "strict amber".to_string())
+        .await
+        .expect("connect");
+    assert!(!status.connected, "must not be connected before handshake");
+
+    let deadline = tokio::time::Instant::now() + Duration::from_secs(20);
+    loop {
+        let status = signer.status().await;
+        if let Some(err) = &status.error {
+            panic!("strict handshake failed: {err}");
+        }
+        if status.connected {
+            break;
+        }
+        assert!(
+            tokio::time::Instant::now() < deadline,
+            "strict handshake never completed; last status: {:?}",
+            signer.status().await
+        );
+        tokio::time::sleep(Duration::from_millis(100)).await;
+    }
+
+    // Identity must be the REAL key from get_public_key — the actual user
+    // pubkey the account manager stores — never the URI comms key and never
+    // the client's own ephemeral key.
+    let resolved = SignerTrait::get_public_key(&signer)
+        .await
+        .expect("identity resolved");
+    assert_eq!(resolved, identity.public_key());
+    assert_ne!(resolved, comms.public_key());
+
+    let identity_npub = identity.public_key().to_bech32().unwrap();
+    let app_guard = app.lock().await;
+    assert!(
+        app_guard
+            .vault
+            .profiles
+            .iter()
+            .any(|p| p.public_key == identity_npub && p.secret_key.trim().is_empty()),
+        "remote profile row for the real identity must be stored with no secret"
+    );
+    assert_eq!(
+        app_guard.vault.active_profile.as_deref(),
+        Some(identity_npub.as_str()),
+        "the connected account must become the active profile"
+    );
+    drop(app_guard);
+
+    signer.disconnect().await.ok();
+}
+
 // ---------------------------------------------------------------------------
 // QR pairing (client-initiated nostrconnect://): the fake signer plays the
 // scanner role — it reads the pairing token the GUI would render, sends the
@@ -625,6 +855,11 @@ async fn run_fake_scanner(
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+/// Serializes the e2e tests against each other (see the whole-body `_vault_guard`
+/// below). `await_holding_lock` is allowed here deliberately: the guard is a
+/// test-only serialization lock, never nested, never shared with production
+/// code — holding it across awaits is the entire point.
+#[allow(clippy::await_holding_lock)]
 async fn nip46_qr_pairing_handshake_and_sign() {
     run_qr_pairing("request").await;
 }
@@ -633,16 +868,24 @@ async fn nip46_qr_pairing_handshake_and_sign() {
 /// what Amber sends — is a connect *response* (`{"id","result":"<secret>"}`),
 /// not a `connect` request. Pairing must complete on that shape too.
 #[tokio::test(flavor = "multi_thread", worker_threads = 4)]
+/// Serializes the e2e tests against each other (see the whole-body `_vault_guard`
+/// below). `await_holding_lock` is allowed here deliberately: the guard is a
+/// test-only serialization lock, never nested, never shared with production
+/// code — holding it across awaits is the entire point.
+#[allow(clippy::await_holding_lock)]
 async fn nip46_qr_pairing_connect_response_shape() {
     run_qr_pairing("response").await;
 }
 
+/// Whole-body env lock like the test fns above (test-only, never nested).
+#[allow(clippy::await_holding_lock)]
 async fn run_qr_pairing(connect_shape: &'static str) {
     let relay_url = start_relay().await;
 
-    // Isolated vault + pairing relay, serialized with the other e2e test.
+    // Isolated vault + pairing relay; the env lock is held for the whole
+    // helper body (see above) so parallel tests cannot cross-write vaults.
+    let _vault_guard = VAULT_ENV_LOCK.lock().unwrap();
     let app = {
-        let _guard = VAULT_ENV_LOCK.lock().unwrap();
         let tmp = std::env::temp_dir().join(format!(
             "keynectr-e2e-pair-{}-{}",
             std::process::id(),