fix(mcpl): a reference from a server with no address says so when refused

changelog.d/converter-references-bound.fixed.md

@@ -2,7 +2,8 @@
   a context injection or the successful result of an MCPL tool called in
   process is registered with the server that sent it, as the references in
   the result of an MCPL tool call the agent makes are, so `fetch_reference`
-  can fetch it from that server. These were registered with no server, so
+  can fetch it from that server, where the host has an address for it.
+  These were registered with no server, so
   `fetch_reference` refused them, unless a tool result had registered the
   same uri with its server. A reference now keeps every server that
   delivered its uri, and `fetch_reference` dials through the first of them

changelog.d/no-address-server-refusal.fixed.md

@@ -0,0 +1,18 @@
+- Where `fetch_reference` refused a reference from an MCPL server the host
+  doesn't connect to over a network, such as a stdio server, as
+  `unknown server: <id>`, it now says `no address: this host doesn't
+  connect to server <id> over a network, so there's nowhere to fetch it
+  from`. A server the host spawns from a command has no address either,
+  even when its config also holds a `url`, on whose origin its references
+  used to be fetched; nor has a WebSocket server whose `url` the transport
+  won't dial: one that isn't a `ws://` or `wss://` url, or that has a
+  fragment. A WebSocket server whose `url` has a fragment, which the `ws`
+  library refuses to dial, now fails to connect with the transport's own
+  error naming the fragment. `save_recent_image` now gives the reason a
+  `fetch_reference` call naming no `max_bytes` would be refused with before
+  dialing, and points at `fetch_reference` only where that fetch would go
+  ahead. The eager fetch of the references in the result of an MCPL tool
+  call the agent makes no longer spends one of its slots on a reference the
+  fetch would refuse before dialing, such as one from a server whose fetch
+  budget is spent, though it still joins a fetch of a reference already
+  under way.

src/framework.ts

@@ -1,6 +1,6 @@
 import { ToolPresentation, presentationTools, isPresentationTool, renderCatalogue, type PresentationSnapshot } from "./tool-presentation.js";
 import { dirname, join } from 'node:path';
-import { INLINE_WITHHELD_TEXT, REFERENCE_LIMITS, classifyBlock, isInlineContradiction, isReferenceExpired, referenceRegistry, referenceStubOrNull, sanitizeLabel, unfetchableReason } from './mcpl/references.js';
+import { INLINE_WITHHELD_TEXT, REFERENCE_LIMITS, classifyBlock, isInlineContradiction, isReferenceExpired, referenceRegistry, referenceStubOrNull, sanitizeLabel } from './mcpl/references.js';
 import { ReferenceFetcher, DEFAULT_FETCH_MAX_BYTES, EAGER_FETCH_TIMEOUT_MS } from './mcpl/reference-fetcher.js';
 import {
   appendFileSync, closeSync, existsSync, fsyncSync, mkdirSync, openSync, readFileSync, renameSync, unlinkSync,
@@ -481,7 +481,7 @@ import { UsageTracker, persistedUsageProblem, type PersistedUsageState } from '.
 import type { SessionUsageSnapshot, UsageUpdatedEvent } from './usage/types.js';
 import { stderrTailOf, withStderr, type McplConnectionCloseInfo, type McplServerConnection } from './mcpl/server-connection.js';
 import { boundedForLog, describeCause } from './mcpl/errors.js';
-import { assertServerConfigBooleans, isWebSocketTransport } from './mcpl/transport.js';
+import { assertServerConfigBooleans, buildWebSocketUrl, isWebSocketTransport } from './mcpl/transport.js';
 import type {
   McplServerConfig,
   McplHostCapabilities,
@@ -4690,7 +4690,18 @@ export class AgentFramework {
       this.referenceFetcherInstance = new ReferenceFetcher(
         (serverId) => {
           const cfg = this.mcplServerConfigs.get(serverId);
-          if (!cfg?.url) return undefined;
+          if (!cfg) return undefined;
+          // A server's address is the url the host dials it at, as the
+          // transport decides: none for a server the host spawns, as a stdio
+          // server, nor for a WebSocket server whose url the transport won't
+          // dial, which buildWebSocketUrl refuses: one that isn't a ws:// or
+          // wss:// url, or has a fragment.
+          if (!isWebSocketTransport(cfg) || !cfg.url) return 'no-address';
+          try {
+            buildWebSocketUrl(cfg, null);
+          } catch {
+            return 'no-address';
+          }
           return { url: cfg.url, token: cfg.token, autofetch: cfg.autofetch };
         },
         async (fileName, data, mimeType) => {
@@ -17368,11 +17379,12 @@ export class AgentFramework {
       if (result.isError) return mcplToolFailure(result.content);
       // RFC-005 §6.2: register the result's references with this server, so
       // a stub later made of them is bound and fetch_reference can fetch
-      // them. Nothing is fetched here: an in-process caller may never show
-      // them to an agent, and an eager fetch would spend the server's fetch
-      // budget, which the agent's own fetch_reference needs, on bytes nobody
-      // asked for. A failure keeps only its text (mcplToolFailure), so no
-      // stub is made of its references and they aren't registered.
+      // them through this server's address, where it has one. Nothing is
+      // fetched here: an in-process caller may never show them to an agent,
+      // and an eager fetch would spend the server's fetch budget, which the
+      // agent's own fetch_reference needs, on bytes nobody asked for. A
+      // failure keeps only its text (mcplToolFailure), so no stub is made of
+      // its references and they aren't registered.
       await this.autoFetchReferences(result.content, serverId, false)
         .catch((e) => console.error('[rfc005] reference registration error:', (e as Error).message));
       return {
@@ -23017,22 +23029,24 @@ export class AgentFramework {
             };
           }
           if (candidate.kind === 'reference') {
-            // Point at fetch_reference only where it can deliver: not for a
-            // reference this process holds no record of (fetch_reference
-            // looks the id up in the same registry), nor, as the stub does,
-            // for one whose record shows the fetcher would refuse it as
-            // unbound or expired. A saved copy it serves.
+            // Point at fetch_reference only where its fetch would go ahead: not
+            // for a reference this process holds no record of (fetch_reference
+            // looks the id up in the same registry), nor for one the fetcher
+            // would refuse before dialing. Its refusal() gives the reason a
+            // fetch_reference call naming no max_bytes would be refused with,
+            // from the checks its fetch makes, and none where the fetch would
+            // serve a saved copy or join a fetch already under way.
             const record = referenceRegistry.get(candidate.refId);
-            const unfetchable = record ? unfetchableReason(record) : null;
+            const refusal = record ? this.getReferenceFetcher().refusal(record) : null;
             throw new Error(
               `save_recent_image: image at index ${position} is a reference (${candidate.refId}, ${candidate.mediaType}, ` +
               `from ${describeTool(candidate)}) — its bytes are not held here. Nothing written. ` +
               (!record
                 ? 'This process holds no record of it (a restart or an eviction drops records, and a stub quoted ' +
                   "from elsewhere never had one), so fetch_reference can't fetch it either. If its tool can produce " +
                   'it again, re-run that tool for a fresh reference.'
-                : unfetchable
-                  ? `It can't be fetched either — ${unfetchable}.`
+                : refusal
+                  ? `It can't be fetched either — ${refusal.reason}.`
                   : 'Fetch it with fetch_reference and save the materialized file instead.'),
             );
           }

src/mcpl/convert-block.ts

@@ -17,8 +17,8 @@ import type { McplContentBlock } from './types.js';
  * shows as NON_STRING_TEXT_PLACEHOLDER. `provenance` is what a reference stub
  * says the reference came on, such as 'attachment on push event'. `serverId`
  * is the server that sent the content: a reference is registered with it, as
- * a tool result's references are, so fetch_reference can fetch it from that
- * server.
+ * a tool result's references are, so fetch_reference can fetch it through that
+ * server's address, where it has one.
  */
 export function convertBlock(block: McplContentBlock, provenance: string, serverId: string): ContentBlock {
   switch (block.type) {

src/mcpl/reference-fetcher.ts

@@ -30,7 +30,7 @@
  */
 import { createHash } from 'node:crypto';
 import type { ReferenceRecord } from './references.js';
-import { EXPIRY_REASON, isReferenceExpired, referenceExpiry, REFERENCE_LIMITS, sanitizeLabel, UNBOUND_REASON } from './references.js';
+import { EXPIRY_REASON, noAddressReason, referenceExpiry, REFERENCE_LIMITS, sanitizeLabel, UNBOUND_REASON } from './references.js';
 
 /** Per-fetch hard ceiling on actual bytes (overridable per server / call). */
 export const DEFAULT_FETCH_MAX_BYTES = 256 * 1024 * 1024;
@@ -49,13 +49,35 @@ const BUDGET_LASTS = "the budget doesn't renew on its own, so calling again won'
 const EAGER_FETCH_TIMEOUT_MS = 10_000;
 
 export interface ResolvedServerBinding {
-  /** The url the connection was dialed to (ws/wss/http/https). */
+  /** The url the host dials the server at. The framework's resolver gives only
+   *  a ws:// or wss:// url its WebSocket transport dials; the origin check
+   *  reads ws as http and wss as https. */
   url: string;
   /** Connection credential, applied host-side as Authorization: Bearer. */
   token?: string;
   autofetch?: { maxBytes?: number; maxTotalBytes?: number };
 }
 
+/** What the host knows of a server a reference came from: its binding, or
+ *  'no-address' for one the host doesn't reach over a network, such as a
+ *  stdio server, which has no address to dial. A server the host doesn't
+ *  know resolves to undefined. */
+export type ServerResolution = ResolvedServerBinding | 'no-address';
+
+/** Why the fetcher would refuse a reference without dialing. `ofReference`
+ *  marks one of the reference's own phrases that its stub shares (unbound,
+ *  expired), which fetch_reference words as "reference <reason>"; it gives
+ *  the others as they are. */
+export interface Refusal {
+  reason: string;
+  ofReference: boolean;
+}
+
+/** fetch_reference's words for a refusal. */
+function refusalText(refusal: Refusal): string {
+  return refusal.ofReference ? `reference ${refusal.reason}` : refusal.reason;
+}
+
 export interface FetchOutcome {
   ok: boolean;
   /** Mount-prefixed workspace path of the saved payload (on success). */
@@ -118,41 +140,103 @@ export class ReferenceFetcher {
   private inFlight = new Map<string, Promise<FetchOutcome>>();
 
   constructor(
-    private readonly resolveServer: (serverId: string) => ResolvedServerBinding | undefined,
+    private readonly resolveServer: (serverId: string) => ServerResolution | undefined,
     /** Persist the payload; returns a mount-prefixed path or null when no
      *  writable workspace exists. File name is host-generated (§7.3). */
     private readonly saveFile: (fileName: string, data: Buffer, mimeType: string) => Promise<string | null>,
   ) {}
 
-  /** Would the eager (autofetch) default fetch this record? Not one the
-   *  fetcher refuses as expired, which would only take an eager slot. It
-   *  takes the eager limit of the server the fetch would dial. */
+  /** Would the eager (autofetch) default fetch this record? Not one the fetch
+   *  would refuse before dialing, asked as the eager fetch asks it, with the
+   *  default ceiling, so such a record takes no eager slot; and only one whose
+   *  claimed size is within the eager limit of the server it dials. For one
+   *  whose fetch is already under way, which the eager fetch would join as
+   *  fetch() does, only the server and its eager limit enter: the expiry,
+   *  the budget and the ceiling were checked when that fetch began. */
   isEagerEligible(record: ReferenceRecord): boolean {
     if (record.fetchedPath) return false;
-    if (isReferenceExpired(record.testimony)) return false;
-    const server = this.serverFor(record);
-    if ('refusal' in server) return false;
-    const cap = server.binding.autofetch?.maxBytes ?? DEFAULT_EAGER_FETCH_MAX_BYTES;
+    const check = this.inFlight.has(record.refId) ? this.serverFor(record) : this.beforeDial(record);
+    if ('refusal' in check) return false;
+    const cap = check.binding.autofetch?.maxBytes ?? DEFAULT_EAGER_FETCH_MAX_BYTES;
     return record.testimony.sizeBytes !== undefined && record.testimony.sizeBytes <= cap;
   }
 
   /** The server to dial for a record (§6.2): the first, in the order they
    *  delivered it, whose address is on the uri's origin. When none is, the
-   *  first server's refusal (unknown to this host, or the origin check's),
-   *  and for a record no server delivered, that it's unbound. */
-  private serverFor(record: ReferenceRecord): { serverId: string; binding: ResolvedServerBinding } | { refusal: string } {
-    let refusal: string | undefined;
+   *  first server's refusal (unknown to this host, with no address to dial,
+   *  or the origin check's), and for a record no server delivered, that it's
+   *  unbound. */
+  private serverFor(record: ReferenceRecord): { serverId: string; binding: ResolvedServerBinding } | { refusal: Refusal } {
+    let refusal: Refusal | undefined;
     for (const serverId of record.serverIds) {
       const binding = this.resolveServer(serverId);
       if (!binding) {
-        refusal ??= `unknown server: ${serverId}`;
+        refusal ??= { reason: `unknown server: ${serverId}`, ofReference: false };
+        continue;
+      }
+      if (binding === 'no-address') {
+        refusal ??= { reason: noAddressReason(serverId), ofReference: false };
         continue;
       }
       const offOrigin = this.originCheck(record, binding);
       if (offOrigin === null) return { serverId, binding };
-      refusal ??= offOrigin;
+      refusal ??= { reason: offOrigin, ofReference: false };
     }
-    return { refusal: refusal ?? `reference ${UNBOUND_REASON}` };
+    return { refusal: refusal ?? { reason: UNBOUND_REASON, ofReference: true } };
+  }
+
+  /** Every check the fetch makes before it dials, in its order: the server
+   *  (none, unknown, no address, off the uri's origin), the expiry, the
+   *  server's spent budget, a claimed size over the ceiling. For a record
+   *  with no saved copy and no fetch under way, the fetch, `refusal()` and
+   *  the eager check all ask this, so what save_recent_image says is the
+   *  reason the fetch would refuse with, and no eager slot goes to a fetch
+   *  that would be refused. */
+  private beforeDial(record: ReferenceRecord, opts?: { maxBytes?: number }):
+    { refusal: Refusal } | { serverId: string; binding: ResolvedServerBinding; ceiling: number; budget: number } {
+    const server = this.serverFor(record);
+    if ('refusal' in server) return server;
+    const { serverId, binding } = server;
+    const t = record.testimony;
+    const expiry = referenceExpiry(t);
+    if (expiry) return { refusal: { reason: EXPIRY_REASON[expiry], ofReference: true } };
+
+    const ceiling = opts?.maxBytes ?? DEFAULT_FETCH_MAX_BYTES;
+    // The budget is checked before the ceiling, here and while reading: a
+    // larger max_bytes lifts the ceiling, and nothing the caller does lifts
+    // the budget, so a fetch past both is told about the budget.
+    const budget = binding.autofetch?.maxTotalBytes ?? DEFAULT_MAX_TOTAL_BYTES;
+    const used = this.totals.get(serverId) ?? 0;
+    if (used >= budget) {
+      return { refusal: { reason: `per-server fetch budget exhausted (${used}/${budget} bytes); ${BUDGET_LASTS}`, ofReference: false } };
+    }
+    // Vector 4: a claim already over the ceiling refuses without dialing. One
+    // that's also over what's left of the budget is told about the budget,
+    // which no max_bytes lifts. A claim over what's left but within the
+    // ceiling still dials, and the budget stops it at its first chunk past.
+    if (t.sizeBytes !== undefined && t.sizeBytes > ceiling) {
+      if (t.sizeBytes > budget - used) {
+        return {
+          refusal: {
+            reason: `claimed ${t.sizeBytes} bytes exceeds what's left of the per-server fetch budget (${used}/${budget} bytes); ${BUDGET_LASTS}`,
+            ofReference: false,
+          },
+        };
+      }
+      return { refusal: { reason: `claimed ${t.sizeBytes} bytes exceeds fetch ceiling ${ceiling}`, ofReference: false } };
+    }
+    return { serverId, binding, ceiling, budget };
+  }
+
+  /** Why a fetch of this record would be refused before dialing, with the
+   *  default ceiling, which a fetch_reference call naming no max_bytes has.
+   *  Null where the fetch would serve a saved copy, join a fetch of it
+   *  already under way, or dial (where it can still fail), the same three
+   *  steps fetch() takes. */
+  refusal(record: ReferenceRecord): Refusal | null {
+    if (record.fetchedPath || this.inFlight.has(record.refId)) return null;
+    const check = this.beforeDial(record);
+    return 'refusal' in check ? check.refusal : null;
   }
 
   /** §6.1/§7.1 policy check; returns a refusal reason or null. */
@@ -188,34 +272,9 @@ export class ReferenceFetcher {
 
   private async doFetch(record: ReferenceRecord, opts?: { maxBytes?: number; timeoutMs?: number }): Promise<FetchOutcome> {
     const t = record.testimony;
-    const server = this.serverFor(record);
-    if ('refusal' in server) return { ok: false, error: server.refusal };
-    const { serverId, binding } = server;
-    const expiry = referenceExpiry(t);
-    if (expiry) return { ok: false, error: `reference ${EXPIRY_REASON[expiry]}` };
-
-    const ceiling = opts?.maxBytes ?? DEFAULT_FETCH_MAX_BYTES;
-    // The budget is checked before the ceiling, here and while reading: a
-    // larger max_bytes lifts the ceiling, and nothing the caller does lifts
-    // the budget, so a fetch past both is told about the budget.
-    const budget = binding.autofetch?.maxTotalBytes ?? DEFAULT_MAX_TOTAL_BYTES;
-    const used = this.totals.get(serverId) ?? 0;
-    if (used >= budget) {
-      return { ok: false, error: `per-server fetch budget exhausted (${used}/${budget} bytes); ${BUDGET_LASTS}` };
-    }
-    // Vector 4: a claim already over the ceiling refuses without dialing. One
-    // that's also over what's left of the budget is told about the budget,
-    // which no max_bytes lifts. A claim over what's left but within the
-    // ceiling still dials, and the budget stops it at its first chunk past.
-    if (t.sizeBytes !== undefined && t.sizeBytes > ceiling) {
-      if (t.sizeBytes > budget - used) {
-        return {
-          ok: false,
-          error: `claimed ${t.sizeBytes} bytes exceeds what's left of the per-server fetch budget (${used}/${budget} bytes); ${BUDGET_LASTS}`,
-        };
-      }
-      return { ok: false, error: `claimed ${t.sizeBytes} bytes exceeds fetch ceiling ${ceiling}` };
-    }
+    const check = this.beforeDial(record, opts);
+    if ('refusal' in check) return { ok: false, error: refusalText(check.refusal) };
+    const { serverId, binding, ceiling, budget } = check;
 
     const controller = new AbortController();
     const timer = setTimeout(() => controller.abort(), opts?.timeoutMs ?? FETCH_TIMEOUT_MS);