From ed99ae9722942c37d3f8730c7d5c9f965a924d29 Mon Sep 17 00:00:00 2001
From: Glenn Gore <glenn.g@affinidi.com>
Date: Sat, 29 Aug 2026 21:13:28 +0800
Subject: [PATCH] fix(tsp): say why an inbound frame was not claimed as a reply
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The claim predicate added with the TSP inbound work returned `false` for every
failure mode without recording any of them. On a shared socket that is right in
principle — a frame from another peer is not this request's problem — but it
made the one case that *is* a problem indistinguishable from silence: when the
VTA's reply is declined, the request waits out its full timeout and reports
`timed out awaiting reply frame`, with no record anywhere that a frame arrived
at all, let alone why it was rejected.

That is a regression in diagnosability against the code it replaced, which
threw a typed error naming the sender mismatch or the parse failure. The
predicate is the only thing in the path that knows; if it does not report, by
construction nothing downstream can.

It now records the reason for the most recent decline — unpack failure, sender
mismatch, unparseable payload, or a `threadId` that does not thread to this
request (naming both values, plus the reply's own type and id) — and attaches it
to the error the caller sees when the wait ends. The frame is still declined in
exactly the same cases; only the silence is removed.

Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
---
 packages/core/src/vta/tsp-channel.ts | 60 ++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/packages/core/src/vta/tsp-channel.ts b/packages/core/src/vta/tsp-channel.ts
index 8e3c69b..4f5b146 100644
--- a/packages/core/src/vta/tsp-channel.ts
+++ b/packages/core/src/vta/tsp-channel.ts
@@ -196,6 +196,12 @@ export class TspChannel implements TrustTaskChannel {
     // *proven* to come from that VTA, and (c) threads to this request is our
     // reply. Anything else — most importantly an executor-initiated push
     // landing mid-request — is left for the next waiter or the inbox.
+    // Why the last frame was declined, so a request that times out can say what
+    // it saw instead of only that it waited. Declining silently is what turned
+    // a correlation mismatch into a 30s hang with no reason anywhere — the
+    // predicate has to report, because by design nothing downstream can.
+    let lastDecline: string | undefined;
+
     const claims: TspFrameClaim = async (bytes) => {
       let reply;
       try {
@@ -204,29 +210,59 @@ export class TspChannel implements TrustTaskChannel {
           senderEncryptionKey: this.vta.encryptionPublicKey,
           senderSigningKey: this.vta.signingPublicKey,
         });
-      } catch {
-        // Not unpackable under these keys, so not ours. Not an error: another
-        // waiter's peer, or an inbound from someone else entirely.
+      } catch (err) {
+        // Not unpackable under these keys, so not ours. Expected on a shared
+        // socket — another peer's frame — but recorded, because "nothing I can
+        // read arrived" and "the VTA answered something I did not expect" are
+        // very different problems and they look identical from the timeout.
+        lastDecline = `unpack failed: ${(err as Error).message}`;
+        return false;
+      }
+      if (reply.sender !== this.vta.vid) {
+        lastDecline = `sealed by ${reply.sender}, not the VTA ${this.vta.vid}`;
         return false;
       }
-      if (reply.sender !== this.vta.vid) return false;
-      let doc: { type?: string; payload?: unknown; threadId?: unknown };
+      let doc: { type?: string; id?: unknown; payload?: unknown; threadId?: unknown };
       try {
         doc = JSON.parse(fromUtf8.decode(reply.payload)) as typeof doc;
-      } catch {
+      } catch (err) {
+        lastDecline = `payload not JSON: ${(err as Error).message}`;
         return false;
       }
       // `threadId` on a response is the request's `threadId` or, as here, its
-      // `id` — the same `thid ?? id` rule DIDComm correlates on.
-      if (doc.threadId !== envelope.id) return false;
+      // `id` — the same `thid ?? id` rule DIDComm correlates on, and what the
+      // framework's `respond_with` sets.
+      if (doc.threadId !== envelope.id) {
+        lastDecline =
+          `threadId ${JSON.stringify(doc.threadId)} != request id ` +
+          `${JSON.stringify(envelope.id)} (reply type ${JSON.stringify(doc.type)}, ` +
+          `reply id ${JSON.stringify(doc.id)})`;
+        return false;
+      }
       claimedDoc = doc;
       return true;
     };
 
-    await this.transport.sendAndAwaitReply(packed.bytes, {
-      timeoutMs: opts.timeoutMs ?? this.timeoutMs,
-      claims,
-    });
+    try {
+      await this.transport.sendAndAwaitReply(packed.bytes, {
+        timeoutMs: opts.timeoutMs ?? this.timeoutMs,
+        claims,
+      });
+    } catch (err) {
+      // A timeout here almost always means a frame *did* arrive and was
+      // declined. Say which, on the error the caller actually sees: without it
+      // the only evidence is a silent 30s wait, and the difference between
+      // "the VTA never answered" and "it answered something I did not
+      // recognise" is the whole diagnosis.
+      if (lastDecline) {
+        throw new VtaClientError(
+          (err as VtaClientError).code ?? "e.client.network",
+          `${(err as Error).message} — last inbound frame declined: ${lastDecline}`,
+          { details: { lastDecline } },
+        );
+      }
+      throw err;
+    }
 
     // `claims` returned true, so it parsed the document; the transport cannot
     // resolve without one having claimed. Defensive only.