diff --git a/changelog/unreleased/1720.md b/changelog/unreleased/1720.md new file mode 100644 index 000000000..ebd60c46c --- /dev/null +++ b/changelog/unreleased/1720.md @@ -0,0 +1,6 @@ +- `protocol/json_rpc`: `errorResponseOf` is now exported and joined by + `successResponseOf`, so protocols layered on this module build response + envelopes with the owner's constructors instead of their own. +- `protocol/json_rpc/types`: new `SuccessResponse` and `ErrorResponse` — the two + branches of the `Response` union, for a caller that holds one and wants + `.result` or `.error` without an `in` check. diff --git a/fjs/effects/todo/map-step-combinator.md b/fjs/effects/todo/map-step-combinator.md index 32813c2f7..31170eccc 100644 --- a/fjs/effects/todo/map-step-combinator.md +++ b/fjs/effects/todo/map-step-combinator.md @@ -23,10 +23,10 @@ Two shapes, both the same thing: *Projection* (`x => pure(f(x))`): ```ts -// fjs/protocol/mcp/module.f.mjs:347-350 +// fjs/protocol/mcp/module.f.mjs:411-414 : step( handlers.toolsList(pr), - r => pure(_okResponse(id)(r)), + r => pure(successResponseOf(id)(r)), ) ``` diff --git a/fjs/protocol/json_rpc/module.f.mjs b/fjs/protocol/json_rpc/module.f.mjs index cdc68a508..4fa9157a9 100644 --- a/fjs/protocol/json_rpc/module.f.mjs +++ b/fjs/protocol/json_rpc/module.f.mjs @@ -15,7 +15,7 @@ * @module * * @import { Unknown } from '../../media/json/types.ts' - * @import { Id, RpcError, Handlers, Response } from './types.ts' + * @import { Id, RpcError, Handlers, Response, SuccessResponse, ErrorResponse } from './types.ts' */ import { at } from '../../types/object/module.f.mjs' @@ -78,8 +78,39 @@ export const methodNotFound = rpcError(-32601)('Method not found') export const invalidParams = rpcError(-32602)('Invalid params') export const internalError = rpcError(-32603)('Internal error') -/** @type {(id: Id) => (error: RpcError) => Response} */ -const errorResponseOf = id => error => ({ jsonrpc, error, id }) +/** + * The error half of the response envelope: `{ jsonrpc, error, id }`. + * + * Exported as one of a pair with {@link successResponseOf} — the `Response` + * schema, `jsonrpc`, `Id` and `RpcError` are all owned here, so the two shapes + * built from them are too. Every protocol layered on this module needs both + * (`fjs/protocol/mcp` and its stdio transport are the two consumers today), + * and a private constructor is what made each of them re-roll its own. + * + * It answers the `Response` union rather than {@link ErrorResponse}, the branch + * it always builds. That is deliberate: every consumer in the tree is a + * dispatcher answering either arm — `dispatch` here, `mcpStep` and the stdio + * transport in `fjs/protocol/mcp` — and `Handle` is defined in terms of the + * union, so the branch type would have to be widened again at each of them. + * {@link ErrorResponse} is exported for a caller that does want it. + * + * @type {(id: Id) => (error: RpcError) => Response} + */ +export const errorResponseOf = id => error => ({ jsonrpc, error, id }) + +/** + * The success half of the response envelope: `{ jsonrpc, result, id }`. + * + * The `…Of` suffix pairs with {@link errorResponseOf}, and both name the + * already-exported `successResponse` / `errorResponse` schemas they build a + * value of. + * + * It answers the union for the reason {@link errorResponseOf} does, and + * {@link SuccessResponse} names the branch for a caller that wants it. + * + * @type {(id: Id) => (result: Unknown) => Response} + */ +export const successResponseOf = id => result => ({ jsonrpc, result, id }) /** * Dispatches an already-parsed JSON-RPC value against `handlers`. @@ -110,6 +141,6 @@ export const dispatch = handlers => value => { } const [t2, result] = handler(params) return t2 === 'ok' - ? { jsonrpc, result, id } + ? successResponseOf(id)(result) : errorResponseOf(id)(result) } diff --git a/fjs/protocol/json_rpc/proof.f.mjs b/fjs/protocol/json_rpc/proof.f.mjs index 5adf83cd9..54dfe3437 100644 --- a/fjs/protocol/json_rpc/proof.f.mjs +++ b/fjs/protocol/json_rpc/proof.f.mjs @@ -15,6 +15,9 @@ import { methodNotFound, invalidParams, internalError, + response, + errorResponseOf, + successResponseOf, } from './module.f.mjs' /** @type {(r: readonly [string, unknown]) => boolean} */ @@ -102,4 +105,30 @@ export const proof = { assertEq(r, null) }, }, + + // The two envelope constructors are public API, so they are proven + // directly rather than only through `dispatch`. Each result is parsed back + // with this module's own `response` schema, which is the fact worth + // pinning: the constructor and the schema describing the shape it builds + // cannot drift apart. + responseOf: { + success: () => { + const r = successResponseOf(1)('pong') + assertEq(r.jsonrpc, '2.0') + assert('result' in r && r.result === 'pong', r) + assertEq(r.id, 1) + assert(isOk(parse(response)(r)), r) + }, + // A non-`null` id on purpose: an `id` forced to `null` is a mutation + // this case would not see if it asked for `null` to begin with, and a + // string exercises the other arm of `Id` besides. `errorResponseOf`'s + // `null` id is covered by `dispatch.invalidRequest`. + error: () => { + const r = errorResponseOf('abc')(parseError) + assertEq(r.jsonrpc, '2.0') + assert('error' in r && r.error.code === -32700, r) + assertEq(r.id, 'abc') + assert(isOk(parse(response)(r)), r) + }, + }, } diff --git a/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md b/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md index 6b6659478..1e5766fc7 100644 --- a/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md +++ b/fjs/protocol/json_rpc/todo/effectful-dispatch-skeleton.md @@ -20,11 +20,11 @@ const handler = at(method)(handlers) if (handler === null) { return errorResponseOf(id)(methodNotFound) } ``` -Effectful `mcpStep` (`fjs/protocol/mcp/module.f.mjs:264-287`): +Effectful `mcpStep` (`fjs/protocol/mcp/module.f.mjs:314-346`): ```js const [t, message] = decodeRequest(value) -if (t === 'error') { return pure(_errResponse(null)(invalidRequest)) } +if (t === 'error') { return pure(errorResponseOf(null)(invalidRequest)) } const { id, method, params } = message if (id === undefined) { if (method === 'notifications/initialized') { ... } @@ -78,6 +78,7 @@ fold into the 66D envelope work if it touches the same lines anyway. ## Related -- `fjs/protocol/json_rpc/todo/response-constructors.md` — the envelope - *constructors*; this issue is the envelope *routing*. Complementary. +- `errorResponseOf` / `successResponseOf` (`../module.f.mjs`) — the envelope + *constructors*, exported from this module; this issue is the envelope + *routing*. Complementary, and the skeleton below builds on them. - `fjs/protocol/mcp/todo/README.md` (66D) — per-method validate/response arms. diff --git a/fjs/protocol/json_rpc/todo/response-constructors.md b/fjs/protocol/json_rpc/todo/response-constructors.md deleted file mode 100644 index c1da2cd24..000000000 --- a/fjs/protocol/json_rpc/todo/response-constructors.md +++ /dev/null @@ -1,101 +0,0 @@ -## `json/rpc`: own and export the response-envelope constructors - -**Priority:** P4 -**Status:** open - -### Problem - -Building a JSON-RPC response envelope — `{ jsonrpc, result, id }` on success, -`{ jsonrpc, error, id }` on failure — is a `json/rpc` concern: the module owns -`jsonrpc`, `Id`, `RpcError`, and the `Response` schema. Yet the constructors -for those two shapes exist in **two modules**, once privately here and once -re-rolled downstream: - -```ts -// fjs/protocol/json_rpc/module.f.mjs:81 — private -const errorResponseOf = (id: Id) => (error: RpcError): Response => - ({ jsonrpc, error, id }) -// fjs/protocol/json_rpc/module.f.mjs:110 — success shape inlined in dispatch - ? { jsonrpc, result, id } - -// fjs/protocol/mcp/module.f.mjs:220-224 — the same two constructors, rebuilt -const _errResponse = (id: Id) => (error: RpcError): Response => - ({ jsonrpc, error, id }) -const _okResponse = (id: Id) => (result: Unknown): Response => - ({ jsonrpc, result, id }) - -// fjs/protocol/mcp/stdio/module.f.mjs:57,62 — third and fourth copies of the error envelope -const parseErrorResponse: Response = { jsonrpc, error: parseError, id: null } -const internalErrorResponse = (id: Response['id']): Response => ({ jsonrpc, error: internalError, id }) -``` - -(`parseErrorResponse` is `errorResponseOf(null)(parseError)` evaluated once; -`internalErrorResponse` is `errorResponseOf` specialized to `internalError` — -both collapse onto the exported constructor.) - -`mcp` already imports `jsonrpc`, `rpcError`, `invalidRequest`, `invalidParams`, -and `methodNotFound` from `json/rpc` — the error *values* travel across the -module boundary, but the envelope *constructors* had to be reinvented because -they are private. That is the same separation-of-concerns smell already fixed -for `ToolsCallResult` (`okResult`/`errorResult` now live in `mcp` and -`cas/mcp` imports `okResult` instead of hand-rolling it) one layer down: the -owner module exports half of a pair and every consumer hand-rolls the rest. -`_errResponse` is byte-identical to `errorResponseOf`; `_okResponse` is the -inlined success literal from `dispatch` given a name. Both are used throughout -`mcpStep` (8+ call sites) and will be used by every future protocol built on -`json/rpc`. - -Per `AGENTS.md`: "When a sibling module already has the type or helper you -need, import it — add `export` to the existing declaration if it's not yet -exported, rather than duplicating it." Two real consumers exist (`dispatch` -itself and `mcpStep`), so the extraction is past the second-consumer bar. - -### Proposal - -Export both constructors from `fjs/protocol/json_rpc/module.f.mjs` and consume them in -`dispatch` and `mcp`: - -```ts -// fjs/protocol/json_rpc/module.f.mjs -export const errorResponseOf = (id: Id) => (error: RpcError): Response => - ({ jsonrpc, error, id }) - -export const successResponseOf = (id: Id) => (result: Unknown): Response => - ({ jsonrpc, result, id }) -``` - -- Naming: the schemas `successResponse`/`errorResponse` are already exported, - so the constructors take the `…Of` suffix that `errorResponseOf` already - uses. Keep the pair's names parallel. -- `dispatch` replaces its inline `{ jsonrpc, result, id }` with - `successResponseOf(id)(result)`. -- `fjs/protocol/mcp/module.f.mjs` deletes `_errResponse`/`_okResponse` and imports the - exported pair; no behavior change. -- Future JSON-RPC-based servers (the `resources/*`, `prompts/*`, `logging/*` - methods from [i665-mcp](../../mcp/todo/README.md)) get the constructors - for free instead of copying them a third time. - -This composes cleanly with -[66D-mcp-validate-response-envelope](../../mcp/todo/README.md): that -issue's `validated` helper is defined *in terms of* the error constructor; -after this change it builds on the imported one. - -### Tasks - -- [ ] Export `errorResponseOf`; add and export `successResponseOf` in - `fjs/protocol/json_rpc/module.f.mjs`; use it in `dispatch`. -- [ ] Replace `_errResponse`/`_okResponse` in `fjs/protocol/mcp/module.f.mjs` with the - imported constructors. -- [ ] Rebuild `parseErrorResponse`/`internalErrorResponse` in - `fjs/protocol/mcp/stdio/module.f.mjs` on the imported `errorResponseOf`. -- [ ] `npx tsc` clean; `fjs t` passes (`fjs/protocol/json_rpc/proof.f.mjs`, - `fjs/protocol/mcp/proof.f.mjs`). - -### Related - -- `fjs/protocol/json_rpc/module.f.mjs` — owner of the `Response` envelope. -- `fjs/protocol/mcp/module.f.mjs:220-224` — the duplicated private constructors. -- `fjs/protocol/mcp/module.f.mjs` `okResult`/`errorResult` — the same - owner-exports-the-pair pattern, already applied to `ToolsCallResult`. -- [66D-mcp-validate-response-envelope](../../mcp/todo/README.md) — builds - its `validated` helper on top of these constructors. diff --git a/fjs/protocol/json_rpc/types.ts b/fjs/protocol/json_rpc/types.ts index eb7e2b59b..3ea1fbcbc 100644 --- a/fjs/protocol/json_rpc/types.ts +++ b/fjs/protocol/json_rpc/types.ts @@ -8,7 +8,9 @@ import type { Unknown } from '../../media/json/types.ts' import type { Result } from '../../types/result/types.ts' -import type { _id, request, error as errorSchema, response } from './module.f.mjs' +import type { + _id, request, error as errorSchema, response, successResponse, errorResponse, +} from './module.f.mjs' import type { Ts } from '../../types/rtti/ts/types.ts' export type Id = Ts @@ -26,6 +28,18 @@ export type RpcError = Ts */ export type Response = Ts +/** + * The success branch of {@link Response}: `result` is present, `error` is not. + * + * `successResponseOf` answers the `Response` union, not this — every consumer + * in the tree is a dispatcher that returns either arm. Name this where a + * caller genuinely holds one branch and wants `.result` without an `in` check. + */ +export type SuccessResponse = Ts + +/** The error branch of {@link Response}, the mirror of {@link SuccessResponse}. */ +export type ErrorResponse = Ts + /** A method implementation: maps `params` to a result or an `RpcError`. */ export type Handler = (params: Unknown | undefined) => Result diff --git a/fjs/protocol/mcp/module.f.mjs b/fjs/protocol/mcp/module.f.mjs index 69beced9f..7dc21de1a 100644 --- a/fjs/protocol/mcp/module.f.mjs +++ b/fjs/protocol/mcp/module.f.mjs @@ -17,7 +17,7 @@ * @import { Ts } from '../../types/rtti/ts/types.ts' * @import { Effect, Operation } from '../../effects/types.ts' * @import { Key, MemOp } from '../../effects/memory/types.ts' - * @import { Response, Id, RpcError } from '../json_rpc/types.ts' + * @import { Response } from '../json_rpc/types.ts' * @import { Type } from '../../types/rtti/types.ts' * @import { Implementation, ServerCapabilities, InitializeResult, Tool, ToolsListParams, ToolsCallResult, McpHandlers, ToolEntry, McpSessionState, McpConfig, ProtocolVersions } from './types.ts' */ @@ -29,7 +29,7 @@ import { read, write } from '../../effects/memory/module.f.mjs' import { decodeRequest, rpcError, internalError, invalidRequest, invalidParams, methodNotFound, - jsonrpc, + errorResponseOf, successResponseOf, } from '../json_rpc/module.f.mjs' import { parse } from '../../types/rtti/parse/module.f.mjs' import { toJsonSchema } from '../../media/json/schema/module.f.mjs' @@ -250,12 +250,6 @@ export const fromRegistry = registry => ({ // ── Lifecycle / capability state machine ─────────────────────────────────────── -/** @type {(id: Id) => (error: RpcError) => Response} */ -const _errResponse = id => error => ({ jsonrpc, error, id }) - -/** @type {(id: Id) => (result: Unknown) => Response} */ -const _okResponse = id => result => ({ jsonrpc, result, id }) - /** MCP error -32002: the client called a method before `initialize`. */ export const notInitialized = rpcError(-32002)('Server not initialized') @@ -319,7 +313,7 @@ export const mcpStep = ({ value => { const [t, message] = decodeRequest(value) if (t === 'error') { - return pureOk(_errResponse(null)(invalidRequest)) + return pureOk(errorResponseOf(null)(invalidRequest)) } const { id, method, params } = message @@ -356,8 +350,8 @@ export const mcpStep = ({ if (method === 'ping') { const [pt] = parse(_noParams)(params) return pt === 'error' - ? pureOk(_errResponse(id)(invalidParams)) - : pureOk(_okResponse(id)({})) + ? pureOk(errorResponseOf(id)(invalidParams)) + : pureOk(successResponseOf(id)({})) } // `initialize` transitions uninitialized → initializing; reject if already done. @@ -365,13 +359,13 @@ export const mcpStep = ({ return resultStep( read(stateKey), r => { - if (r[0] === 'error') { return pureOk(_errResponse(id)(internalError)) } + if (r[0] === 'error') { return pureOk(errorResponseOf(id)(internalError)) } if (r[1][0] !== 'uninitialized') { - return pureOk(_errResponse(id)(invalidRequest)) + return pureOk(errorResponseOf(id)(invalidRequest)) } const [pr, pv] = parse(initializeParams)(params) if (pr === 'error') { - return pureOk(_errResponse(id)(invalidParams)) + return pureOk(errorResponseOf(id)(invalidParams)) } /** @type {InitializeResult} */ const result = { @@ -387,8 +381,8 @@ export const mcpStep = ({ return resultStep( write(stateKey, ['initializing']), w => pureOk(w[0] === 'error' - ? _errResponse(id)(internalError) - : _okResponse(id)(result)), + ? errorResponseOf(id)(internalError) + : successResponseOf(id)(result)), ) }, ) @@ -401,39 +395,39 @@ export const mcpStep = ({ return resultStep( read(stateKey), r => { - if (r[0] === 'error') { return pureOk(_errResponse(id)(internalError)) } + if (r[0] === 'error') { return pureOk(errorResponseOf(id)(internalError)) } if (r[1][0] !== 'initialized') { - return pureOk(_errResponse(id)(notInitialized)) + return pureOk(errorResponseOf(id)(notInitialized)) } if (method === 'tools/list') { if (capabilities.tools === undefined) { - return pureOk(_errResponse(id)(methodNotFound)) + return pureOk(errorResponseOf(id)(methodNotFound)) } // `params` may be absent — `tools/list` without a cursor. const [t, pr] = parse(toolsListParams)(params === undefined ? {} : params) return t === 'error' - ? pureOk(_errResponse(id)(invalidParams)) + ? pureOk(errorResponseOf(id)(invalidParams)) : ioStep( handlers.toolsList(pr), - r => pureOk(_okResponse(id)(r)), + r => pureOk(successResponseOf(id)(r)), ) } if (method === 'tools/call') { if (capabilities.tools === undefined) { - return pureOk(_errResponse(id)(methodNotFound)) + return pureOk(errorResponseOf(id)(methodNotFound)) } const [t, pr] = parse(toolsCallParams)(params) return t === 'error' - ? pureOk(_errResponse(id)(invalidParams)) + ? pureOk(errorResponseOf(id)(invalidParams)) : ioStep( handlers.toolsCall(pr), - r => pureOk(_okResponse(id)(r)), + r => pureOk(successResponseOf(id)(r)), ) } - return pureOk(_errResponse(id)(methodNotFound)) + return pureOk(errorResponseOf(id)(methodNotFound)) }, ) } diff --git a/fjs/protocol/mcp/stdio/module.f.mjs b/fjs/protocol/mcp/stdio/module.f.mjs index c3d4f972e..c3f7dcf90 100644 --- a/fjs/protocol/mcp/stdio/module.f.mjs +++ b/fjs/protocol/mcp/stdio/module.f.mjs @@ -41,19 +41,18 @@ import { ioError, readLine, write } from '../../../effects/node/module.f.mjs' import { tryUtf8 } from '../../../text/module.f.mjs' import { parse, stringify } from '../../../media/json/module.f.mjs' import { sort } from '../../../types/object/module.f.mjs' -import { internalError, jsonrpc, parseError } from '../../json_rpc/module.f.mjs' +import { errorResponseOf, internalError, parseError } from '../../json_rpc/module.f.mjs' import { error } from '../../../types/result/module.f.mjs' const stringifyJson = stringify(sort) /** The parse-error response (`-32700`, `id: null`) for a malformed input line. */ -/** @type {Response} */ -const parseErrorResponse = { jsonrpc, error: parseError, id: null } +const parseErrorResponse = errorResponseOf(null)(parseError) /** An internal-error response (`-32603`) carrying `id`. * @type {(id: Response['id']) => Response} */ -const internalErrorResponse = id => ({ jsonrpc, error: internalError, id }) +const internalErrorResponse = id => errorResponseOf(id)(internalError) /** Encodes a response as a newline-terminated UTF-8 line and writes it to `stdout`. * @type {(resp: Response) => Effect} diff --git a/fjs/protocol/mcp/todo/README.md b/fjs/protocol/mcp/todo/README.md index 77b4271be..063ea865d 100644 --- a/fjs/protocol/mcp/todo/README.md +++ b/fjs/protocol/mcp/todo/README.md @@ -7,9 +7,14 @@ > **Quotes refreshed against the current handler (2026-08).** `mcpStep` has been > restructured since this was written — `validate` → `parse`, `pure` → `pureOk`, -> the `.step` method → the standalone `ioStep`, and a lifecycle `resultStep(read( -> stateKey), …)` now wraps `initialize` and the `tools/*` pair. The repetition -> survived all of it: the code below is the handler as it stands. +> the `.step` method → the standalone `ioStep`, a lifecycle `resultStep(read( +> stateKey), …)` now wraps `initialize` and the `tools/*` pair, and the private +> `_errResponse` / `_okResponse` wrappers are gone in favour of `json_rpc`'s +> exported `errorResponseOf` / `successResponseOf`. The repetition survived all +> of it: the code below is the handler as it stands. +> +> **Build the helper on the imported constructors**, not on new local wrappers — +> reintroducing a private pair is what this module just stopped doing. ### Problem @@ -20,7 +25,7 @@ every method arm. The shape is always: ```ts const [t, pr] = parse()() return t === 'error' - ? pureOk(_errResponse(id)(invalidParams)) + ? pureOk(errorResponseOf(id)(invalidParams)) : ``` @@ -28,14 +33,14 @@ It appears four times in `mcpStep` (in `fjs/protocol/mcp/module.f.mjs`), and the four do not even spell the destructure the same way: - `ping` — `parse(_noParams)(params)` as `[pt]`, success is - `pureOk(_okResponse(id)({}))`. + `pureOk(successResponseOf(id)({}))`. - `initialize` — `parse(initializeParams)(params)` as `[pr, pv]`, success builds an `InitializeResult` and writes state; the whole arm sits inside a `resultStep(read(stateKey), …)` that rejects a non-`uninitialized` session. - `tools/list` — `parse(toolsListParams)(params === undefined ? {} : params)` as - `[t, pr]`, success is `ioStep(handlers.toolsList(pr), r => pureOk(_okResponse(id)(r)))`. + `[t, pr]`, success is `ioStep(handlers.toolsList(pr), r => pureOk(successResponseOf(id)(r)))`. - `tools/call` — `parse(toolsCallParams)(params)` as `[t, pr]`, success is - `ioStep(handlers.toolsCall(pr), r => pureOk(_okResponse(id)(r)))`. + `ioStep(handlers.toolsCall(pr), r => pureOk(successResponseOf(id)(r)))`. A fifth site, `notifications/initialized`, runs `parse(_noParams)(params)` but answers `pureOk(null)` on either branch, since a notification never gets a @@ -52,21 +57,21 @@ handler runs: ```ts // tools/list if (capabilities.tools === undefined) { - return pureOk(_errResponse(id)(methodNotFound)) + return pureOk(errorResponseOf(id)(methodNotFound)) } const [t, pr] = parse(toolsListParams)(params === undefined ? {} : params) return t === 'error' - ? pureOk(_errResponse(id)(invalidParams)) - : ioStep(handlers.toolsList(pr), r => pureOk(_okResponse(id)(r))) + ? pureOk(errorResponseOf(id)(invalidParams)) + : ioStep(handlers.toolsList(pr), r => pureOk(successResponseOf(id)(r))) // tools/call if (capabilities.tools === undefined) { - return pureOk(_errResponse(id)(methodNotFound)) + return pureOk(errorResponseOf(id)(methodNotFound)) } const [t, pr] = parse(toolsCallParams)(params) return t === 'error' - ? pureOk(_errResponse(id)(invalidParams)) - : ioStep(handlers.toolsCall(pr), r => pureOk(_okResponse(id)(r))) + ? pureOk(errorResponseOf(id)(invalidParams)) + : ioStep(handlers.toolsCall(pr), r => pureOk(successResponseOf(id)(r))) ``` Both now sit inside one `resultStep(read(stateKey), …)` that has already @@ -74,7 +79,7 @@ rejected an unreadable state (`internalError`) and a non-`initialized` one (`notInitialized`), so the capability gate is the only per-method guard left above the envelope. -The repeated `t === 'error' ? pureOk(_errResponse(id)(invalidParams)) : …` +The repeated `t === 'error' ? pureOk(errorResponseOf(id)(invalidParams)) : …` envelope forces a reader to diff each arm to confirm the only thing that varies is the schema and the success branch — exactly the readability cost AGENTS.md calls out: "When two code branches share most of their structure, refactor so @@ -84,7 +89,7 @@ the shared part appears once and only the difference lives in the conditional." 1. **A `validated` helper** that captures the `id`-bound `invalidParams` failure and forwards the decoded value to a success continuation. Because `id`, - `params`, and the `_errResponse`/`invalidParams` pair are per-request, the + `params`, and the `errorResponseOf`/`invalidParams` pair are per-request, the helper lives inside `mcpStep` (it genuinely closes over `id`), or — preferred per the "thread context rather than close over locals" rule — at module scope taking `id` as a parameter: @@ -94,7 +99,7 @@ the shared part appears once and only the difference lives in the conditional." (onOk: (value: Ts) => Effect) => { const [t, pr] = parse(schema)(params) return t === 'error' - ? pureOk(_errResponse(id)(invalidParams)) + ? pureOk(errorResponseOf(id)(invalidParams)) : onOk(/** @type {Ts} */ (pr)) } ``` @@ -133,8 +138,8 @@ the shared part appears once and only the difference lives in the conditional." const toolMethod = (capabilities: ServerCapabilities, id: Id) => (schema: T, params: Unknown, handler: (v: Ts) => Effect) => capabilities.tools === undefined - ? pureOk(_errResponse(id)(methodNotFound)) - : validated(id, schema, params)(pr => ioStep(handler(pr), r => pureOk(_okResponse(id)(r)))) + ? pureOk(errorResponseOf(id)(methodNotFound)) + : validated(id, schema, params)(pr => ioStep(handler(pr), r => pureOk(successResponseOf(id)(r)))) ``` **It has to thread its context, for the same reason `validated` does.** diff --git a/fjs/protocol/mcp/types.ts b/fjs/protocol/mcp/types.ts index 34268a981..c03cf4d04 100644 --- a/fjs/protocol/mcp/types.ts +++ b/fjs/protocol/mcp/types.ts @@ -67,7 +67,7 @@ export type ToolEntry = { * **`never` is a claim, not an absence.** The handler behind this does perform * effects and they can fail — a session-state read is dispatched by a runner * that may decline it. It says `never` because it has *absorbed* those: a - * request's failure becomes `_errResponse(id)(internalError)` and a + * request's failure becomes `errorResponseOf(id)(internalError)` and a * notification's is dropped, there being no frame to put it in. Spelling that * as `Effect<…, never>` puts the decision in the type where a reader can * disagree with it, which an opaque payload could not. diff --git a/todo/retired-issue-identifiers.md b/todo/retired-issue-identifiers.md index b7fc2c4a2..71173839a 100644 --- a/todo/retired-issue-identifiers.md +++ b/todo/retired-issue-identifiers.md @@ -32,9 +32,10 @@ Every cited identifier below has a file: |`i65Y-proof-by-export`|`issues/65Y-proof-by-export.md`|open|`emergent_testing/todo/65y-proof-asserteq-adoption.md` ×1| **18 bare citations across 13 files.** The column counts *bare* occurrences -only. `i665-mcp` also appears twice as a working link — `mcp/todo/README.md`'s -own anchor reference to its `## 665-mcp` section, and one in -`json_rpc/todo/response-constructors.md` — and those need nothing. Regenerate the +only. `i665-mcp` also appears once as a working link — `mcp/todo/README.md`'s +own anchor reference to its `## 665-mcp` section — and that needs nothing. A +second such link lived in `json_rpc/todo/response-constructors.md`, which was +deleted when the constructors shipped. Regenerate the whole column rather than trusting it — an earlier revision of this issue built it from a scan that printed only the first two paths per identifier, and listed two of `i183`'s four sites.