diff --git a/changelog.d/fetch-reference-credentials.fixed.md b/changelog.d/fetch-reference-credentials.fixed.md new file mode 100644 index 000000000..3c8bb2ec9 --- /dev/null +++ b/changelog.d/fetch-reference-credentials.fixed.md @@ -0,0 +1 @@ +- A reference whose address carries a user name or password is refused before anything is dialed, in words that don't repeat them. `fetch` refused such an address before connecting, with a message holding the whole address, password included, which `fetch_reference` gave the agent as its error, and the host's log carried it when the eager fetch tried it. diff --git a/changelog.d/fetch-reference-words.changed.md b/changelog.d/fetch-reference-words.changed.md new file mode 100644 index 000000000..fbad821d9 --- /dev/null +++ b/changelog.d/fetch-reference-words.changed.md @@ -0,0 +1,6 @@ +- `fetch_reference`'s failures now say what happened to the reference and what calling again can do: `fetch_reference couldn't fetch "": . `, where they gave the fetcher's reason alone, such as `HTTP 404`, `fetch timed out or aborted`, `claimed bytes exceeds fetch ceiling ` or `third-party origin ( != dialed ); not fetched by this host`. Where `save_recent_image` says a reference can't be fetched, it gives the same two. + - **What happened** names the server and what it answered (`the "" server answered HTTP 503`), the limit a fetch met (`it claims bytes, over the bytes this call allows`, `the download had reached bytes, past …`), what kept a fetch from dialing (`it points to , not to the "" server's own address (), and this host fetches only from the server itself`), or that what arrived didn't match the reference's digest or couldn't be stored. The eager fetch's and a script's fetch's log lines read the same words. + - **What calling again can do**, such as trying again later (after a timeout, 408, 409, 423, 425, 429, or a 5xx other than 501 and 505, and after a failed request, with whoever runs the host to check its connection to the server if it fails the same way), waiting for a fresh reference to it (an expired or unbound reference, and the other 4xx), passing a larger `max_bytes` (naming the size it needs, and no more than the server's fetch budget has left), asking whoever runs the host (who can check its credentials for the server after a 401 or 403, raise or renew a spent budget, or add back a server it no longer has), or nothing, as for an address on another origin, a server this host doesn't connect to over a network, or a claim past the 256 MiB this host fetches at most, which no `max_bytes` or setting lifts. A claim past those 256 MiB is refused for them first, and a download past them is stopped for them first, whatever the server's budget has left. + - **The budget's texts** read `this host's fetch budget for the "" server is used up ( of bytes)` and, partway, `this host's fetch budget for the "" server ran out during the download ( of bytes), so it was stopped and nothing was kept`, where they read `per-server fetch budget exhausted (/ bytes)`, the one partway after `aborted: `. + - **A `ws:` or `wss:` reference** on its server's own origin is refused for its scheme before anything is dialed, where `fetch` refused it before connecting and the agent read `fetch failed`. A server whose address doesn't parse is refused, saying so, and that whoever runs the host can correct it. + - **The host's log** says why a fetched payload couldn't be stored: no workspace, no writable mount, or the write's error. A save that throws is that failure too, logged, where its message reached the agent as the fetch's own error. diff --git a/changelog.d/no-address-server-refusal.fixed.md b/changelog.d/no-address-server-refusal.fixed.md index d78b3e834..bfbea0ee9 100644 --- a/changelog.d/no-address-server-refusal.fixed.md +++ b/changelog.d/no-address-server-refusal.fixed.md @@ -1,8 +1,8 @@ - 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: `, it now says `no address: this host doesn't - connect to server over a network, so there's nowhere to fetch it - from`. A server the host spawns from a command has no address either, + `unknown server: `, it now says `this host doesn't connect to the + "" server 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 diff --git a/src/framework.ts b/src/framework.ts index cb39c3006..75566c83a 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -4920,11 +4920,19 @@ export class AgentFramework { } return { url: cfg.url, token: cfg.token, autofetch: cfg.autofetch }; }, + // A null tells the agent only that the payload couldn't be stored; + // why, which can name the host's paths, goes to the log. async (fileName, data, mimeType) => { const workspace = this.getWorkspaceModule(); - if (!workspace) return null; + if (!workspace) { + console.error(`[refs] couldn't store ${fileName}: this host has no workspace`); + return null; + } const mount = this.firstWritableMountName(workspace); - if (!mount) return null; + if (!mount) { + console.error(`[refs] couldn't store ${fileName}: no workspace mount is writable`); + return null; + } const path = `${mount}/refs/${fileName}`; const res = await workspace.writeBinary(path, data, mimeType); if (res.success) return path; @@ -4934,6 +4942,7 @@ export class AgentFramework { console.error(`[refs] ${path} is in the workspace's Chronicle tree, but the rest of its write failed: ${res.error ?? 'unknown error'}`); return path; } + console.error(`[refs] couldn't store ${path}: ${res.error ?? 'unknown error'}`); return null; }, ); @@ -23684,7 +23693,10 @@ export class AgentFramework { result = outcome.ok ? { success: true, data: JSON.stringify({ path: outcome.path, bytes: outcome.bytes, mime_type: outcome.mimeType, digest_verified: outcome.digestVerified ?? 'no digest claimed' }) } - : { success: false, isError: true, error: outcome.error }; + // The fetcher says what happened as a clause about the reference, + // and what calling again can do as a sentence of its own. + : { success: false, isError: true, + error: `fetch_reference couldn't fetch "${refId}": ${outcome.error}. ${outcome.remedy}` }; } this.emitTrace({ type: result.isError ? 'tool:failed' : 'tool:completed', @@ -23937,7 +23949,7 @@ export class AgentFramework { "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.' : refusal - ? `It can't be fetched either — ${refusal.reason}.` + ? `It can't be fetched either — ${refusal.error}. ${refusal.remedy}` : 'Fetch it with fetch_reference and save the materialized file instead.'), ); } diff --git a/src/mcpl/reference-fetcher.ts b/src/mcpl/reference-fetcher.ts index 3d9a9261e..03a7257b1 100644 --- a/src/mcpl/reference-fetcher.ts +++ b/src/mcpl/reference-fetcher.ts @@ -8,8 +8,10 @@ * not fetch third-party origins at all (deferred with declared reference * origins); the connection credential is applied as an Authorization * header by this code alone and never surfaces to model, script, or URI. - * - **scheme allowlist (§7.1, vector 11):** https; http only when the dialed - * origin itself is plaintext (a ws:// server references its own origin). + * - **scheme allowlist (§7.1, vector 11):** the uri's own scheme is http: or + * https:, and since origins compare with their schemes, http only from a + * server dialed in plaintext (ws://), https only from one dialed over TLS + * (wss://). * - **no redirect traversal (§7.2, vector 12):** any 3xx refuses. * - **actual-byte ceiling (§7.3 list, vector 10):** streamed with abort the * moment real octets exceed the ceiling, regardless of claimed sizeBytes. @@ -30,9 +32,11 @@ */ import { createHash } from 'node:crypto'; import type { ReferenceRecord } from './references.js'; +import { boundedForLog, describeCause } from './errors.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). */ +/** Per-fetch hard ceiling on actual bytes. A call can lower it for itself + * (fetch_reference's max_bytes, which it clamps to this); nothing raises it. */ export const DEFAULT_FETCH_MAX_BYTES = 256 * 1024 * 1024; /** Eager (autofetch) threshold: at most what could have shipped inline. */ export const DEFAULT_EAGER_FETCH_MAX_BYTES = 256 * 1024; @@ -41,13 +45,35 @@ export const DEFAULT_EAGER_FETCH_MAX_BYTES = 256 * 1024; * kept. */ export const DEFAULT_MAX_TOTAL_BYTES = 1024 * 1024 * 1024; const FETCH_TIMEOUT_MS = 60_000; -/** What a refusal by the per-server budget adds. The total lives as long as - * this fetcher, one per framework, so only someone running the host renews - * it, by starting a new one, or raises the budget (the server's config's - * autofetch.maxTotalBytes, which each fetch reads afresh). */ -const BUDGET_LASTS = "the budget doesn't renew on its own, so calling again won't change that unless whoever runs this host raises or renews it"; const EAGER_FETCH_TIMEOUT_MS = 10_000; +// What calling again can do, as each failure's remedy says it. The fetcher +// doesn't know where a reference came from (a tool result, a push, a channel +// message), so none names a tool, and fetch_reference's max_bytes is named +// only where a larger one can fetch it. +const NOTHING_TO_DO = "Calling again won't change that."; +/** The registry keys a record by its uri, so a fresh reference to the same + * uri arrives as this very id and refreshes its record. */ +const FRESH_REFERENCE = "Once a fresh reference to it arrives, fetching may work; until then, fetching it again won't."; +const TRY_LATER = 'Trying again later may work.'; +/** A request that failed before an answer: a refused or reset connection + * can clear, and a cause that repeats on every try (a port fetch refuses, a + * name that doesn't resolve, a certificate this host doesn't trust) is + * whoever runs the host's to change, so the remedy names both. */ +const REQUEST_FAILED = "Trying again later may work; if it fails the same way, whoever runs this host can check its connection to the server."; +const CREDENTIALS = "Calling again won't change that; whoever runs this host can check its credentials for the server."; +const FRESH_OR_CREDENTIALS = "Once a fresh reference to it arrives, fetching may work; if that's refused too, whoever runs this host can check its credentials for the server."; +/** The total lives as long as this fetcher, one per framework, so only + * someone running the host renews it, by starting a new one, or raises the + * budget (the server's config's autofetch.maxTotalBytes, which each fetch + * reads afresh). */ +const BUDGET = "The budget doesn't renew on its own, so calling again won't change that unless whoever runs this host raises or renews it."; +/** The host's own ceiling: fetch_reference clamps max_bytes to it, and no + * config sets another. */ +const HOST_CEILING = "No max_bytes or setting lifts that limit, so calling again won't change it."; +/** The resolver reads the host's server configs as they are at each fetch. */ +const SERVER_BACK = "Calling again won't change that unless whoever runs this host adds that server back."; + export interface ResolvedServerBinding { /** 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 @@ -64,28 +90,75 @@ export interface ResolvedServerBinding { * 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 "; it gives - * the others as they are. */ +/** Why a fetch failed, or would be refused before dialing. */ export interface Refusal { - reason: string; - ofReference: boolean; + /** What happened, as a clause that completes "couldn't fetch : ", + * whatever it's about: fetch_reference's sentence, save_recent_image's, + * and the eager and script-demand fetches' log lines read it. */ + error: string; + /** What calling again can do, as a sentence. fetch_reference and + * save_recent_image show it; the eager and script-demand fetches log the + * error alone. */ + remedy: string; } -/** fetch_reference's words for a refusal. */ -function refusalText(refusal: Refusal): string { - return refusal.ofReference ? `reference ${refusal.reason}` : refusal.reason; +/** A fetch's result: the saved copy, or why it failed, which every failure + * says with both a clause and a remedy. The fields the other side lacks are + * declared undefined, so a caller can read them before checking ok. */ +export type FetchOutcome = + | { + ok: true; + /** Mount-prefixed workspace path of the saved payload. */ + path: string; + bytes?: number; + mimeType?: string; + digestVerified?: boolean; + error?: undefined; + remedy?: undefined; + } + | ({ ok: false; path?: undefined; bytes?: undefined; mimeType?: undefined; digestVerified?: undefined } & Refusal); + +const refused = (error: string, remedy: string): { refusal: Refusal } => ({ refusal: { error, remedy } }); +const fail = (error: string, remedy: string): FetchOutcome => ({ ok: false, error, remedy }); + +/** What calling again can do about a server's HTTP status. A later try can + * work after a timeout, a conflict or lock the server's state can clear, a + * request it calls too early, a rate limit or a server's passing trouble, + * though not after 501 or 505, which say the server doesn't support the + * request. A 401 is about this host's credentials for the server, which a + * fresh reference doesn't change, and a 403 can be either those or this + * reference. The rest of 4xx is about this reference, which a fresh one may + * not share. */ +function statusRemedy(status: number): string { + if (status === 501 || status === 505) return NOTHING_TO_DO; + if (status === 408 || status === 409 || status === 423 || status === 425 || status === 429 || status >= 500) return TRY_LATER; + if (status === 401) return CREDENTIALS; + if (status === 403) return FRESH_OR_CREDENTIALS; + return FRESH_REFERENCE; } -export interface FetchOutcome { - ok: boolean; - /** Mount-prefixed workspace path of the saved payload (on success). */ - path?: string; - bytes?: number; - mimeType?: string; - digestVerified?: boolean; - error?: string; +/** What a download stopped at a call's own ceiling, below the host's, can do. + * Its whole size is unknown, only that it's past what arrived, and a fetch + * again reads it from the start, within the host's ceiling and what the + * server's budget has left after this one's bytes. The read loop stops a + * download past the host's ceiling before it asks, so what arrived is within + * it, and only the budget can be what binds. */ +function largerMaxBytes(received: number, left: number, server: string): string { + const most = Math.min(DEFAULT_FETCH_MAX_BYTES, left); + if (received <= most) return `It's at least ${received} bytes; a larger max_bytes, up to ${most}, may fetch it.`; + return `It's at least ${received} bytes, more than the ${left} bytes left of this host's fetch budget for ${server}. ${BUDGET}`; +} + +/** What a thrown fetch says went wrong. fetch's own "fetch failed" says only + * that, so its cause's words come first. A cause from trying each of a + * host's addresses (an AggregateError) has no words of its own, so its + * attempts' stand in. */ +function whyFetchFailed(e: unknown): string { + const source = (e as { cause?: unknown } | null | undefined)?.cause ?? e; + const own = describeCause(source); + if (own !== '') return own; + const attempts = (source as { errors?: unknown } | null | undefined)?.errors; + return Array.isArray(attempts) && attempts.length > 0 ? attempts.map(describeCause).join('; ') : describeCause(e); } /** Magic-byte sniff for the formats this pipeline actually moves. The @@ -118,6 +191,15 @@ const EXT_BY_MIME = new Map(Object.entries({ 'application/pdf': 'pdf', 'application/zip': 'zip', })); +/** A server's address as it's configured, its scheme and host, for a + * clause: never its userinfo, path or query. URL's own origin is 'null' for + * a scheme that isn't special, which normalizedOrigin accepts. The caller + * has already parsed it. */ +function configuredOrigin(url: string): string { + const u = new URL(url); + return `${u.protocol}//${u.host}`; +} + /** ws→http / wss→https with default ports elided, for origin comparison. */ export function normalizedOrigin(url: string): string | null { try { @@ -167,31 +249,35 @@ export class ReferenceFetcher { * 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; + let refusal: { refusal: Refusal } | undefined; for (const serverId of record.serverIds) { const binding = this.resolveServer(serverId); if (!binding) { - refusal ??= { reason: `unknown server: ${serverId}`, ofReference: false }; + refusal ??= refused(`this host no longer has the "${serverId}" server it came from`, SERVER_BACK); continue; } if (binding === 'no-address') { - refusal ??= { reason: noAddressReason(serverId), ofReference: false }; + refusal ??= refused(noAddressReason(serverId), NOTHING_TO_DO); continue; } - const offOrigin = this.originCheck(record, binding); + const offOrigin = this.originCheck(record, binding, serverId); if (offOrigin === null) return { serverId, binding }; - refusal ??= { reason: offOrigin, ofReference: false }; + refusal ??= offOrigin; } - return { refusal: refusal ?? { reason: UNBOUND_REASON, ofReference: true } }; + // A later delivery of the same uri by a server binds this record. + return refusal ?? refused(`it's ${UNBOUND_REASON}`, FRESH_REFERENCE); } /** 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. */ + * (none, unknown, no address, an address of its own or the reference's + * that doesn't parse, a reference address carrying a user name or + * password, a scheme other than http: or https:, off the uri's origin), + * the expiry, a claimed size over the host's own ceiling, the server's + * spent budget, a claimed size over a call's 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); @@ -199,16 +285,21 @@ export class ReferenceFetcher { const { serverId, binding } = server; const t = record.testimony; const expiry = referenceExpiry(t); - if (expiry) return { refusal: { reason: EXPIRY_REASON[expiry], ofReference: true } }; + if (expiry) return refused(`the reference is ${EXPIRY_REASON[expiry]}`, FRESH_REFERENCE); 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 + // A claim past the host's own ceiling is refused first: nothing lifts + // it, not even a budget whoever runs the host raises. + if (t.sizeBytes !== undefined && t.sizeBytes > DEFAULT_FETCH_MAX_BYTES) { + return refused(`it claims ${t.sizeBytes} bytes, over the ${DEFAULT_FETCH_MAX_BYTES} bytes this host fetches at most`, HOST_CEILING); + } + // The budget is checked before a call's ceiling, here and while reading: + // a larger max_bytes lifts that 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 } }; + return refused(`this host's fetch budget for the "${serverId}" server is used up (${used} of ${budget} bytes)`, BUDGET); } // 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, @@ -216,14 +307,15 @@ export class ReferenceFetcher { // 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 refused( + `it claims ${t.sizeBytes} bytes, more than the ${budget - used} bytes left of this host's fetch budget for the "${serverId}" server`, + BUDGET, + ); } - return { refusal: { reason: `claimed ${t.sizeBytes} bytes exceeds fetch ceiling ${ceiling}`, ofReference: false } }; + // Within what's left of the budget and the host's ceiling, so a + // ceiling of the call's own is what a larger max_bytes lifts. + return refused(`it claims ${t.sizeBytes} bytes, over the ${ceiling} bytes this call allows`, + `Pass max_bytes of at least ${t.sizeBytes} to fetch it.`); } return { serverId, binding, ceiling, budget }; } @@ -239,19 +331,39 @@ export class ReferenceFetcher { return 'refusal' in check ? check.refusal : null; } - /** §6.1/§7.1 policy check; returns a refusal reason or null. */ - private originCheck(record: ReferenceRecord, binding: ResolvedServerBinding): string | null { + /** §6.1/§7.1 policy check; returns a refusal or null. */ + private originCheck(record: ReferenceRecord, binding: ResolvedServerBinding, serverId: string): { refusal: Refusal } | null { const refOrigin = normalizedOrigin(record.testimony.uri); + if (!refOrigin) return refused("its address doesn't parse as a URL", NOTHING_TO_DO); + // A server's address that doesn't parse leaves nothing to dial. The + // framework's resolver gives only an address its transport dials, so this + // guards other resolvers. const dialedOrigin = normalizedOrigin(binding.url); - if (!refOrigin) return 'reference uri does not parse'; - if (!dialedOrigin) return 'server url does not parse'; - const scheme = refOrigin.split('//')[0]; - if (scheme !== 'https:' && scheme !== 'http:') return `scheme not allowed: ${scheme}`; - if (refOrigin !== dialedOrigin) { - return `third-party origin (${refOrigin} != dialed ${dialedOrigin}); not fetched by this host`; + if (!dialedOrigin) { + return refused(`the "${serverId}" server's address doesn't parse as a URL, so there's nowhere to fetch it from`, + "Calling again won't change that unless whoever runs this host corrects the server's address."); + } + const refUrl = new URL(record.testimony.uri); + // A user name or password in the uri is a credential embedded in it, + // which RFC-005 §6.1 forbids a server to mint and lets a host refuse + // (vector 6). This host neither sends nor repeats it: fetch refuses such + // a url with a message that holds it whole. + if (refUrl.username !== '' || refUrl.password !== '') { + return refused("its address carries a user name or password, which this host doesn't send", NOTHING_TO_DO); } - if (scheme === 'http:' && dialedOrigin.startsWith('https:')) { - return 'plaintext reference from a secure origin'; + // The uri's own scheme, not its origin's: an origin reads ws: as http: + // and wss: as https:, and fetch refuses a ws: or wss: uri ('unknown + // scheme') before connecting, which would read as a failed request a + // later try can mend. + const scheme = refUrl.protocol; + if (scheme !== 'https:' && scheme !== 'http:') { + return refused(`its address uses ${scheme}, and this host fetches only http: and https: addresses`, NOTHING_TO_DO); + } + // Origins compare with their schemes, so this also refuses an http: + // reference from a server dialed over TLS (wss://). + if (refOrigin !== dialedOrigin) { + return refused(`it points to ${refOrigin}, not to the "${serverId}" server's own address (${configuredOrigin(binding.url)}), ` + + 'and this host fetches only from the server itself', NOTHING_TO_DO); } return null; } @@ -273,20 +385,22 @@ export class ReferenceFetcher { private async doFetch(record: ReferenceRecord, opts?: { maxBytes?: number; timeoutMs?: number }): Promise { const t = record.testimony; const check = this.beforeDial(record, opts); - if ('refusal' in check) return { ok: false, error: refusalText(check.refusal) }; + if ('refusal' in check) return { ok: false, ...check.refusal }; const { serverId, binding, ceiling, budget } = check; + const server = `the "${serverId}" server`; + const timeoutMs = opts?.timeoutMs ?? FETCH_TIMEOUT_MS; const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), opts?.timeoutMs ?? FETCH_TIMEOUT_MS); + const timer = setTimeout(() => controller.abort(), timeoutMs); try { const headers: Record = {}; if (binding.token) headers.authorization = `Bearer ${binding.token}`; const resp = await fetch(t.uri, { headers, redirect: 'manual', signal: controller.signal }); if (resp.status >= 300 && resp.status < 400) { - return { ok: false, error: `redirect refused (HTTP ${resp.status}) — no redirect traversal` }; + return fail(`${server} answered HTTP ${resp.status}, a redirect, and this host doesn't follow redirects`, NOTHING_TO_DO); } - if (!resp.ok) return { ok: false, error: `HTTP ${resp.status}` }; - if (!resp.body) return { ok: false, error: 'no response body' }; + if (!resp.ok) return fail(`${server} answered HTTP ${resp.status}`, statusRemedy(resp.status)); + if (!resp.body) return fail(`${server} answered without any content`, FRESH_REFERENCE); const hash = createHash('sha256'); const chunks: Buffer[] = []; @@ -300,13 +414,24 @@ export class ReferenceFetcher { // this server read the same total, so none spends what another took. const total = (this.totals.get(serverId) ?? 0) + value.byteLength; this.totals.set(serverId, total); + // The limit nothing lifts first, as before dialing: the host's + // ceiling, then the budget, then a call's own ceiling. + if (received > DEFAULT_FETCH_MAX_BYTES) { + controller.abort(); + return fail(`the download had reached ${received} bytes, past the ${DEFAULT_FETCH_MAX_BYTES} bytes this host fetches at most, ` + + 'so it was stopped and nothing was kept', HOST_CEILING); + } if (total > budget) { controller.abort(); - return { ok: false, error: `aborted: per-server fetch budget exhausted (${total}/${budget} bytes); ${BUDGET_LASTS}` }; + // The server's running total, as the next fetch's refusal gives it: + // other fetches through the server may have spent part of it. + return fail(`this host's fetch budget for ${server} ran out during the download (${total} of ${budget} bytes), ` + + 'so it was stopped and nothing was kept', BUDGET); } if (received > ceiling) { controller.abort(); - return { ok: false, error: `aborted: actual bytes exceeded ceiling at ${received}` }; + return fail(`the download had reached ${received} bytes, past the ${ceiling} bytes this call allows, ` + + 'so it was stopped and nothing was kept', largerMaxBytes(received, budget - total, server)); } const buf = Buffer.from(value); hash.update(buf); @@ -319,7 +444,11 @@ export class ReferenceFetcher { const got = 'sha256:' + hash.digest('base64url'); if (got !== t.digest) { // Vector 5: never presented as the described content. - return { ok: false, error: `digest mismatch (claimed ${t.digest.slice(0, 20)}…, got ${got.slice(0, 20)}…)` }; + return fail( + `the bytes that arrived don't match the reference's digest (${t.digest.slice(0, 20)}…, against ${got.slice(0, 20)}…), ` + + "so they weren't kept", + "Trying again may work; if it fails the same way, fetching may work once a fresh reference to it arrives.", + ); } digestVerified = true; } @@ -334,8 +463,21 @@ export class ReferenceFetcher { const observedMime = sniffMime(data) ?? headerMime ?? claimedMime ?? 'application/octet-stream'; const ext = EXT_BY_MIME.get(observedMime) ?? EXT_BY_MIME.get(headerMime ?? '') ?? EXT_BY_MIME.get(claimedMime ?? '') ?? 'bin'; // §7.3: host-generated storage name; the server's `name` is display-only. - const path = await this.saveFile(`${record.refId}.${ext}`, data, observedMime); - if (path === null) return { ok: false, error: 'no writable workspace mount to store the payload' }; + // A null from the save hook covers no workspace, no writable mount and + // a failed write, and the hook logs which. A throw is the save's failure + // too, not the fetch's, so it's logged here. + const fileName = `${record.refId}.${ext}`; + let path: string | null; + try { + path = await this.saveFile(fileName, data, observedMime); + } catch (e) { + console.error(`[refs] couldn't store ${fileName}: ${boundedForLog(describeCause(e))}`); + path = null; + } + if (path === null) { + return fail("it was fetched, but this host couldn't store it in its workspace, so nothing was kept", + 'Calling again is likely to fail the same way; whoever runs this host can see why in its log.'); + } record.fetchedPath = path; record.verifiedBytes = data.length; @@ -343,8 +485,13 @@ export class ReferenceFetcher { record.digestVerified = digestVerified; return { ok: true, path, bytes: data.length, mimeType: observedMime, digestVerified }; } catch (e) { - const msg = (e as Error).name === 'AbortError' ? 'fetch timed out or aborted' : (e as Error).message; - return { ok: false, error: msg }; + // The timer is the only abort that throws: the budget and the ceiling + // abort the download and return at once. + if ((e as Error)?.name === 'AbortError') { + const seconds = timeoutMs / 1000; + return fail(`it took longer than ${seconds} second${seconds === 1 ? '' : 's'}`, TRY_LATER); + } + return fail(`the request to ${server} failed: ${boundedForLog(whyFetchFailed(e))}`, REQUEST_FAILED); } finally { clearTimeout(timer); } diff --git a/src/mcpl/references.ts b/src/mcpl/references.ts index e37bab3f0..f33879c58 100644 --- a/src/mcpl/references.ts +++ b/src/mcpl/references.ts @@ -223,12 +223,13 @@ export const EXPIRY_REASON = { export const UNBOUND_REASON = "not tied to any server, so there's nowhere to fetch it from"; /** The fetcher's refusal for a server it has no address to dial, one the - * host doesn't connect to over a network, such as a stdio server; the - * fetcher dials only through a server's address. save_recent_image gives it - * too. A stub can't know it, since only the framework holds its servers' + * host doesn't connect to over a network, such as a stdio server, as a + * clause that completes "couldn't fetch : " and says so of the server; + * the fetcher dials only through a server's address. save_recent_image gives + * it too. A stub can't know it, since only the framework holds its servers' * configs. */ export function noAddressReason(serverId: string): string { - return `no address: this host doesn't connect to server ${serverId} over a network, so there's nowhere to fetch it from`; + return `this host doesn't connect to the "${serverId}" server over a network, so there's nowhere to fetch it from`; } /** Strip control characters (C0, DEL, C1) and every bidi control (marks, diff --git a/test/mcpl-reference-binding.test.ts b/test/mcpl-reference-binding.test.ts index c12dc95a6..678a20060 100644 --- a/test/mcpl-reference-binding.test.ts +++ b/test/mcpl-reference-binding.test.ts @@ -174,31 +174,35 @@ test("fetch_reference reaches a push event's reference through its server's own const fetched = await fetchReference(own!.refId); assert.equal(fetched.success, true, fetched.error); assert.equal(dialed, 1); + // fetch_reference's sentence: the fetcher's clause, then what calling again can do. + const refusedWith = (refId: string, clause: string, remedy = "Calling again won't change that.") => + `fetch_reference couldn't fetch "${refId}": ${clause}. ${remedy}`; const refused = await fetchReference(elsewhere!.refId); assert.equal(refused.success, false); - assert.match(refused.error ?? '', /third-party origin/); + assert.match(refused.error ?? '', /^fetch_reference couldn't fetch "ref_\S+": it points to \S+, not to the "\S+" server's own address /); const noAddress = await fetchReference(stdio!.refId); assert.equal(noAddress.success, false); - assert.equal(noAddress.error, noAddressReason('stdio-srv')); + assert.equal(noAddress.error, refusedWith(stdio!.refId, noAddressReason('stdio-srv'))); const notDialed = await fetchReference(spawned!.refId); assert.equal(notDialed.success, false); - assert.equal(notDialed.error, noAddressReason('spawned-srv')); + assert.equal(notDialed.error, refusedWith(spawned!.refId, noAddressReason('spawned-srv'))); const unknown = await fetchReference(gone!.refId); assert.equal(unknown.success, false); - assert.equal(unknown.error, 'unknown server: gone-srv'); + assert.equal(unknown.error, refusedWith(gone!.refId, 'this host no longer has the "gone-srv" server it came from', + "Calling again won't change that unless whoever runs this host adds that server back.")); assert.equal(dialed, 1, 'nothing dialed for the other four'); const throughWebSocket = await fetchReference(wsCommand!.refId); assert.equal(throughWebSocket.success, true, throughWebSocket.error); assert.equal(dialed, 2); const spawnedByTransport = await fetchReference(stdioUrl!.refId); assert.equal(spawnedByTransport.success, false); - assert.equal(spawnedByTransport.error, noAddressReason('stdio-url-srv')); + assert.equal(spawnedByTransport.error, refusedWith(stdioUrl!.refId, noAddressReason('stdio-url-srv'))); const undialable = await fetchReference(wsHttp!.refId); assert.equal(undialable.success, false); - assert.equal(undialable.error, noAddressReason('ws-http-srv')); + assert.equal(undialable.error, refusedWith(wsHttp!.refId, noAddressReason('ws-http-srv'))); const fragment = await fetchReference(wsFragment!.refId); assert.equal(fragment.success, false); - assert.equal(fragment.error, noAddressReason('ws-fragment-srv')); + assert.equal(fragment.error, refusedWith(wsFragment!.refId, noAddressReason('ws-fragment-srv'))); assert.equal(dialed, 2, 'nothing dialed for the last three'); }); diff --git a/test/mcpl-reference-fetcher.test.ts b/test/mcpl-reference-fetcher.test.ts index 4e201885b..dbbb930f4 100644 --- a/test/mcpl-reference-fetcher.test.ts +++ b/test/mcpl-reference-fetcher.test.ts @@ -8,7 +8,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { createServer } from 'node:http'; import { createHash } from 'node:crypto'; -import { ReferenceFetcher, normalizedOrigin, type ServerResolution } from '../src/mcpl/reference-fetcher.js'; +import { DEFAULT_FETCH_MAX_BYTES, ReferenceFetcher, normalizedOrigin, type ServerResolution } from '../src/mcpl/reference-fetcher.js'; import { EXPIRY_REASON, noAddressReason, ReferenceRegistry, UNBOUND_REASON, classifyBlock, referenceExpiry, referenceRegistry } from '../src/mcpl/references.js'; import { AgentFramework } from '../src/framework.js'; @@ -54,6 +54,9 @@ const server = createServer((req, res) => { // claims small in testimony, streams big (vector 10) res.writeHead(200, { 'content-type': 'application/octet-stream' }); res.end(Buffer.alloc(200_000, 7)); + } else if (url.pathname.startsWith('/status/')) { + // answers with the status its path names, and nothing else + res.writeHead(Number(url.pathname.slice('/status/'.length))); res.end(); } else { res.writeHead(404); res.end(); } @@ -76,6 +79,15 @@ function makeRecord(reg: ReferenceRegistry, uri: string, extra: Record, { kind: 'reference' }>).testimony, serverId); } +// What a failure says calling again can do, in the fetcher's words. +const NOTHING_TO_DO = "Calling again won't change that."; +const FRESH_REFERENCE = "Once a fresh reference to it arrives, fetching may work; until then, fetching it again won't."; +const TRY_LATER = 'Trying again later may work.'; +const REQUEST_FAILED = "Trying again later may work; if it fails the same way, whoever runs this host can check its connection to the server."; +const BUDGET = "The budget doesn't renew on its own, so calling again won't change that unless whoever runs this host raises or renews it."; +const HOST_CEILING = "No max_bytes or setting lifts that limit, so calling again won't change it."; +const SERVER_BACK = "Calling again won't change that unless whoever runs this host adds that server back."; + test('happy path: origin-bound fetch, bearer auth applied host-side, digest verified', async () => { const reg = new ReferenceRegistry(); const f = makeFetcher({ srv: { url: `ws://127.0.0.1:${port}`, token: 'sekrit' } }); @@ -97,7 +109,8 @@ test('vector 5: digest mismatch is never presented', async () => { const rec = makeRecord(reg, `${ORIGIN}/ok`, { digest: 'sha256:' + 'B'.repeat(43) }); const out = await f.fetch(rec); assert.equal(out.ok, false); - assert.match(out.error!, /digest mismatch/); + assert.match(out.error!, /^the bytes that arrived don't match the reference's digest \(sha256:B{13}…, against sha256:[\w-]{13}…\), so they weren't kept$/); + assert.equal(out.remedy, "Trying again may work; if it fails the same way, fetching may work once a fresh reference to it arrives."); assert.equal(rec.fetchedPath, undefined); }); @@ -107,7 +120,10 @@ test('vector 10: stream past the ceiling aborts on actual bytes', async () => { const rec = makeRecord(reg, `${ORIGIN}/liar`, { sizeBytes: 100 }); const out = await f.fetch(rec, { maxBytes: 50_000 }); assert.equal(out.ok, false); - assert.match(out.error!, /exceeded ceiling/); + const [, received] = /^the download had reached (\d+) bytes, past the 50000 bytes this call allows, so it was stopped and nothing was kept$/.exec(out.error!) ?? []; + assert.ok(received, out.error); + // Its whole size is unknown, only that it's past what arrived, and the budget has room for the host's ceiling. + assert.equal(out.remedy, `It's at least ${received} bytes; a larger max_bytes, up to ${DEFAULT_FETCH_MAX_BYTES}, may fetch it.`); }); test('vector 4: a claim already over the ceiling refuses without dialing', async () => { @@ -116,7 +132,13 @@ test('vector 4: a claim already over the ceiling refuses without dialing', async const rec = makeRecord(reg, `${ORIGIN}/ok`, { sizeBytes: 10_000_000 }); const out = await f.fetch(rec, { maxBytes: 1000 }); assert.equal(out.ok, false); - assert.match(out.error!, /exceeds fetch ceiling/); + assert.equal(out.error, 'it claims 10000000 bytes, over the 1000 bytes this call allows'); + assert.equal(out.remedy, 'Pass max_bytes of at least 10000000 to fetch it.'); + // Past the host's own ceiling, no max_bytes can fetch it. + const huge = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?huge`, { sizeBytes: DEFAULT_FETCH_MAX_BYTES + 1 }), { maxBytes: 1000 }); + assert.equal(huge.error, `it claims ${DEFAULT_FETCH_MAX_BYTES + 1} bytes, over the ${DEFAULT_FETCH_MAX_BYTES} bytes this host fetches at most`); + assert.equal(huge.remedy, HOST_CEILING); + assert.equal(hits.get('/ok?huge') ?? 0, 0, 'nothing dialed'); }); test('vector 11: non-http(s) scheme refused', async () => { @@ -125,7 +147,22 @@ test('vector 11: non-http(s) scheme refused', async () => { const rec = makeRecord(reg, 'file:///etc/passwd'); const out = await f.fetch(rec); assert.equal(out.ok, false); - assert.match(out.error!, /scheme not allowed|third-party/); + assert.equal(out.error, 'its address uses file:, and this host fetches only http: and https: addresses'); + assert.equal(out.remedy, NOTHING_TO_DO); +}); + +test("a ws: or wss: reference on its server's own origin is refused for its scheme, with nothing dialed", async () => { + // An origin reads ws: as http: and wss: as https:, so each is on the dialed + // origin. fetch can't dial either, and would fail only once dialed. + const reg = new ReferenceRegistry(); + for (const [scheme, dialed] of [['ws:', `ws://127.0.0.1:${port}`], ['wss:', 'wss://ws-ref.example.invalid']] as const) { + const f = makeFetcher({ srv: { url: dialed } }); + const host = scheme === 'ws:' ? `127.0.0.1:${port}` : 'ws-ref.example.invalid'; + const out = await f.fetch(makeRecord(reg, `${scheme}//${host}/ok?scheme=${scheme}`)); + assert.equal(out.error, `its address uses ${scheme}, and this host fetches only http: and https: addresses`, scheme); + assert.equal(out.remedy, NOTHING_TO_DO, scheme); + } + assert.equal(hits.get('/ok?scheme=ws:') ?? 0, 0, 'nothing dialed'); }); test('vector 12: redirects refused, no traversal', async () => { @@ -134,7 +171,8 @@ test('vector 12: redirects refused, no traversal', async () => { const rec = makeRecord(reg, `${ORIGIN}/redirect`); const out = await f.fetch(rec); assert.equal(out.ok, false); - assert.match(out.error!, /redirect refused/); + assert.equal(out.error, `the "srv" server answered HTTP 302, a redirect, and this host doesn't follow redirects`); + assert.equal(out.remedy, NOTHING_TO_DO); }); test('third-party origin refused (v1 policy: dialed origin only)', async () => { @@ -143,20 +181,55 @@ test('third-party origin refused (v1 policy: dialed origin only)', async () => { const rec = makeRecord(reg, 'https://cdn.example.com/thing.wav'); const out = await f.fetch(rec); assert.equal(out.ok, false); - assert.match(out.error!, /third-party origin/); + assert.equal(out.error, + `it points to https://cdn.example.com, not to the "srv" server's own address (${ORIGIN}), and this host fetches only from the server itself`); + assert.equal(out.remedy, NOTHING_TO_DO); +}); + +test('a reference whose address carries a user name or password is refused before dialing, without repeating them', async () => { + // fetch refuses such a url with a message that holds it whole, so the + // credential would reach the agent's context and the host's log. + const reg = new ReferenceRegistry(); + const f = makeFetcher({ srv: { url: `ws://127.0.0.1:${port}` } }); + const clause = "its address carries a user name or password, which this host doesn't send"; + // A user name with a password, a user name alone, and a password alone. + for (const uri of [`http://user:s3cret@127.0.0.1:${port}/ok?userinfo`, `http://justauser@127.0.0.1:${port}/ok?userinfo-name`, + `http://:s3cret@127.0.0.1:${port}/ok?userinfo-password`]) { + // A claimed size within the eager limit, so only the refusal keeps the eager fetch away. + const rec = makeRecord(reg, uri, { sizeBytes: 1000 }); + assert.deepEqual(f.refusal(rec), { error: clause, remedy: NOTHING_TO_DO }, uri); + assert.equal(f.isEagerEligible(rec), false, uri); + const out = await f.fetch(rec); + assert.equal(out.error, clause, uri); + assert.equal(out.remedy, NOTHING_TO_DO, uri); + } + assert.equal(['/ok?userinfo', '/ok?userinfo-name', '/ok?userinfo-password'].reduce((n, k) => n + (hits.get(k) ?? 0), 0), 0, 'nothing dialed'); +}); + +test('an http: reference from a server dialed over TLS is refused as another origin, with nothing dialed', async () => { + // The same host and port, so only the scheme differs: the origin check, which + // compares origins with their schemes, is all that refuses it. + const reg = new ReferenceRegistry(); + const f = makeFetcher({ srv: { url: `wss://127.0.0.1:${port}` } }); + const rec = makeRecord(reg, `${ORIGIN}/ok?plaintext-from-tls`); + const out = await f.fetch(rec); + assert.equal(out.error, + `it points to ${ORIGIN}, not to the "srv" server's own address (wss://127.0.0.1:${port}), and this host fetches only from the server itself`); + assert.equal(out.remedy, NOTHING_TO_DO); + assert.equal(hits.get('/ok?plaintext-from-tls') ?? 0, 0, 'nothing dialed'); }); test('vector 16: expired (and unparseable-expiry) references refuse locally, and take no eager slot', async () => { const reg = new ReferenceRegistry(); const f = makeFetcher({ srv: { url: ORIGIN } }); - for (const [expiresAt, why] of [['2001-01-01T00:00:00Z', /^reference expired$/], ['not-a-date', /^reference treated as expired, since its expiry time can't be read$/]] as const) { + for (const [expiresAt, why] of [['2001-01-01T00:00:00Z', /^the reference is expired$/], ['not-a-date', /^the reference is treated as expired, since its expiry time can't be read$/]] as const) { // Small, sized and from the dialed origin: only its expiry keeps it from the eager default. const rec = makeRecord(reg, `${ORIGIN}/ok?e=${expiresAt}`, { expiresAt, sizeBytes: PAYLOAD.length }); assert.equal(f.isEagerEligible(rec), false, `${expiresAt}: the eager default would only be refused`); const out = await f.fetch(rec); assert.equal(out.ok, false); - assert.match(out.error!, /expired/); assert.match(out.error!, why); + assert.equal(out.remedy, FRESH_REFERENCE); } }); @@ -171,7 +244,7 @@ test('vector 16: an expiry rejected as a field refuses locally, with nothing dia assert.equal(f.isEagerEligible(rec), false, `${label}: the eager default would only be refused`); const out = await f.fetch(rec); assert.equal(out.ok, false, label); - assert.equal(out.error, "reference treated as expired, since its expiry time isn't in a form this host accepts", label); + assert.equal(out.error, "the reference is treated as expired, since its expiry time isn't in a form this host accepts", label); } assert.equal(saved.length, before, 'nothing saved'); assert.equal(lastAuth, 'untouched', 'the origin was never dialed'); @@ -378,11 +451,10 @@ for (const expired of [true, false]) { }); } -/** A refusal by the budget says it lasts, so a second try is no use unless - * whoever runs the host raises or renews it. */ -const BUDGET_LASTS = "the budget doesn't renew on its own, so calling again won't change that unless whoever runs this host raises or renews it"; /** The budget's stop partway, with the server's running total and the budget. */ -const STOPPED_BY_BUDGET = new RegExp(`^aborted: per-server fetch budget exhausted \\((\\d+)/(\\d+) bytes\\); ${BUDGET_LASTS}$`); +const STOPPED_BY_BUDGET = /^this host's fetch budget for the "srv" server ran out during the download \((\d+) of (\d+) bytes\), so it was stopped and nothing was kept$/; +/** The budget's refusal before dialing, with what's used of it. */ +const usedUp = (used: number | string, budget: number | string) => `this host's fetch budget for the "srv" server is used up (${used} of ${budget} bytes)`; test('per-server budget exhausts cumulatively, each fetch counting once', async () => { const reg = new ReferenceRegistry(); @@ -394,7 +466,10 @@ test('per-server budget exhausts cumulatively, each fetch counting once', async assert.equal(b.ok, true, b.error); const c = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?n=3`)); assert.equal(c.ok, false); - assert.equal(c.error, `per-server fetch budget exhausted (${2 * PAYLOAD.length}/${2 * PAYLOAD.length} bytes); ${BUDGET_LASTS}`); + assert.equal(c.error, usedUp(2 * PAYLOAD.length, 2 * PAYLOAD.length)); + // A refusal by the budget says it lasts, so a second try is no use unless + // whoever runs the host raises or renews it. + assert.equal(c.remedy, BUDGET); }); test("a download the budget stops says so, and what arrived counts toward the next fetch's refusal", async () => { @@ -405,8 +480,9 @@ test("a download the budget stops says so, and what arrived counts toward the ne const [, total, budget] = STOPPED_BY_BUDGET.exec(out.error!) ?? []; assert.ok(Number(total) > 1000, out.error); assert.equal(budget, '1000'); + assert.equal(out.remedy, BUDGET); const next = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?after-budget-abort`)); - assert.equal(next.error, `per-server fetch budget exhausted (${total}/1000 bytes); ${BUDGET_LASTS}`); + assert.equal(next.error, usedUp(total, 1000)); }); test('a budget crossed on a later chunk stops the download there, with every chunk counted', async () => { @@ -433,14 +509,15 @@ test('concurrent fetches from one server share its budget, and none takes what a for (const o of refused) assert.match(o.error!, STOPPED_BY_BUDGET); // Every byte read counts, so a fifth is refused before it dials. const fifth = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?shared-5`)); - assert.match(fifth.error!, /^per-server fetch budget exhausted/); + assert.match(fifth.error!, /^this host's fetch budget for the "srv" server is used up /); }); test("bytes the workspace couldn't store still count against the budget", async () => { const reg = new ReferenceRegistry(); const unsaved = new ReferenceFetcher(() => ({ url: ORIGIN, autofetch: { maxTotalBytes: PAYLOAD.length + 100 } }), async () => null); const notStored = await unsaved.fetch(makeRecord(reg, `${ORIGIN}/ok?unsaved-1`)); - assert.equal(notStored.error, 'no writable workspace mount to store the payload'); + assert.equal(notStored.error, "it was fetched, but this host couldn't store it in its workspace, so nothing was kept"); + assert.equal(notStored.remedy, 'Calling again is likely to fail the same way; whoever runs this host can see why in its log.'); assert.match((await unsaved.fetch(makeRecord(reg, `${ORIGIN}/ok?unsaved-2`))).error!, STOPPED_BY_BUDGET); }); @@ -448,7 +525,7 @@ test("bytes that don't match their digest still count against the budget", async const reg = new ReferenceRegistry(); const f = makeFetcher({ srv: { url: ORIGIN, autofetch: { maxTotalBytes: PAYLOAD.length + 100 } } }); const mismatch = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?mismatch-charged`, { digest: 'sha256:' + 'B'.repeat(43) })); - assert.match(mismatch.error!, /^digest mismatch/); + assert.match(mismatch.error!, /^the bytes that arrived don't match the reference's digest /); const after = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?after-mismatch`)); // The stop shows the server's total, the mismatched payload included, not // this download's own bytes. @@ -463,7 +540,8 @@ test('bytes that arrived before a timeout still count against the budget', async // 1000 bytes arrive at once, and the rest only after the fetch gives up. // The timeout leaves a second for the headers and the first 1000 bytes. const slow = await f.fetch(makeRecord(reg, `${ORIGIN}/drip?first=1000&rest=1000&wait=60000`), { timeoutMs: 1000 }); - assert.equal(slow.error, 'fetch timed out or aborted'); + assert.equal(slow.error, 'it took longer than 1 second'); + assert.equal(slow.remedy, TRY_LATER); assert.match((await f.fetch(makeRecord(reg, `${ORIGIN}/ok?after-timeout`))).error!, STOPPED_BY_BUDGET); }); @@ -473,7 +551,12 @@ test('bytes past the ceiling still count against the budget', async () => { // 1200 bytes arrive before the pause: past max_bytes, within the budget. A // payload alone fits what's left only if what was read went uncounted. const out = await f.fetch(makeRecord(reg, `${ORIGIN}/drip?first=1200&rest=5000&wait=10000`), { maxBytes: 1000 }); - assert.match(out.error!, /^aborted: actual bytes exceeded ceiling at \d+$/); + const [, received] = /^the download had reached (\d+) bytes, past the 1000 bytes this call allows, so it was stopped and nothing was kept$/.exec(out.error!) ?? []; + assert.ok(received, out.error); + // Past max_bytes, and no more than the 1200 bytes sent before the pause, however they were chunked. + assert.ok(Number(received) > 1000 && Number(received) <= 1200, received); + // A larger max_bytes can read no more than the budget has left after these bytes. + assert.equal(out.remedy, `It's at least ${received} bytes; a larger max_bytes, up to ${PAYLOAD.length + 500 - Number(received)}, may fetch it.`); assert.match((await f.fetch(makeRecord(reg, `${ORIGIN}/ok?after-ceiling`))).error!, STOPPED_BY_BUDGET); }); @@ -501,11 +584,13 @@ test("a claim over max_bytes and over what's left of the budget is refused for t assert.equal((await f.fetch(makeRecord(reg, `${ORIGIN}/ok?spend-most`))).ok, true); // 100 bytes are left: a larger max_bytes wouldn't let this claim through. const out = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?claim-past-both`, { sizeBytes: 10_000 }), { maxBytes: 1000 }); - assert.equal(out.error, `claimed 10000 bytes exceeds what's left of the per-server fetch budget (${PAYLOAD.length}/${PAYLOAD.length + 100} bytes); ${BUDGET_LASTS}`); + assert.equal(out.error, `it claims 10000 bytes, more than the 100 bytes left of this host's fetch budget for the "srv" server`); + assert.equal(out.remedy, BUDGET); // Within what's left, the same claim is refused for the ceiling, which a larger max_bytes lifts. const roomy = makeFetcher({ srv: { url: ORIGIN } }); const ceilingOnly = await roomy.fetch(makeRecord(reg, `${ORIGIN}/ok?claim-past-ceiling`, { sizeBytes: 10_000 }), { maxBytes: 1000 }); - assert.equal(ceilingOnly.error, 'claimed 10000 bytes exceeds fetch ceiling 1000'); + assert.equal(ceilingOnly.error, 'it claims 10000 bytes, over the 1000 bytes this call allows'); + assert.equal(ceilingOnly.remedy, 'Pass max_bytes of at least 10000 to fetch it.'); }); test("a claim over what's left of the budget but within max_bytes still dials, since a claim can be wrong", async () => { @@ -522,7 +607,7 @@ test('a claim over max_bytes, once the budget is spent, is refused for the budge const f = makeFetcher({ srv: { url: ORIGIN, autofetch: { maxTotalBytes: PAYLOAD.length } } }); assert.equal((await f.fetch(makeRecord(reg, `${ORIGIN}/ok?spend`))).ok, true); const out = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?claim-over`, { sizeBytes: 10_000 }), { maxBytes: 1000 }); - assert.equal(out.error, `per-server fetch budget exhausted (${PAYLOAD.length}/${PAYLOAD.length} bytes); ${BUDGET_LASTS}`); + assert.equal(out.error, usedUp(PAYLOAD.length, PAYLOAD.length)); }); test('eager eligibility: small + sized + connection origin only', () => { @@ -590,7 +675,8 @@ test('fetch_reference answers an unbound reference with the reason its stub give const { result } = await answered; assert.equal(result.success, false); assert.equal(result.isError, true); - assert.equal(result.error, `reference ${UNBOUND_REASON}`); + // The fetcher's clause and remedy, in fetch_reference's sentence. + assert.equal(result.error, `fetch_reference couldn't fetch "${record.refId}": it's ${UNBOUND_REASON}. ${FRESH_REFERENCE}`); assert.equal(hits.get('/ok?unbound') ?? 0, 0, 'nothing dialed'); }); @@ -602,7 +688,8 @@ test('a reference both unbound and expired is refused as unbound, the reason its assert.equal(referenceExpiry(t), 'passed'); const out = await makeFetcher({ srv: { url: ORIGIN } }).fetch(reg.register(t)); assert.equal(out.ok, false); - assert.equal(out.error, `reference ${UNBOUND_REASON}`); + assert.equal(out.error, `it's ${UNBOUND_REASON}`); + assert.equal(out.remedy, FRESH_REFERENCE); assert.equal(hits.get('/ok?unbound-expired') ?? 0, 0, 'nothing dialed'); }); @@ -639,15 +726,16 @@ test('a uri on none of its servers\' origins is refused with the first server\'s assert.equal(f.isEagerEligible(record), false); const out = await f.fetch(record); assert.equal(out.ok, false); - assert.match(out.error ?? '', new RegExp(`^third-party origin \\(http://127\\.0\\.0\\.1:\\d+ != dialed https://${order[0]}\\.example\\.invalid\\)`)); + assert.equal(out.error, + `it points to ${ORIGIN}, not to the "${order[0]}" server's own address (https://${order[0]}.example.invalid), and this host fetches only from the server itself`); assert.equal(hits.get(`/ok?none-on-origin=${order[0]}`) ?? 0, 0, 'nothing dialed'); } }); test('a server unknown to the host ahead of one off the origin is the refusal, and the other way round the origin check\'s is', async () => { for (const [order, refusal] of [ - [['gone', 'a'], /^unknown server: gone$/], - [['a', 'gone'], /^third-party origin \(http:\/\/127\.0\.0\.1:\d+ != dialed https:\/\/a\.example\.invalid\)/], + [['gone', 'a'], /^this host no longer has the "gone" server it came from$/], + [['a', 'gone'], /^it points to http:\/\/127\.0\.0\.1:\d+, not to the "a" server's own address \(https:\/\/a\.example\.invalid\)/], ] as const) { const reg = new ReferenceRegistry(); const f = makeFetcher({ a: { url: 'https://a.example.invalid' } }); @@ -682,7 +770,10 @@ test('a server unknown to the host is its refusal while it is the only one, and const f = makeFetcher({ owner: { url: `ws://127.0.0.1:${port}` } }); const uri = `${ORIGIN}/ok?unknown-first`; const record = makeRecord(reg, uri, {}, 'gone'); - assert.equal((await f.fetch(record)).error, 'unknown server: gone'); + const gone = await f.fetch(record); + assert.equal(gone.error, 'this host no longer has the "gone" server it came from'); + // The resolver reads the host's configs at each fetch, so the server's return would change it. + assert.equal(gone.remedy, SERVER_BACK); makeRecord(reg, uri, {}, 'owner'); assert.deepEqual(record.serverIds, ['gone', 'owner']); const out = await f.fetch(record); @@ -695,8 +786,8 @@ test('a server with no address is refused in a phrase naming it, and passed over const uri = `${ORIGIN}/ok?no-address`; // Small enough for an eager slot, so it takes none only for the server it came from. const record = makeRecord(reg, uri, { sizeBytes: PAYLOAD.length }, 'stdio-srv'); - assert.deepEqual(f.refusal(record), { reason: noAddressReason('stdio-srv'), ofReference: false }); - assert.equal((await f.fetch(record)).error, "no address: this host doesn't connect to server stdio-srv over a network, so there's nowhere to fetch it from"); + assert.deepEqual(f.refusal(record), { error: noAddressReason('stdio-srv'), remedy: NOTHING_TO_DO }); + assert.equal((await f.fetch(record)).error, "this host doesn't connect to the \"stdio-srv\" server over a network, so there's nowhere to fetch it from"); assert.equal(f.isEagerEligible(record), false); assert.equal(hits.get('/ok?no-address') ?? 0, 0, 'nothing dialed'); makeRecord(reg, uri, { sizeBytes: PAYLOAD.length }, 'owner'); @@ -708,15 +799,15 @@ test('a server with no address is refused in a phrase naming it, and passed over test('a server with no address ahead of one off the origin is the refusal, and the other way round the origin check\'s is', async () => { for (const [order, refusal] of [ - [['stdio-srv', 'a'], /^no address: this host doesn't connect to server stdio-srv over a network/], - [['a', 'stdio-srv'], /^third-party origin \(http:\/\/127\.0\.0\.1:\d+ != dialed https:\/\/a\.example\.invalid\)/], + [['stdio-srv', 'a'], /^this host doesn't connect to the "stdio-srv" server over a network/], + [['a', 'stdio-srv'], /^it points to http:\/\/127\.0\.0\.1:\d+, not to the "a" server's own address \(https:\/\/a\.example\.invalid\)/], ] as const) { const reg = new ReferenceRegistry(); const f = makeFetcher({ 'stdio-srv': 'no-address', a: { url: 'https://a.example.invalid' } }); const uri = `${ORIGIN}/ok?no-address-and-off-origin=${order[0]}`; const record = makeRecord(reg, uri, {}, order[0]); makeRecord(reg, uri, {}, order[1]); - assert.match(f.refusal(record)?.reason ?? '', refusal); + assert.match(f.refusal(record)?.error ?? '', refusal); assert.match((await f.fetch(record)).error ?? '', refusal); assert.equal(hits.get(`/ok?no-address-and-off-origin=${order[0]}`) ?? 0, 0, 'nothing dialed'); } @@ -728,32 +819,40 @@ test('refusal() answers what the fetch would refuse before dialing, case by case srv: { url: `ws://127.0.0.1:${port}` }, 'stdio-srv': 'no-address', spent: { url: `ws://127.0.0.1:${port}`, autofetch: { maxTotalBytes: 0 } }, + // A scheme that isn't special, whose URL origin is 'null', and a token in its userinfo. + relay: { url: 'mcpl://tok:sekrit@relay.example:9000/path?key=x' }, // An eager limit past the fetch's own ceiling and its budget: those still decide. roomy: { url: `ws://127.0.0.1:${port}`, autofetch: { maxBytes: 4 * 1024 * 1024 * 1024 } }, }); const small = { sizeBytes: PAYLOAD.length }; - const cases: Array<[string, ReturnType, string | null]> = [ + // Each with what it says happened, and what it says calling again can do. + const cases: Array<[string, ReturnType, string, string]> = [ // makeRecord's server defaults when given undefined, so this one registers itself. - ['unbound', reg.register((classifyBlock({ type: 'resource', uri: `${ORIGIN}/ok?case=unbound`, ...small }) as Extract, { kind: 'reference' }>).testimony), `reference ${UNBOUND_REASON}`], - ['unknown server', makeRecord(reg, `${ORIGIN}/ok?case=unknown`, small, 'gone'), 'unknown server: gone'], - ['no address', makeRecord(reg, `${ORIGIN}/ok?case=no-address`, small, 'stdio-srv'), noAddressReason('stdio-srv')], - ['off the origin', makeRecord(reg, 'https://cdn.example.invalid/case-origin', small, 'srv'), null], - ['expired', makeRecord(reg, `${ORIGIN}/ok?case=expired`, { ...small, expiresAt: '2001-01-01T00:00:00Z' }, 'srv'), `reference ${EXPIRY_REASON.passed}`], - ['budget spent', makeRecord(reg, `${ORIGIN}/ok?case=budget`, small, 'spent'), `per-server fetch budget exhausted (0/0 bytes); ${BUDGET_LASTS}`], - ['over the ceiling', makeRecord(reg, `${ORIGIN}/ok?case=ceiling`, { sizeBytes: 512 * 1024 * 1024 }, 'roomy'), 'claimed 536870912 bytes exceeds fetch ceiling 268435456'], - ['over the ceiling and what is left of the budget', makeRecord(reg, `${ORIGIN}/ok?case=both`, { sizeBytes: 2 * 1024 * 1024 * 1024 }, 'roomy'), - `claimed 2147483648 bytes exceeds what's left of the per-server fetch budget (0/1073741824 bytes); ${BUDGET_LASTS}`], + ['unbound', reg.register((classifyBlock({ type: 'resource', uri: `${ORIGIN}/ok?case=unbound`, ...small }) as Extract, { kind: 'reference' }>).testimony), + `it's ${UNBOUND_REASON}`, FRESH_REFERENCE], + ['unknown server', makeRecord(reg, `${ORIGIN}/ok?case=unknown`, small, 'gone'), 'this host no longer has the "gone" server it came from', SERVER_BACK], + ['no address', makeRecord(reg, `${ORIGIN}/ok?case=no-address`, small, 'stdio-srv'), noAddressReason('stdio-srv'), NOTHING_TO_DO], + ['off the origin', makeRecord(reg, 'https://cdn.example.invalid/case-origin', small, 'srv'), + `it points to https://cdn.example.invalid, not to the "srv" server's own address (ws://127.0.0.1:${port}), and this host fetches only from the server itself`, NOTHING_TO_DO], + // The server's address as configured, its scheme and host, without its userinfo, path or query. + ['off the origin of a server on another scheme', makeRecord(reg, 'https://cdn.example.invalid/case-relay', small, 'relay'), + 'it points to https://cdn.example.invalid, not to the "relay" server\'s own address (mcpl://relay.example:9000), and this host fetches only from the server itself', NOTHING_TO_DO], + ['expired', makeRecord(reg, `${ORIGIN}/ok?case=expired`, { ...small, expiresAt: '2001-01-01T00:00:00Z' }, 'srv'), `the reference is ${EXPIRY_REASON.passed}`, FRESH_REFERENCE], + ['budget spent', makeRecord(reg, `${ORIGIN}/ok?case=budget`, small, 'spent'), 'this host\'s fetch budget for the "spent" server is used up (0 of 0 bytes)', BUDGET], + ['over the ceiling', makeRecord(reg, `${ORIGIN}/ok?case=ceiling`, { sizeBytes: 512 * 1024 * 1024 }, 'roomy'), + 'it claims 536870912 bytes, over the 268435456 bytes this host fetches at most', HOST_CEILING], + // Past the host's own ceiling, whatever the budget has left, since no budget lifts it. + ['over the host ceiling and what is left of the budget', makeRecord(reg, `${ORIGIN}/ok?case=both`, { sizeBytes: 2 * 1024 * 1024 * 1024 }, 'roomy'), + 'it claims 2147483648 bytes, over the 268435456 bytes this host fetches at most', HOST_CEILING], ]; - for (const [what, record, error] of cases) { + for (const [what, record, error, remedy] of cases) { const refusal = f.refusal(record); assert.ok(refusal, `${what}: a refusal`); assert.equal(f.isEagerEligible(record), false, `${what}: no eager slot`); - const said = refusal.ofReference ? `reference ${refusal.reason}` : refusal.reason; + assert.deepEqual(refusal, { error, remedy }, what); const out = await f.fetch(record); assert.equal(out.ok, false, what); - assert.equal(out.error, said, `${what}: the fetch answers what refusal() said`); - if (error !== null) assert.equal(said, error, what); - else assert.match(said, /^third-party origin \(https:\/\/cdn\.example\.invalid != dialed http:\/\/127\.0\.0\.1:\d+\); not fetched by this host$/, what); + assert.deepEqual({ error: out.error, remedy: out.remedy }, refusal, `${what}: the fetch answers what refusal() said`); } assert.equal([...hits.keys()].filter((k) => k.startsWith('/ok?case=')).length, 0, 'nothing dialed'); // Null where the fetch serves a saved copy or dials, and the one it would dial takes an eager slot. @@ -770,20 +869,23 @@ test('refusal() answers what the fetch would refuse before dialing, case by case assert.equal(f.isEagerEligible(keptCopy), false); }); -test('a reference with two faults is refused for the first the fetch checks: the server, then the expiry, the spent budget, the claimed size', async () => { +test('a reference with two faults is refused for the first the fetch checks: the server, then the expiry, a claim past the host\'s ceiling, the spent budget', async () => { const reg = new ReferenceRegistry(); const f = makeFetcher({ 'stdio-srv': 'no-address', spent: { url: `ws://127.0.0.1:${port}`, autofetch: { maxTotalBytes: 0 } } }); const past = '2001-01-01T00:00:00Z'; const huge = 512 * 1024 * 1024; const cases: Array<[string, ReturnType, string]> = [ ['no address, expired', makeRecord(reg, `${ORIGIN}/ok?two=1`, { expiresAt: past }, 'stdio-srv'), noAddressReason('stdio-srv')], - ['expired, budget spent, over the ceiling', makeRecord(reg, `${ORIGIN}/ok?two=2`, { expiresAt: past, sizeBytes: huge }, 'spent'), `reference ${EXPIRY_REASON.passed}`], - ['budget spent, over the ceiling', makeRecord(reg, `${ORIGIN}/ok?two=3`, { sizeBytes: huge }, 'spent'), `per-server fetch budget exhausted (0/0 bytes); ${BUDGET_LASTS}`], + ['no address, over the host ceiling', makeRecord(reg, `${ORIGIN}/ok?two=5`, { sizeBytes: huge }, 'stdio-srv'), noAddressReason('stdio-srv')], + ['expired, budget spent, over the ceiling', makeRecord(reg, `${ORIGIN}/ok?two=2`, { expiresAt: past, sizeBytes: huge }, 'spent'), `the reference is ${EXPIRY_REASON.passed}`], + // Nothing lifts the host's ceiling, so it comes before a budget whoever runs the host can raise. + ['over the host ceiling, budget spent', makeRecord(reg, `${ORIGIN}/ok?two=3`, { sizeBytes: huge }, 'spent'), 'it claims 536870912 bytes, over the 268435456 bytes this host fetches at most'], + ['budget spent, within the ceiling', makeRecord(reg, `${ORIGIN}/ok?two=4`, { sizeBytes: 1000 }, 'spent'), 'this host\'s fetch budget for the "spent" server is used up (0 of 0 bytes)'], ]; for (const [what, record, error] of cases) { const refusal = f.refusal(record); assert.ok(refusal, what); - assert.equal(refusal.ofReference ? `reference ${refusal.reason}` : refusal.reason, error, what); + assert.equal(refusal.error, error, what); assert.equal((await f.fetch(record)).error, error, what); } }); @@ -855,7 +957,7 @@ test('a fetch is charged to the budget of the server it dials, not of the first assert.equal(first.ok, true, first.error); // That spent the owner's budget, so its own next reference is refused for it. const next = await f.fetch(makeRecord(reg, `${ORIGIN}/ok?charged-own`, {}, 'owner')); - assert.equal(next.error, `per-server fetch budget exhausted (${PAYLOAD.length}/${PAYLOAD.length} bytes); ${BUDGET_LASTS}`); + assert.equal(next.error, `this host's fetch budget for the "owner" server is used up (${PAYLOAD.length} of ${PAYLOAD.length} bytes)`); assert.equal(hits.get('/ok?charged-own') ?? 0, 0, 'nothing dialed'); }); @@ -885,3 +987,125 @@ test('vector 13: magic bytes beat a lying Content-Type header for storage type', assert.equal(out.mimeType, 'audio/wav', 'sniffed type must win'); assert.ok(rec.fetchedPath!.endsWith('.wav'), rec.fetchedPath); }); + +test("each HTTP status says what calling again can do: later, a fresh reference, the host's credentials, or nothing", async () => { + const reg = new ReferenceRegistry(); + const f = makeFetcher({ srv: { url: ORIGIN } }); + const expect: Array<[number, string]> = [ + // Passing trouble, which can clear. + [408, TRY_LATER], [429, TRY_LATER], [500, TRY_LATER], [503, TRY_LATER], + // A conflict or a lock the server's state can clear, and a request it calls too early. + [409, TRY_LATER], [423, TRY_LATER], [425, TRY_LATER], + // The server doesn't support the request, which later won't change. + [501, NOTHING_TO_DO], [505, NOTHING_TO_DO], + // This host's credentials for the server, which a fresh reference keeps. + [401, "Calling again won't change that; whoever runs this host can check its credentials for the server."], + [403, "Once a fresh reference to it arrives, fetching may work; if that's refused too, whoever runs this host can check its credentials for the server."], + // About this reference, which a fresh one may not share. + [404, FRESH_REFERENCE], [410, FRESH_REFERENCE], + ]; + for (const [status, remedy] of expect) { + const out = await f.fetch(makeRecord(reg, `${ORIGIN}/status/${status}`)); + assert.equal(out.error, `the "srv" server answered HTTP ${status}`, String(status)); + assert.equal(out.remedy, remedy, String(status)); + } + // A success with no body at all. + const empty = await f.fetch(makeRecord(reg, `${ORIGIN}/status/204`)); + assert.equal(empty.error, 'the "srv" server answered without any content'); + assert.equal(empty.remedy, FRESH_REFERENCE); +}); + +test('a fetch that meets no listener says the request failed, with its cause, and that a later try may work', async (t) => { + const reg = new ReferenceRegistry(); + // A port just freed refuses: fetch's "fetch failed" carries the refusal as its cause. + const freed = createServer(); + await new Promise((r) => freed.listen(0, '127.0.0.1', r)); + const deadPort = (freed.address() as { port: number }).port; + await new Promise((r) => freed.close(() => r())); + const f = makeFetcher({ dead: { url: `ws://127.0.0.1:${deadPort}` } }); + const out = await f.fetch(makeRecord(reg, `http://127.0.0.1:${deadPort}/x`, {}, 'dead')); + assert.equal(out.error, `the request to the "dead" server failed: connect ECONNREFUSED 127.0.0.1:${deadPort}`); + assert.equal(out.remedy, REQUEST_FAILED); + // A host with several addresses is refused at each, and its cause, an + // AggregateError, has no words of its own: each attempt's stand in. + t.mock.method(globalThis, 'fetch', async () => { + const attempts = [new Error('connect ECONNREFUSED ::1:4321'), new Error('connect ECONNREFUSED 127.0.0.1:4321')]; + throw new TypeError('fetch failed', { cause: Object.assign(new AggregateError(attempts), { code: 'ECONNREFUSED' }) }); + }); + const both = await makeFetcher({ dual: { url: 'ws://dual.example.invalid:4321' } }) + .fetch(makeRecord(reg, 'http://dual.example.invalid:4321/x', {}, 'dual')); + assert.equal(both.error, 'the request to the "dual" server failed: connect ECONNREFUSED ::1:4321; connect ECONNREFUSED 127.0.0.1:4321'); + assert.equal(both.remedy, REQUEST_FAILED); +}); + +test("a save that throws is the workspace's failure, logged for whoever runs the host, not the fetch's", async (t) => { + const errors = t.mock.method(console, 'error', () => {}); + const reg = new ReferenceRegistry(); + // Its words take one line of the log, bounded as the agent's text would be. + const f = new ReferenceFetcher(() => ({ url: ORIGIN }), async () => { throw new Error(`disk on fire\n${'x'.repeat(300)}`); }); + const rec = makeRecord(reg, `${ORIGIN}/ok?save-throws`); + const out = await f.fetch(rec); + assert.equal(out.error, "it was fetched, but this host couldn't store it in its workspace, so nothing was kept"); + assert.equal(out.remedy, 'Calling again is likely to fail the same way; whoever runs this host can see why in its log.'); + assert.deepEqual(errors.mock.calls.map((c) => String(c.arguments[0])), [`[refs] couldn't store ${rec.refId}.wav: disk on fire?${'x'.repeat(187)}…`]); + assert.equal(rec.fetchedPath, undefined); +}); + +test("a payload fetched but not stored says so, and the host's log says why: no workspace, no writable mount, or the write's error", async (t) => { + const errors = t.mock.method(console, 'error', () => {}); + type Answer = { result: { success: boolean; error?: string } }; + const cases: Array<[string, Record, (file: string) => string]> = [ + ['no workspace', { getWorkspaceModule: () => undefined }, (file) => `[refs] couldn't store ${file}: this host has no workspace`], + ['no writable mount', { getWorkspaceModule: () => ({}), firstWritableMountName: () => null }, + (file) => `[refs] couldn't store ${file}: no workspace mount is writable`], + ['a failed write', { + getWorkspaceModule: () => ({ writeBinary: async () => ({ success: false, error: 'EROFS: read-only file system' }) }), + firstWritableMountName: () => 'scratch', + }, (file) => `[refs] couldn't store scratch/refs/${file}: EROFS: read-only file system`], + ]; + for (const [what, workspace, logLine] of cases) { + // The framework's own fetcher and save hook, with this file's server as an MCPL server's address. + const framework = Object.create(AgentFramework.prototype) as { + pushEvent(event: unknown): void; + dispatchFetchReferenceToolCall(agentName: string, call: { id: string; name: string; input: unknown }): void; + }; + Object.assign(framework, { + mcplServerConfigs: new Map([['srv', { id: 'srv', url: `ws://127.0.0.1:${port}` }]]), + emitTrace: () => {}, + ...workspace, + }); + // Bytes no sniffer knows, with no type, so the file is saved as .bin. + const record = makeRecord(referenceRegistry, `${ORIGIN}/untyped?unstored=${encodeURIComponent(what)}`); + const answered = new Promise((resolve) => { framework.pushEvent = (event) => resolve(event as Answer); }); + framework.dispatchFetchReferenceToolCall('prime', { id: `call_${what}`, name: 'fetch_reference', input: { ref_id: record.refId } }); + const { result } = await answered; + assert.equal(result.error, `fetch_reference couldn't fetch "${record.refId}": it was fetched, but this host couldn't store it in its workspace, ` + + 'so nothing was kept. Calling again is likely to fail the same way; whoever runs this host can see why in its log.', what); + const logged = errors.mock.calls.map((c) => String(c.arguments[0])); + assert.ok(logged.includes(logLine(`${record.refId}.bin`)), `${what}: ${logged.join('\n')}`); + } +}); + +test("an address that doesn't parse is refused before anything is dialed: the reference's, or the server's, each saying which", async () => { + const reg = new ReferenceRegistry(); + const bad = await makeFetcher({ srv: { url: ORIGIN } }).fetch(makeRecord(reg, 'not a url at all')); + assert.equal(bad.error, "its address doesn't parse as a URL"); + assert.equal(bad.remedy, NOTHING_TO_DO); + const misconfigured = await makeFetcher({ srv: { url: 'not a url either' } }).fetch(makeRecord(reg, `${ORIGIN}/ok?misconfigured`)); + assert.equal(misconfigured.error, "the \"srv\" server's address doesn't parse as a URL, so there's nowhere to fetch it from"); + assert.equal(misconfigured.remedy, "Calling again won't change that unless whoever runs this host corrects the server's address."); + assert.equal(hits.get('/ok?misconfigured') ?? 0, 0, 'nothing dialed'); +}); + +test("past what's left of the server's budget, a download stopped at max_bytes is told the budget is the limit", async () => { + const reg = new ReferenceRegistry(); + const f = makeFetcher({ srv: { url: ORIGIN, autofetch: { maxTotalBytes: 1500 } } }); + // 1200 bytes arrive before the pause: past max_bytes, within the budget, + // which then has less left than what's known of the payload. + const out = await f.fetch(makeRecord(reg, `${ORIGIN}/drip?first=1200&rest=5000&wait=10000`), { maxBytes: 1000 }); + const [, received] = /^the download had reached (\d+) bytes, past the 1000 bytes this call allows, so it was stopped and nothing was kept$/.exec(out.error!) ?? []; + assert.ok(received, out.error); + assert.ok(Number(received) > 1000 && Number(received) <= 1200, received); + assert.equal(out.remedy, + `It's at least ${received} bytes, more than the ${1500 - Number(received)} bytes left of this host's fetch budget for the "srv" server. ${BUDGET}`); +}); diff --git a/test/save-recent-image.test.ts b/test/save-recent-image.test.ts index 3b7eec8b8..8993db243 100644 --- a/test/save-recent-image.test.ts +++ b/test/save-recent-image.test.ts @@ -582,7 +582,7 @@ describe('save_recent_image provenance (issue #104)', () => { const refused = await waitForToolResult(h.framework, 'call_save0'); assert.strictEqual(refused.isError, true); assert.match(refused.content, new RegExp(`index 0 is a reference \\(${expired.refId}, image/png, from`)); - assert.match(refused.content, /It can't be fetched either — expired\./); + assert.match(refused.content, /It can't be fetched either — the reference is expired\. Once a fresh reference to it arrives, fetching may work; until then, fetching it again won't\./); assert.ok(!refused.content.includes('fetch_reference'), refused.content); const keptRefusal = await waitForToolResult(h.framework, 'call_save1'); assert.strictEqual(keptRefusal.isError, true); @@ -628,7 +628,9 @@ describe('save_recent_image provenance (issue #104)', () => { const refused = await waitForToolResult(h.framework, 'call_save0'); assert.strictEqual(refused.isError, true); assert.match(refused.content, new RegExp(`index 0 is a reference \\(${unbound.refId}, image/png, from`)); - assert.ok(refused.content.includes(`It can't be fetched either — ${UNBOUND_REASON}.`), refused.content); + assert.ok(refused.content.includes( + `It can't be fetched either — it's ${UNBOUND_REASON}. Once a fresh reference to it arrives, fetching may work; until then, fetching it again won't.`, + ), refused.content); assert.ok(!refused.content.includes('fetch_reference'), refused.content); const boundRefusal = await waitForToolResult(h.framework, 'call_save1'); assert.strictEqual(boundRefusal.isError, true); @@ -682,13 +684,18 @@ describe('save_recent_image provenance (issue #104)', () => { }, }); try { - const budgetLasts = "the budget doesn't renew on its own, so calling again won't change that unless whoever runs this host raises or renews it"; + // The fetcher's clause, then what calling again can do, as fetch_reference would say them. + const nothing = "Calling again won't change that."; const expected = [ - ['call_save0', noAddress.refId, `It can't be fetched either — ${noAddressReason('stdio')}.`], - ['call_save1', offOrigin.refId, "It can't be fetched either — third-party origin (https://cdn.example.invalid != dialed https://example.invalid); not fetched by this host."], - ['call_save2', gone.refId, "It can't be fetched either — unknown server: gone."], - ['call_save3', big.refId, "It can't be fetched either — claimed 536870912 bytes exceeds fetch ceiling 268435456."], - ['call_save4', spent.refId, `It can't be fetched either — per-server fetch budget exhausted (0/0 bytes); ${budgetLasts}.`], + ['call_save0', noAddress.refId, `It can't be fetched either — ${noAddressReason('stdio')}. ${nothing}`], + ['call_save1', offOrigin.refId, "It can't be fetched either — it points to https://cdn.example.invalid, not to the \"vst\" server's own address " + + `(wss://example.invalid), and this host fetches only from the server itself. ${nothing}`], + ['call_save2', gone.refId, "It can't be fetched either — this host no longer has the \"gone\" server it came from. " + + "Calling again won't change that unless whoever runs this host adds that server back."], + ['call_save3', big.refId, "It can't be fetched either — it claims 536870912 bytes, over the 268435456 bytes this host fetches at most. " + + "No max_bytes or setting lifts that limit, so calling again won't change it."], + ['call_save4', spent.refId, "It can't be fetched either — this host's fetch budget for the \"spent\" server is used up (0 of 0 bytes). " + + "The budget doesn't renew on its own, so calling again won't change that unless whoever runs this host raises or renews it."], ] as const; for (const [call, refId, reason] of expected) { const refused = await waitForToolResult(h.framework, call); diff --git a/test/tool-result-reference-uri.test.ts b/test/tool-result-reference-uri.test.ts index c16402d29..6f9f82dbb 100644 --- a/test/tool-result-reference-uri.test.ts +++ b/test/tool-result-reference-uri.test.ts @@ -337,6 +337,6 @@ describe('a script given such data', () => { } assert.ok(!text.includes(SIGNATURE), text); assert.match(text, /^zz-text\n\[ref_[^\]]+\] zz\.pdf — application\/pdf — from tool result/); - assert.ok(logged.some((line) => /\[rfc005\] script-demand fetch of ref_\S+ failed: reference not tied to any server, so there's nowhere to fetch it from/.test(line)), logged.join('\n')); + assert.ok(logged.some((line) => /\[rfc005\] script-demand fetch of ref_\S+ failed: it's not tied to any server, so there's nowhere to fetch it from$/.test(line)), logged.join('\n')); }); });