diff --git a/changelog/unreleased/1583.md b/changelog/unreleased/1583.md new file mode 100644 index 0000000000..147fb637aa --- /dev/null +++ b/changelog/unreleased/1583.md @@ -0,0 +1,4 @@ +- `mcp/cas`: `cas_get`'s unreachable `fromVec`/`base64Encode` null-checks + become `assertNotNullish`; adds proof coverage for `cas_add`'s + `writeBytes`-failure race and `cas_get`'s hash-vanished-between-reads + races, bringing the module to 100% line/branch/function coverage diff --git a/fjs/mcp/cas/module.f.mjs b/fjs/mcp/cas/module.f.mjs index 3468ed6e21..49d064702e 100644 --- a/fjs/mcp/cas/module.f.mjs +++ b/fjs/mcp/cas/module.f.mjs @@ -134,6 +134,7 @@ import { identity } from '../../types/function/module.f.mjs' import { sha256 } from '../../crypto/sha2/module.f.mjs' import { nonEmpty, empty as elEmpty } from '../../effects/list/module.f.mjs' import { syncRevision } from '../../cas/evo/module.f.mjs' +import { assertNotNullish } from '../../asserts/module.f.mjs' // ── Argument schemas (declared once, used for both inputSchema and validate) ───── @@ -269,19 +270,21 @@ export const casToolRegistry = home => cacheKey => { /** @type {_Meta} */ const refinedMeta = { length: Number(refined.length), mimeType: refined.mime_type, type: refined.type, uri } if (refined.type === 'text') { - // `type: 'text'` means the detector validated `value` as UTF-8, - // so `fromVec` is non-null here; guard defensively regardless. - const str = fromVec(value) - return pure(str === null - ? errorResult(`content is not byte-aligned: ${r.hash}`) - : okResult(toJson({ ...refinedMeta, text: str })) - ) + // `type: 'text'` means the detector validated `value` as + // whole-blob UTF-8 with a byte-aligned length (see + // `media/type`'s `finish`) — the same two conditions + // `fromVec` checks, via the same decoder — so `fromVec` + // cannot return `null` here (mirrors `media`'s own `detect`). + const str = assertNotNullish(fromVec(value), 'cas_get: type text implies fromVec succeeds') + return pure(okResult(toJson({ ...refinedMeta, text: str }))) } - const blob = base64Encode(value) - return pure(blob === null - ? errorResult(`content is not byte-aligned: ${r.hash}`) - : okResult(toJson({ ...refinedMeta, blob })) - ) + // Every byte ever written through `cas_add`/the CAS store is + // whole-byte chunks (UTF-8 text or already-decoded base64), so + // `value` is always byte-aligned regardless of which branch + // classified it — `base64Encode` only rejects a non-byte-aligned + // input. + const blob = assertNotNullish(base64Encode(value), 'cas_get: stored content is always byte-aligned') + return pure(okResult(toJson({ ...refinedMeta, blob }))) }, ) }, diff --git a/fjs/mcp/cas/proof.f.mjs b/fjs/mcp/cas/proof.f.mjs new file mode 100644 index 0000000000..eb59ecb789 --- /dev/null +++ b/fjs/mcp/cas/proof.f.mjs @@ -0,0 +1,157 @@ +/** + * @import { Effect } from '../../effects/types.ts' + * @import { FileCasOperation } from '../../cas/types.ts' + * @import { MemOp } from '../../effects/memory/types.ts' + * @import { Cache } from '../../cas/evo/types.ts' + * @import { Key } from '../../effects/memory/types.ts' + * @import { ToolsCallResult } from '../../protocol/mcp/types.ts' + */ + +import { casToolRegistry } from './module.f.mjs' +import { match } from '../../effects/module.f.mjs' +import { error, ok } from '../../types/result/module.f.mjs' +import { vec, vec8 } from '../../types/bit_vec/module.f.mjs' +import { vecToCBase32 } from '../../basen/cbase32/module.f.mjs' +import { assert, assertEq } from '../../asserts/module.f.mjs' +import { number as rttiNumber, string as rttiString } from '../../types/rtti/module.f.mjs' +import { parse as rttiParse } from '../../types/rtti/parse/module.f.mjs' +import { unwrap } from '../../types/result/module.f.mjs' +import { parse as parseJson } from '../../media/json/module.f.mjs' + +// A harmless "always succeeds" response for a command, used by `drive` once a +// test's overrides for that command are exhausted — same technique as +// `fjs/cas/proof.f.mjs`'s `drive`, extended with `MemOp` since `cas_add` also +// touches the Evo cache on its success path. +/** @type {(cmd: string) => unknown} */ +const defaultResponse = cmd => { + switch (cmd) { + case 'now': return 0 + case 'randomInt': return 0 + case 'mkdir': case 'createExclusive': case 'rename': case 'rm': + case 'writeBytes': case 'access': + return ok(undefined) + case 'readdir': return ok(/** @type {readonly unknown[]} */ ([])) + case 'stat': return ok({ size: 0 }) + // An empty chunk reads as end-of-stream, so a `readBytes` call with no + // override reads as an immediately-empty file by default. + case 'readBytes': return ok(vec(0n)(0n)) + case 'memCreate': return /** @type {unknown} */ ('mem-key') + case 'memRead': return undefined + case 'memWrite': return undefined + default: return ok(undefined) + } +} + +/** + * Drives a `FileCasOperation | MemOp` effect to completion with synthetic op + * responses instead of a filesystem. `overrides[cmd]` is a queue consumed in + * call order; once a command's queue is empty (or was never given), + * `defaultResponse` supplies an always-succeeds value. + * + * `cas_add`'s write-failure branch and `cas_get`'s "hash vanished between + * reads" branches are real only under a race (a failing disk, a concurrent + * writer, a GC sweep) between two of the tool's own steps — the same shape of + * branch `fjs/cas/proof.f.mjs` reaches with its own `drive` helper, applied + * here one layer up at the MCP tool boundary. + * + * @type {(overrides: Partial>) => (e: Effect) => unknown} + */ +const drive = overrides => { + /** @type {(cmd: string) => unknown} */ + const next = cmd => { + const queue = overrides[cmd] + return queue !== undefined && queue.length > 0 ? queue.shift() : defaultResponse(cmd) + } + const handlers = /** @type {Parameters[0]} */ ({ + access: () => next('access'), + createExclusive: () => next('createExclusive'), + mkdir: () => next('mkdir'), + now: () => next('now'), + randomInt: () => next('randomInt'), + readBytes: () => next('readBytes'), + readdir: () => next('readdir'), + rename: () => next('rename'), + rm: () => next('rm'), + stat: () => next('stat'), + writeBytes: () => next('writeBytes'), + memCreate: () => next('memCreate'), + memRead: () => next('memRead'), + memWrite: () => next('memWrite'), + }) + const matcher = match(handlers) + /** @type {(e: Effect) => unknown} */ + const run_ = e => { + const m = matcher(e) + return m[0] === 'done' ? m[1] : run_(m[2](/** @type {any} */ (m[1]))) + } + return run_ +} + +// `syncRevision` is only reached on a *successful* write, which none of these +// cases exercise, so the cache key's actual identity never matters — only its +// type does. +const cacheKey = /** @type {Key} */ (/** @type {any} */ ('unused-cache-key')) + +const registry = casToolRegistry('.')(cacheKey) + +/** @type {(name: string) => (args: any) => Effect} */ +const toolHandle = name => { + const entry = registry.find(t => t.name === name) + assert(entry !== undefined, `no such tool: ${name}`) + return /** @type {NonNullable} */ (entry).handle +} + +// Any well-formed cBase32 hash works: none of these cases resolve it against a +// real store, since every filesystem op is driven synthetically. +const someHash = vecToCBase32(vec(256n)(0n)) + +const meta = /** @type {const} */ ({ + length: rttiNumber, + mimeType: rttiString, + type: rttiString, + uri: rttiString, +}) +const parseMeta = rttiParse(meta) + +export const proof = { + // cas_add: a writeBytes failure mid-upload (disk full, permissions) is + // reported as a tool-level error, not a thrown exception or a silent + // success. + casAddWriteErrorReturnsError: () => { + const result = /** @type {ToolsCallResult} */ ( + drive({ writeBytes: [error('disk full')] })(toolHandle('cas_add')({ content: 'hello' })) + ) + assert(result.isError === true, ['expected isError', result]) + }, + // cas_get, content:false: the streaming metadata pass finds a small + // whole-blob-text hash, so it starts a second, independent read to refine + // `mimeType` via the dialect-aware detector. If that second read finds the + // hash gone (a GC sweep raced it away), the code falls back to the + // streaming verdict rather than fail the whole request. + casGetMetadataRefineHashVanishesFallsBackToStreamingVerdict: () => { + const result = /** @type {ToolsCallResult} */ ( + drive({ readBytes: [ok(vec8(0x41n)), ok(vec(0n)(0n)), error('vanished')] }) + (toolHandle('cas_get')({ hash: someHash, content: false })) + ) + assert(result.isError !== true, ['expected ok result', result]) + const text = result.content[0] + assert(text.type === 'text', ['expected text content', text]) + const parsed = unwrap(parseMeta(unwrap(parseJson(text.text)))) + assertEq(parsed.type, 'text') + assertEq(parsed.mimeType, 'text/plain') + assertEq(parsed.length, 1) + }, + // cas_get, content:true: the streaming metadata pass succeeds and the blob + // fits inline, so a second read materializes it. If that second read finds + // the hash gone, the whole request fails — there is no streaming verdict + // to fall back to once inline content was actually promised. + casGetContentFetchHashVanishesReturnsError: () => { + const result = /** @type {ToolsCallResult} */ ( + drive({ readBytes: [ok(vec8(0x41n)), ok(vec(0n)(0n)), error('vanished')] }) + (toolHandle('cas_get')({ hash: someHash, content: true })) + ) + assert(result.isError === true, ['expected isError', result]) + const text = result.content[0] + assert(text.type === 'text' && text.text.includes('no such hash'), ['expected no-such-hash message', text]) + }, +}