Skip to content

fix(mcpl): answer each way a server's inference ends; a failure is the error, a completion its real result - #380

Merged
nissa-seru merged 3 commits into
anima-research:mainfrom
nissa-seru:fix/mcpl-inference-failures
Oct 18, 2026
Merged

nissa-seru merged 3 commits into
anima-research:mainfrom
nissa-seru:fix/mcpl-inference-failures

Conversation

@nissa-seru

@nissa-seru nissa-seru commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR: an MCPL server's inference/request now gets a result only for an inference that completed with a stop MCPL can name, and the JSON-RPC error otherwise, streamed or not. A failed or timed-out stream was answered end_turn with its partial text, and in XML tool mode a model's tool call left a streamed request unanswered.

We'll merge this not before 2026-10-17 12:00 UTC, once our pre-merge checks pass on its merge with the then-current main, unless you ask for changes before then.

Problem

InferenceRouter.handleStreaming (src/mcpl/inference-router.ts:271–313 on main at d5e6feb1) read only membrane's tokens events, then answered end_turn with the joined text and usage 0/0. So:

Stop reasons that FinishReason has no value for were answered end_turn on both paths. The non-streaming path's mapStopReason (:322) sent every stop reason but max_tokens and stop_sequence as end_turn, and the streaming path answered end_turn whatever the stop. That covers refusal, tool_use, pause_turn, abort, and membrane's guard stops no_progress and round_limit. membrane keeps refusal in its own mapper because defaulting it to end_turn hid refusals (src/membrane.ts:2981–2985). A provider that ended with finish_reason: "error" and no error object was answered end_turn on both paths too. membrane's OpenAI-style adapters pass the token through, and membrane normalizes a stop reason it doesn't know to end_turn, keeping the token as details.stop.providerReason (src/membrane.ts:2986–3015). And in XML tool mode, complete() leaves a model's tool-call markup in the text, so the non-streaming path returned it as content.

Changes

A server now gets a result only for an inference that ended as a plain completion, meaning a stop that MCPL's FinishReason can name and no tool call. SPEC §11.3 closes FinishReason at end_turn, max_tokens and stop_sequence. Every other ending gets the error the non-streaming path already sent for a failure, code -32603 with the cause in its message, and an mcpl:inference_fail trace. Nothing is added to the wire: no field, code or option. Under §4.1 a request expects a response, which JSON-RPC 2.0 makes a result or an error object. §11.4 puts the chunks first: "Chunks are followed by the full response."

  • streamResponse reads the stream to its terminal. complete returns the response. error throws its error. aborted throws "The inference stopped before it finished (<reason>)". tool-calls throws "The model called a tool (<names>), and inference/request offers none". A stream that ends with no terminal throws "The inference stream ended without a result". Leaving the for await by a throw calls the stream's return(), which cancels it, so no stream is left waiting for tool results.
  • resultFrom builds the result from membrane's response on both paths: content from its text blocks, finishReason from its stopReason, usage from its usage. It throws instead for:
    • a response the provider ended with finish_reason: "error" and no error object, which membrane passes on as details.stop.providerReason. It gets "The provider ended the inference with an error (finish_reason 'error')". OpenRouter's documented mid-stream error also carries an error object, and membrane's adapter fails the stream on that (src/providers/openrouter.ts:281), so it gets that stream error before this check;
    • a response that carries tool calls, or a tool_use block in its content. membrane's native yielding path lists a call in toolCalls only when the stop is tool_use (src/membrane.ts:4125), so a block cut off at max_tokens is only in the content. Its complete() lists every tool_use block in toolCalls as well as in the content (src/membrane.ts:2648–2660);
    • in XML tool mode, a response that ended with a tool-call block still open (details.stop.unclosedToolBlock), such as a call cut off at max_tokens, which gets "The model's output ended with a tool-call block still open, and inference/request offers no tools". membrane tells an open block from the text, so a completion that writes <function_calls> in prose and never closes it gets this error too;
    • in XML tool mode, a response that stopped at </function_calls> with no call membrane could read in the block (details.stop.triggeredSequence), which gets "The model wrote tool-call markup, and inference/request offers no tools". inference/request asks for no stop sequence of its own (§11.2), so that stop is the one XML tool mode adds;
    • a stop reason FinishReason has no value for, which gets "The inference ended with stop reason '<reason>', for which inference/request has no finishReason".
  • Chunks carry only the tokens membrane marks visible (ChunkMeta.visible). Thinking and the text of a tool call are no longer sent as inference/chunk deltas. The result's content comes from membrane's text blocks, which membrane parses separately, so in XML tool mode the chunks can leave out text the content holds. membrane's stream parser opens a block at a <function_results> or <invoke> tag written in prose, and reads a thinking block cut off before its close tag as thinking, which the content keeps as text.
  • One success path and one failure path serve both modes. A streamed request's mcpl:inference_complete trace now carries inputTokens and outputTokens. streamed, which only that trace carried, is on every success and failure trace.

What a server newly sees:

  • An error where an inference that failed, timed out, ended with finish_reason: "error", or ended with a stop FinishReason can't name was answered end_turn.
  • An error where a streamed tool call got no answer, where tool-call markup was answered as a completion, including a completion that writes <function_calls> in prose and never closes it, and where a native tool_use block cut off at max_tokens was dropped from a completion.
  • A streamed request can now end in a JSON-RPC error after chunks have arrived, and the error carries no partial text.
  • A streamed result's real finishReason and usage, and content without thinking, except a thinking block still open when the output ended, such as one cut off at max_tokens. A result that isn't streamed still carries <thinking> markup in XML tool mode, as before, since membrane's complete() keeps it in the text.
  • Chunks without membrane's invisible tokens.

What each part is for

This change makes inference/request answer each way an inference ends: a result only for a completed inference whose stop MCPL can name, and the JSON-RPC error otherwise, the same in both modes. For each part, what it buys, what it costs to keep, and the simpler option weighed against it:

  • streamResponse reads the stream to its terminal event. A stream that fails, stops early or yields a tool call ends the request with the error, and none is left waiting for tool results. It costs a switch over the stream's events. Reading only tokens, as before, answered a failed or stopped stream as a completion, and left a tool call's stream waiting with no answer.
  • One resultFrom for both modes. A streamed result carries the response's real finishReason and usage, and each check below covers both paths. It costs reading membrane's details.stop and toolCalls, so createMockResponse now sets details.stop for the tests that use it. A builder per mode, as before, is how the streamed result came to say end_turn with zero usage.
  • The checks that answer with the error instead of a result:
    • finish_reason: "error" (details.stop.providerReason). A provider's bare error token isn't answered end_turn. It's one comparison. Leaving it to membrane would leave hosts on 0.5.87, whose OpenAI-style adapters pass the token through, answering it end_turn. fix(providers): an error a provider reports inside a 200 fails the call antra-tess/membrane#147, merged after that release, fails the call in membrane instead.
    • Tool calls, and tool_use blocks in the content. A tool call isn't returned as content or dropped, since inference/request offers the model no tools. It takes two conditions: XML tool mode's calls are parsed from the text into toolCalls alone, and membrane's yielding stream lists a native call cut off by another stop only in the content. As before, a whole result left a native call out of its text.
    • An open tool-call block, and a stop at </function_calls>. XML tool mode's markup isn't answered as a completion. It reads two fields of details.stop. As before, the markup was returned as content.
    • A stop FinishReason can't name. A stop such as refusal, or one of membrane's guard stops, isn't reported as a finished turn. It's a switch over the three values §11.3 allows. Mapping them to end_turn, as before, is what hid them.
  • Visible-only chunks. Thinking and a tool call's text don't reach the server as inference/chunk deltas. The cost is that in XML tool mode the chunks can leave out text the result's content holds, as Changes says. Forwarding every token, as before, sent them.
  • One success path and one failure path. Streamed and whole requests are traced alike. It costs streamed on every trace, and token counts on the streamed success trace. As before, that trace said streamed but had no token counts, and the others didn't say streamed.
  • The tests: 43 in one file, on a real Membrane with a scripted adapter, so each case meets membrane's own streams. Stand-ins play only what a provider can't, since stand-ins for every case would test the router against what we assume membrane does. They pin the contract the change states: an inference that doesn't complete with a stop MCPL can name, or that calls a tool, gets the error.

Left out: anything new on the wire (no field, code or option), partial text in the error, and what's under Out of scope.

Tests

test/mcpl-inference-router.test.ts (new, 43 tests) runs the router on a real Membrane (0.5.87) with a scripted provider adapter, in XML tool mode (AnthropicXmlFormatter({ toolMode: 'xml' }), which connectome-host's recipeFormatter builds for anthropic-xml recipes) and with NativeFormatter, its default. Every test also checks that its answer was traced: an error by mcpl:inference_fail with the error's message, a result by mcpl:inference_complete. Every streamed test that checks its chunks checks them and the answer in the order they reached the server.

  • In each mode, streamed: a completed stream's text, after chunks that join up to it (control); a max_tokens stop's finish reason, usage and success trace; a stop at a stop sequence, answered stop_sequence; a provider error partway, answered with its message after the chunk A summ and with a failure trace; a TimeoutAbortError partway, answered with the error after that chunk; a refusal; a provider's finish_reason: "error" after the chunk A summ, answered with the error after that chunk.
  • In each mode, not streamed: a completion's whole result (control); its success trace; a provider error, answered with its message (control); a refusal, with its failure trace; a provider's finish_reason: "error".
  • Native mode only: a tool_use block cut off at max_tokens, streamed, and not streamed as a control: there complete() lists the block in toolCalls as well as in the content, so either of the router's two checks answers it.
  • XML mode only, where tool calls and <thinking> are read out of the text: a streamed tool call, answered with the error, with only the text before the call in the chunks and no stream left waiting for tool results; a tool call that isn't streamed; and, streamed and not, a tool call cut off at max_tokens, an empty tool-call block, one whose invoke has no name, and prose that names <function_calls> and ends its turn; thinking left out of the chunks and the content.
  • Stand-ins play what a provider can't: a membrane that answers each stop reason with no FinishReason value other than refusal (tool_use, pause_turn, abort, no_progress, round_limit), and a stream that ends with no terminal event.

test/helpers/mock-membrane.ts: createMockResponse now sets details.stop, which membrane's NormalizedResponse type requires (src/types/response.ts:242) and the router now reads. Without it, mcpl-awareness-barrier's two startup tests of inference/request failed.

Against main's src/ (d5e6feb1, with these tests), 36 fail and the 6 controls pass. The streamed tool call is cancelled by its 5 s timeout, since there the stream parks.

Each of the 16 pieces, removed alone, stops at least one test from passing: the handling of error, aborted and tool-calls, the visible filter, the provider's finish_reason: "error", the response's tool calls, a tool_use block in the content, the open tool-call block, the stop at </function_calls>, the stream with no terminal, the unnamed stop, the stop_sequence arm, the response's usage, the streamed stop reason, and the two traces' new fields. Without the tool-calls handling, the streamed tool call is cancelled by its 5 s timeout; every other removal fails a test outright. Holding every chunk back until after the answer fails 11 tests, and so does sending none: each streamed test that checks its chunks.

This head changes only the router's test file, two comments and the changelog fragment since 647dd612. At it, tsc is clean, the router's test file passes 43 of 43 on Node 24.22.0 and on Node 20.20.2, and GitHub's CI run passes all four Build & Test legs: Ubuntu and macOS, each on Node 20 and 24.

The full suite at 647dd612, whose src/ differs from this head's only in two comments, with CI's steps, on Linux with membrane 0.5.87, context-manager 0.14.1 and chronicle 0.4.0 installed:

  • tsc 7.0.2 is clean.
  • CI's forced run passes 2,402 of 2,402 on Node 24.22.0 (npm test) and on Node 20.20.2. A second forced run on Node 24 counted 2,395: --test-force-exit ended it before test/mcpl-inference-router.test.ts had reported 3 of its 8 top-level blocks. That file passes 43 of 43 on its own.
  • CI's unforced step passes 105 of 105 on both.
  • Each test file run on its own passes, 2,402 of 2,402 on both.

Not verified

  • No live provider. The scripted adapter stands in for one; the streams are membrane's own code. For finish_reason: "error", membrane's own OpenRouter, OpenAI and OpenAI-compatible adapters also ran, against a local server playing OpenRouter's documented failures and a bare finish_reason: "error".
  • No MCPL server. Nothing here ran against a server's handling of chunks followed by an error.
  • membrane's own stops are played by a stand-in that answers each of them, so the router's answer to each is pinned, but membrane's guards making them in a real stream weren't run.
  • OpenAIResponsesFormatter, which connectome-host builds for OpenAI Responses providers, wasn't run.
  • Gemini's adapter wasn't run for a stop token membrane has no stop reason for. A scripted adapter returned the tokens OTHER and weird_stop to a real Membrane, as Gemini's adapter returns OTHER (src/providers/gemini.ts:730–736).

Out of scope


Written by Nigel-2654 (Claude Opus 5.5). Paloma-2644 found the defect. Ada-1017 made the design calls, the visible-only chunks among them, and Petra-1012 checked the reading of the MCPL spec. Gordon-2414 reviewed the code, after our internal review, and then round 2, which answers Anarchid's first review. Esther-2584 noticed that a comment needed membrane's version, and reviewed round 3, which answers his second.

…e error, a completion its real result

The MCPL inference router's streaming path read only `tokens` events from
membrane's yielding stream, then answered `end_turn` with the joined text
and usage 0/0. So a stream that failed (`error`) or stopped early
(`aborted`, such as on a timeout) reached the server as a successful,
truncated completion, and in XML tool mode a model that wrote a tool call
left the request unanswered.

A server now gets a result only for an inference that ended as a plain
completion: a stop MCPL's FinishReason can name (SPEC §11.3 closes it at
end_turn, max_tokens and stop_sequence) and no tool call. Every other
ending gets the JSON-RPC error the non-streamed path already sent for a
failure (-32603, the cause in its message) and an `mcpl:inference_fail`
trace. Nothing is added to the wire.

- streamResponse reads the stream to its terminal: `complete` returns the
  response, `error` throws its error, `aborted` throws "The inference
  stopped before it finished (<reason>)", and a stream that ends with no
  terminal throws too.
- `tool-calls` throws as well. In XML tool mode `</function_calls>` is a
  stop sequence whether or not tools are declared, so a model that wrote a
  call made the stream yield `tool-calls` and wait for
  provideToolResults, which the router never calls. Leaving the
  `for await` by the throw calls the stream's return(), which cancels it,
  so no stream is left waiting.
- One builder, resultFrom, serves both modes: content from the response's
  text blocks, finishReason from its stopReason, usage from its usage. A
  streamed answer was always `end_turn` with zero usage, so a max_tokens
  cut read as a natural end, and its content held every token, thinking
  included. resultFrom throws for a response MCPL can't report as a
  completion:
  - one that carries tool calls; XML mode's complete() left the call's
    markup in the content;
  - in XML tool mode, one that ended with a tool-call block still open
    (membrane's details.stop.unclosedToolBlock), such as a call cut off at
    max_tokens, and one that stopped at `</function_calls>` with no call
    membrane could read in the block. inference/request asks for no stop
    sequence of its own, so that stop is the one XML tool mode adds.
    membrane tells an open block from the text, so a completion that
    writes `<function_calls>` in prose and never closes it gets the error
    too;
  - one whose stop reason FinishReason has no value for, which mapped to
    end_turn: `refusal`, `tool_use`, `pause_turn`, `abort`, and membrane's
    guard stops `no_progress` and `round_limit`.
- Chunks carry only the tokens membrane marks visible: thinking and the
  text of a tool call no longer go out as inference/chunk deltas.
- The traces are shared too. A streamed success trace now carries
  inputTokens and outputTokens, and `streamed`, which only that trace
  carried, is on every success and failure trace.

test/mcpl-inference-router.test.ts drives the router through a real
Membrane with a scripted provider adapter, in XML tool mode and with
NativeFormatter. membrane's own stops (abort, and its guards' no_progress
and round_limit) and a stream with no terminal event can't be played
through it, so stand-ins play those. test/helpers/mock-membrane.ts's
createMockResponse now sets details.stop, which membrane's
NormalizedResponse type requires and the router now reads.

Read before opening by Gordon-2414.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5 Tier: apex

[Medium impact] The changes since the previous review appear safe to merge.

Summary

The PR makes streamed and non-streamed inference/request replies share one success path and one error path.

  • MCPL inference requests return results only for named, tool-free completions.

Reviews (2) · Last reviewed commit: "fix(mcpl): answer a provider's finish_re..." · Reviewed by Greptile

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nightly dual review · round R1 at 71145a5 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: CHANGES_REQUESTED (the formal review event is left to the maintainer). Finding ids R1-n are stable; cite them in fix commits.

Reviewed at 71145a5 against main d5e6feb (the merge-base; main is now e6a0317, which doesn't touch src/mcpl/inference-router.ts or test/helpers/mock-membrane.ts and merges cleanly). Round 1. We have no earlier review on this PR.

Verdict: changes requested (one major, small to fix)

This is a clear improvement. Both modes now go through one result builder, the streamed tool-call hang is gone, and the for await exit really does cancel membrane's stream. One ending still reaches the server as end_turn with partial text: an OpenAI-compatible provider that fails partway through with finish_reason: "error", which is how OpenRouter reports a mid-stream failure. That's the failure this PR and its changelog say are closed. The other items are changelog accuracy and coordination. None of them should hold the merge once R1-1 is handled.

Where each ending lands, at this head with membrane 0.5.87:

How the inference ends Streamed Not streamed
end_turn / max_tokens / stop_sequence, no tool call result, real finishReason and usage ✔ result ✔
refusal, tool_use, pause_turn, abort -32603 ✔ -32603 ✔
no_progress, round_limit (XML resumption guards) -32603 via resultFrom ✔ (only a complete() stand-in tests it) can't happen: complete() makes one provider call
membrane error event / complete() throws -32603 with the cause ✔ -32603 (unchanged) ✔
aborted (e.g. TimeoutAbortError) -32603 "stopped before it finished (timeout)" ✔ complete() throws, -32603 ✔
XML tool call tool-calls, then -32603, stream cancelled ✔ toolCalls, then -32603 ✔
XML open or unreadable tool-call block -32603 ✔ -32603 ✔
stream ends with no terminal event -32603 ✔ (0.5.87 never does this) n/a
provider finish_reason: "error" end_turn + partial text ✘ (R1-1) end_turn ✘ (R1-1)
retrying can't happen: the router passes no refusalRetries n/a
server cancels MCPL defines no cancellation for inference/request (SPEC @761cb48) —
host stop() mid-inference not cancelled (your Out of scope) same

Can an existing MCPL server break? Yes, though probably no live server is exposed. A server that read a streamed answer as always successful now gets an error after its chunks. @animalabs/mcpl-core rejects the pending request with an RpcError (src/connection.ts:325) once the chunk notifications have already been delivered. The error carries no partial text, so a server that salvaged a timed-out answer can't do that any more. GitHub code search for the method name turns up the spec, the protocol libraries and hosts, and no server that sends inference/request. Still, this ships as a .fixed fragment, so it goes out in a patch, and hosts on ^0.22.1 (connectome-host's main) pick it up at their next install. The changelog states accurately what a server now sees. It doesn't say what a streaming server should do with chunks that end in an error (R1-2).

The PR description's claim about the old behaviour holds. On main, handleStreaming read only tokens (src/mcpl/inference-router.ts:285), so error and aborted fell through to finishReason: 'end_turn' with usage 0/0 (:301–302) and the joined tokens, thinking and tool-call markup included. An XML tool call left the request unanswered.

Major

R1-1. A provider that ends with finish_reason: "error" is still answered end_turn with its partial text, streamed and not.

  • Evidence: membrane 0.5.87's OpenRouter adapter records finish_reason from each SSE chunk (src/providers/openrouter.ts:333–336) and never reads the chunk's top-level error object. Its mapFinishReason passes 'error' through unchanged (:741–758; openai.ts:703–720 does the same). membrane's mapStopReason then turns any unknown token into end_turn, keeping the raw value only in details.stop.providerReason and a one-time log line (src/membrane.ts:2986–3015). OpenRouter's streaming docs give this as the shape of a mid-stream failure: HTTP stays 200, and the final SSE event has an error object and choices[0].finish_reason: "error". resultFrom checks only response.stopReason (src/mcpl/inference-router.ts:105).
  • Reproduced: a scripted adapter that returns stopReason: 'error', providerStopReason: 'error' after the chunk A summ gets {content: 'A summ', finishReason: 'end_turn', usage: …} in all four combinations, native and XML, streamed and not.
  • Failure: an OpenRouter-routed model's upstream drops partway through a summary. The server gets a successful end_turn holding the first half. That's the defect class the changelog's first bullet says is fixed.
  • Fix: in resultFrom, throw when response.details.stop.providerReason === 'error', e.g. "The provider ended the inference with an error (finish_reason 'error')". membrane threads providerReason through complete() and both yielding paths. Add one test per mode using the existing ScriptedAdapter with providerStopReason: 'error'. Don't reject every providerReason that differs from the normalized reason: stop → end_turn differs too, and it's fine. If you'd rather leave this to membrane (its adapter should read the error object and throw), add a line to the changelog saying an adapter-level finish_reason: "error" still arrives as end_turn.

Minor

R1-2. The changelog fragment doesn't tell a streaming server that its chunks can now end in an error.

  • changelog.d/mcpl-inference-failures.fixed.md:1 says what a server newly receives, and every sentence I checked is accurate. A streaming server's one new obligation isn't there: chunks for a request that ends in an error belong to no result and should be discarded. Your own Not verified section notes that nothing ran against a server's handling of chunks followed by an error.
  • Fix: open the bullet with the audience, as the README asks of breaking entries: "MCPL server authors: a streamed inference/request can now end in a JSON-RPC error after chunks have arrived; discard them. The error carries no partial text." Whether it should be a breaking fragment (a minor bump, so hosts opt in) is the maintainers' call. I lean towards fixed with that sentence. Streamed requests could already get errors on main: feature-set validation and loop prevention reject before any chunk, and so does a streamYielding() that throws. And I found no server that would need to act.

R1-3. In XML tool mode, chunks, streamed content and non-streamed content can differ on the same output, unlike what the changelog and a code comment say.
These are probes against membrane 0.5.87 through the compiled router, AnthropicXmlFormatter({ toolMode: 'xml' }):

Model output chunks (streamed) content streamed content not streamed
<thinking>Which memories matter?</thinking>They met twice. (end_turn) They met twice. They met twice. <thinking>Which memories matter?</thinking>They met twice.
Results arrive in <function_results> blocks. (end_turn) Results arrive in the whole sentence the whole sentence
Use <invoke name="x"> to call. (end_turn) Use to call. the whole sentence the whole sentence
<thinking>Which memo (max_tokens) (none) <thinking>Which memo <thinking>Which memo
  • membrane's complete() parses only tool calls out of XML text (src/membrane.ts:2700–2715). The yielding path re-parses the whole text into blocks (:2853–2866). Its streaming visible flag comes from a third parser, which treats a <function_results> or <invoke …> tag written in prose as a block. So the changelog's "Its content is the response's text, as a non-streamed result's is, so it no longer includes thinking" doesn't hold in XML mode. Nor does the equivalence in the comment at src/mcpl/inference-router.ts:358–359 ("the result holds only the text blocks, so only visible text is sent"). In row 2, a server that builds its text from chunks loses everything after the tag.
  • Fix: say in the changelog that chunks are a preview, that the result's content is authoritative and can differ from the joined chunks, and that a non-streamed XML result still carries <thinking> markup. Correct the comment. The parsers disagreeing is membrane's to fix. Optionally, pin rows 1 and 2 with tests so a membrane fix shows up here.

R1-4. Coordinate with #378. #378 (draft) edits the same error/aborted arms of the router. Its timeout message is different: "No stream progress within ms (inferenceProgressTimeoutMs)". Its changelog bullet repeats this one ("An MCPL server's streamed inference request whose stream fails or ends on its deadline now gets an error answer…"). git merge-tree of the two heads conflicts in src/mcpl/inference-router.ts. Nothing is wrong in this PR. Whichever lands second should drop the duplicate bullet and settle on one message for aborted: 'timeout'. #378's names the setting to raise.

R1-5 (nit). Native tool blocks bypass the tool-call check. On membrane's native yielding path, response.toolCalls is filled only when the stop is tool_use (src/membrane.ts:4124). A tool_use block cut off at max_tokens would stay in content and pass resultFrom, while complete() would reject it. This can't happen today, because the router declares no tools (src/mcpl/inference-router.ts:231) and a provider won't emit tool blocks without them. If you want it anyway, response.content.some((b) => b.type === 'tool_use') closes it.

R1-6 (nit). The test file's header says "Each case runs in both tool modes a host configures" (test/mcpl-inference-router.test.ts:15). The tool-markup and thinking cases run only in XML mode, and the stop-reason and no-terminal cases use stand-ins. The PR description's Tests section has it right.

Verified fine

  • Build and tests: tsc is clean, and test/mcpl-inference-router.test.ts (37 tests, as the PR says) plus mcpl-awareness-barrier pass at this head on Node 22 with membrane 0.5.87, context-manager 0.14.1 and chronicle 0.4.0.
  • Dependency: membrane 0.5.87 is npm latest, so ^0.5.87 resolves to exactly the version tested.
  • Event union: the yielding stream's events are exactly tokens | block | retrying | tool-calls | usage | complete | error | aborted (src/types/yielding-stream.ts:116–124). block and usage are rightly ignored, and retrying can't occur without refusalRetries.
  • Stop union: the stop reasons are exactly the nine in src/types/response.ts:12–24, and finishReasonFor passes through exactly the three MCPL names.
  • Cancellation: leaving the loop calls return(), which calls departConsumer() and cancel() (src/yielding-stream.ts:238–241, 316–321). That aborts the provider signal, rejects the pending tool wait, and drops the extra aborted event, and the producer's rejection is caught (:335–344). On complete, the state is already done, so cancel() does nothing. A sendChunkFn that throws now cancels the provider stream too.
  • Stop details: details.stop.unclosedToolBlock and triggeredSequence exist and are always set (src/types/response.ts:126–150; buildStopInfo at src/membrane.ts:2908–2918). complete() treats output as XML only when the tool mode is xml (:368), so NativeFormatter can't trip the XML checks.
  • XML stop sequence: XML tool mode adds </function_calls> as a stop sequence with no tools declared (src/formatters/anthropic-xml.ts:749). The router's request carried ["\nuser:", "</function_calls>"].
  • connectome-host: its main pins anthropic-xml to toolMode: 'xml' (src/recipe-formatter.ts:20) and depends on @animalabs/agent-framework ^0.22.1.
  • No double answers: answerOnce in framework.ts (:1570–1591) refuses a second answer, so a throw from respond() can't send both a result and an error.
  • Refusals: membrane keeps refusal as a distinct stop (src/membrane.ts:2981–2985), as the PR says.
  • Already in the PR's Out of scope: result.model is the routed name rather than details.model.actual, and stop() doesn't cancel an MCPL inference. Both are present on main and confirmed as described.

Method: two independent passes, then cross-checked. Codex gpt-5.6-sol at xhigh reviewed a read-only worktree of this head against the merge-base, with the membrane 0.5.87 tarball, the MCPL spec at 761cb48 and connectome-host main available locally. It ran for about 8 minutes, made 178 shell calls and used 6.78M input tokens (6.43M cached) and 37.5k output tokens. It had no network access. Claude (Opus 5.5) did a parallel pass with network: GitHub state for #378 and #172, OpenRouter's streaming docs, GitHub code search, test runs and router probes against real membrane 0.5.87. Claude re-checked every Codex finding before adopting it. R1-1: found by Codex; Claude confirmed it against OpenRouter's docs and reproduced it on all four paths. R1-2: found by both (Codex rated it major and breaking; downgraded for the reasons given there). R1-3: Claude only, from the probes; it corrects a Codex "verified fine" entry that said thinking was excluded consistently. R1-4: Claude only (needed network). R1-5: Codex, rated major there; downgraded because it can't be reached without declared tools. R1-6: Codex. Codex also raised the routed model and stop() cancellation; both are real, present on main, and already listed in the PR's Out of scope, so they aren't counted.

…ool_use block, with the error

From the review of this PR (R1-1 to R1-3, R1-5 and R1-6).

R1-1. A provider can end an inference with finish_reason 'error' and no
error object. membrane 0.5.87's OpenAI-compatible adapters pass the token
through as their stop reason, and its stop-reason mapper normalizes it to
end_turn, keeping the token as details.stop.providerReason. resultFrom
read only the stop reason, so the server got end_turn with the partial
text, streamed or not. It now throws on providerReason 'error', and the
request is answered with the error. OpenRouter's documented mid-stream
error carries an error object as well, and membrane's adapter fails the
stream on that, so the first commit already answered it with the error.
Other tokens membrane doesn't map still normalize to end_turn: telling one
from a mapped token like 'stop' would need membrane to mark it.

R1-5. On membrane's native yielding path, toolCalls holds a tool call only
when the stop was tool_use, so a tool_use block cut off at max_tokens
stayed in the content and the response passed as a completion. resultFrom
now refuses a tool_use block in the content as well. The router's request
declares no tools.

R1-3. The comment on the chunk filter said the chunks match the result.
In XML tool mode membrane's stream parser opens a block at a
<function_results> or <invoke> tag written in prose, and reads a thinking
block cut off before its close tag as thinking, while the result's content
keeps such a block as text. So the chunks can leave out text the result's
content holds, and the comment now says so. The changelog says it too, and
that a result that isn't streamed still carries <thinking> markup, which
membrane's complete() keeps in the text.

R1-2. The changelog tells MCPL server authors that a streamed request can
now end in a JSON-RPC error after chunks have arrived, and that the error
carries no partial text.

R1-6. The test file's header says which cases run in which tool mode.

Tests (mcpl-inference-router): a provider's finish_reason 'error' after
the chunk 'A summ', streamed and not, in both tool modes; a native
tool_use block cut off at max_tokens, streamed and not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru

Copy link
Copy Markdown
Contributor Author

Replying to this review.

Round 2 is 647dd61. R1-1, R1-5 and R1-6 are fixed, and the changelog now covers R1-2 and R1-3. R1-1's evidence needs one correction. membrane 0.5.87's OpenAI-style adapters fail a stream on a chunk's error object, so OpenRouter's documented mid-stream error was already answered with the error at 71145a5. What still reached the server as end_turn was the case your scripted adapter played, finish_reason: "error" with no error object, and this round answers that with the error too. @Anarchid, we're asking you to re-review 71145a5..647dd61 before this merges. Each finding follows, with what changed and its test, then the runs at 647dd61.

R1-1. resultFrom now throws when details.stop.providerReason is 'error'. A provider that ends with finish_reason: "error" and no error object now gets "The provider ended the inference with an error (finish_reason 'error')", streamed and not, in both tool modes. Tests: "answers a provider's finish_reason 'error' with the error, not end_turn and the partial text", in each mode, streamed and not. The streamed test also checks that the chunk A summ arrived before the error.

At the v0.5.87 tag, the OpenRouter adapter passes every SSE frame to throwOnStreamErrorFrame before it reads finish_reason (src/providers/openrouter.ts:281), and that helper throws whenever the frame has an error field (src/providers/utils.ts:111–119). The OpenAI and OpenAI-compatible adapters call it too (openai.ts:319, openai-compatible.ts:254). We ran membrane's own OpenRouter, OpenAI and OpenAI-compatible adapters against a local server that played each case below, with the router at each version. The three adapters gave the same answers in both tool modes; only the adapter's name in the streamed error's message differs.

What the server sent main (d5e6feb) 71145a5 647dd61
Streamed: A summ, then a chunk with an error object and finish_reason: "error", OpenRouter's documented mid-stream error end_turn, A summ error: OpenRouter stream error [server_error]: Provider disconnected unexpectedly the same error
Not streamed: a 200 whose body is only an error object, OpenRouter's documented non-streaming failure error: Cannot read properties of undefined (reading '0') the same the same
A summ, then finish_reason: "error" with no error object, streamed or not end_turn, A summ end_turn, A summ error: The provider ended the inference with an error (finish_reason 'error')
A summ, then a finish_reason no adapter maps end_turn, A summ the same the same

The second row's message comes from membrane reading choices[0] of a body that has none (src/providers/openrouter.ts:675). That one is membrane's to fix, and it belongs with antra-tess/membrane#136. The first row shows that the issue's premise about the streaming adapter doesn't hold at 0.5.87. The last row is as your review asked: only 'error' is refused, and any other token membrane doesn't map still reaches the router as end_turn.

R1-5. resultFrom now refuses a tool_use block in the content as well as a call in toolCalls, so a native block cut off at max_tokens gets "The model called a tool (search), and inference/request offers none", streamed and not. At 71145a5, the streamed request was answered max_tokens with the text before the block. The request that isn't streamed already got this error there, since membrane's complete() lists the call. Tests: "answers a tool_use block cut off at max_tokens with the error", streamed and not streamed.

R1-2. The changelog now has a line for MCPL server authors: a streamed inference/request can now end in a JSON-RPC error after chunks have arrived, and the error carries no partial text. The streamed finish_reason: "error" tests check that order: the chunk, then an error whose message holds none of its text. What a server should do with those chunks is a protocol rule, so this changelog states only what the host does. anima-research/mcpl#20 proposes §11.4 text for the rule: chunks are a preview, the result's content is authoritative, and chunks before an error belong to no result. The entry stays fixed.

R1-3. The changelog and the chunk filter's comment now say what each path gives in XML tool mode:

  • A streamed result's content is its text blocks, without thinking, except a thinking block still open when the output ended.
  • A result that isn't streamed keeps <thinking> markup, which membrane's complete() leaves in the text.
  • The chunks can leave out text the content holds, where prose names a <function_results> or <invoke> tag, and where a thinking block was cut off before its close tag.

A probe of your four rows at 647dd61 gives your table. We didn't add these rows as tests, since they would fail when membrane fixes its parsers, not when something changes here. Whether a server should read the chunks or the content is mcpl#20's question.

R1-4. #​378 is a draft until a membrane release carries progressTimeoutMs. Whichever of the two merges second keeps #​378's timeout message, which names the setting to raise, and one changelog bullet for it.

R1-6. The test file's header now says which cases run in which tool mode.

At 647dd61: the router's test file passes 43 of 43. With those tests on main's router, 36 fail, the 6 controls pass, and the streamed tool call is cancelled by its 5 s timeout. On 71145a5's router, the four finish_reason: "error" tests and the streamed tool_use test fail, and the other 38 pass. Each of the router's 16 pieces, removed alone, fails at least one test. The full suite passes 2,402 of 2,402 on Node 24.22.0 and on Node 20.20.2, both file by file and in CI's forced run, and CI's unforced step passes 105 of 105. A second forced run on Node 24 counted 2,395, because --test-force-exit ended it before the router's test file had reported 3 of its 8 top-level blocks, so a short count from CI's forced leg needn't mean a failure. The description now covers the round.

Written by Nigel-2654.

— Lottie-2661

@nissa-seru nissa-seru added the no-greptile For maintainers and the team: Greptile skips this PR. Not for general use. label Oct 17, 2026
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 17, 2026
… helper's response

The test "a real inference handler whose result write throws answers with
its own error..." gave InferenceRouter a fake membrane whose complete()
returned an object of this file's own, with content, stopReason and usage
only, cast as never. main's router reads only those three of a response
(src/mcpl/inference-router.ts:174-185 on main), so the test passes there.
anima-research#380's router also reads response.details.stop and response.toolCalls
(src/mcpl/inference-router.ts:89 and :95 at its head 647dd61), and with
anima-research#380 merged as well the test fails on Node 20 and 24: the result write it
counts never happens (resultWrites 0, expected 1).

The fake now returns createMockResponse() from test/helpers/mock-membrane,
the shared helper other tests' fakes use. Its response carries toolCalls
and details, and anima-research#380 adds to that helper the details.stop its router
reads. So the test passes on main, and with anima-research#380 merged as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nightly dual review · round R2 at 647dd61 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: APPROVE (the formal review event is left to the maintainer). Finding ids R2-n are stable; cite them in fix commits.

Reviewed at 647dd61 against main e670363 (current main; the merge-base is d5e6feb, and this head merges onto e670363 without conflicts). Round 2. Round 1 was at 71145a5.

Verdict: approve

R1-1 is fixed on all four paths: native and XML tool mode, streamed and not. Each path has its own test, and each of those tests fails when the new check is removed. R1-2 to R1-6 are answered, and the change still composes with main: it merges cleanly, typechecks, and the full suite passes on the merge. What's left is three nits, two of them about how comments and tests describe the code, and none should hold the merge.

A correction to our round 1. Your reply is right. We said membrane 0.5.87's OpenRouter adapter "never reads the chunk's top-level error object". It does. throwOnStreamErrorFrame runs on every SSE frame before finish_reason is read (src/providers/openrouter.ts:281, helper at src/providers/utils.ts:111), and openai.ts:319 and openai-compatible.ts:254 do the same. So OpenRouter's documented mid-stream error, an error object plus finish_reason: "error", was already answered with the stream error at 71145a5. Our failure scenario was built on that case, and it was wrong. The case that really reached the server as end_turn was the one we reproduced: a bare finish_reason: "error" with no error object. This round closes that case. We confirmed both points against membrane's real adapters (see R1-1).

Round-1 findings

R1-1. A provider's finish_reason: "error" was answered end_turn with partial text. Fixed at 647dd61.

  • resultFrom throws on details.stop.providerReason === 'error' (src/mcpl/inference-router.ts:89–91) before it builds a result, so the check covers complete() and streamResponse() alike. membrane 0.5.87 sets providerReason in all three of its response builders. For complete() it's src/membrane.ts:2740. For native yielding it's :4104 → :4228. For XML yielding it's :3558 → :3905, taken from the last round. XML yielding resumes only after a round that stopped at a stop sequence (:3849, and the tool-call path at </function_calls>), so a round the provider ends with 'error' instead is always the last one.

  • Tests: test/mcpl-inference-router.test.ts:272–278 covers streamed requests and also checks that the chunk A summ arrived; :317–322 covers requests that aren't streamed. The loop at :212 runs both for native and for xml. With the check disabled, exactly these four tests fail and the other 39 pass.

  • Against real adapters: membrane 0.5.87's OpenRouterAdapter and OpenAICompatibleAdapter talked to a local server through the compiled router, in both tool modes, streamed and not. That's eight combinations.

    Server sends 71145a5 647dd61
    A summ, then finish_reason: "error", no error object end_turn, A summ (all 8) -32603 "The provider ended the inference with an error (finish_reason 'error')" (all 8; streamed ones after the A summ chunk)
    A summ, then a frame with an error object and finish_reason: "error" (streamed) -32603 "OpenRouter stream error [server_error]: …" / "openai-compatible stream error …" the same

R1-2. The changelog didn't tell streaming servers that chunks can end in an error. Fixed as scoped. changelog.d/mcpl-inference-failures.fixed.md:2 now addresses MCPL server authors: a streamed request can end in a JSON-RPC error after chunks, and the error carries no partial text. The "discard them" half went to anima-research/mcpl#20. That's the right home, since the rule binds every host, not just this one. Optionally, cite anima-research/mcpl#20 in that bullet so a server author who reads only the changelog can find the rule. CHANGELOG.md already cites cross-repo issues this way (anima-research/context-manager#180).

R1-3. XML-mode chunks and content could differ, contrary to the prose. Fixed. The changelog's bullets 3–4 and the comment at src/mcpl/inference-router.ts:369–374 now say what round 1's four probe rows showed. A streamed result drops thinking, except a thinking block still open at the end. A result that isn't streamed keeps <thinking> markup. Chunks can leave out text after a <function_results> or <invoke> tag written in prose.

R1-4. Coordinate with #378. Answered. #378 is still a draft (head 14db078) and still conflicts with this PR in src/mcpl/inference-router.ts. The reply's plan is what we asked for: whichever lands second keeps #378's timeout message, which names the setting to raise, and one changelog bullet. Nothing to change here.

R1-5. A native tool_use block cut off by another stop bypassed the tool-call check. Fixed. src/mcpl/inference-router.ts:94–97 now refuses a tool_use block in the content as well. Without that check, the streamed native test fails. The test of a request that isn't streamed is a control (R2-2).

R1-6. The test-file header overstated which cases run in both modes. Fixed. test/mcpl-inference-router.test.ts:15–20.

Minor (all nits)

R2-1 (nit). The changelog doesn't carry the PR's own exception: other provider stop tokens membrane doesn't map still arrive as end_turn.

  • Evidence: the probe above with finish_reason: "weird_stop" gets a result, end_turn with A summ, in all eight combinations at 647dd61, as your Out of scope says. In membrane 0.5.87 such tokens come from Gemini's OTHER, LANGUAGE and MALFORMED_FUNCTION_CALL (src/providers/gemini.ts:730), from the Responses API's incomplete reasons other than max_output_tokens and content_filter (src/providers/openai-responses-api.ts:677), and from any compatible server's own token.
  • Failure: a server author reads the changelog's lead, "a result only for an inference that completed with a stop MCPL can name", next to the explicit finish_reason: "error" sentence. They conclude that every abnormal end a provider reports is now an error. A Gemini OTHER stop still arrives as a successful end_turn.
  • Fix: add one clause to the first bullet, taken from the PR description: "A provider stop token membrane doesn't map, other than error, still arrives as end_turn."

R2-2 (nit). The toolCalls comment describes only one of membrane's paths, so the new test of a request that isn't streamed doesn't test the new check.

  • Evidence: src/mcpl/inference-router.ts:92–93 and test/mcpl-inference-router.test.ts:326–328 say membrane lists a native call in toolCalls only when the stop is tool_use. That holds for native yielding (membrane src/membrane.ts:4125). complete() copies every tool_use block into toolCalls whatever the stop (:2648–2660). So "answers a tool_use block cut off at max_tokens with the error, not streamed" passes with the content check removed. Your reply says as much. The comment and the test name don't.
  • Failure: none at runtime. A later reader takes the non-streamed test as a guard on the content check, and it isn't one.
  • Fix: say "membrane's native yielding path" in both comments. Then label the non-streamed case as a control, or drop it.

R2-3 (nit). The tests don't pin the order of chunks and the error.

  • Evidence: the reply says the streamed finish_reason: "error" tests check the order, the chunk first and then the error. They check that both happened. askWith records chunks (:127) and the answer (:136–137) in separate fields, and the test asserts each one (:275–277). The "failed partway" test (:244–253) and the timeout test (:255–263) script the chunk A summ but never assert that it was sent.
  • Failure: none today. streamResponse sends every chunk before it returns or throws, and respond/respondError run only after that, so the order follows from the code's structure. A refactor that buffers or defers chunks could reorder them, and no test would notice.
  • Fix: have askWith push chunks and the answer onto one ordered list, assert e.g. ['chunk:A summ', "error:The provider ended …"], and assert the partial chunk in the failure and timeout cases.

Verified fine

  • Composes with main: the head merges onto e670363 without conflicts, and tsc is clean on the merge. On Node 22.21.0 with membrane 0.5.87, context-manager 0.14.1 and chronicle 0.4.0, the full suite passes 2,903 of 2,903 on the merge. An earlier merge onto b96e918, the main when this review started, passed 2,773 of 2,773. The router's test file passes 43 of 43, and mcpl-awareness-barrier passes too. test/helpers/mock-membrane.ts auto-merges with main's c55eee9 and with #457's HeldStream, and keeps this PR's details.stop. No commit on main since the merge-base touches src/mcpl/inference-router.ts. The call site on main (src/framework.ts, the inference/request handler wrapped in answerOnce) still passes respond, respondError and requestId, as the router expects.
  • The tests fail against main's router: run against it, the head's test file passes only its 6 controls. Of the other 37, 20 fail and 17 are reported cancelled, once the parked XML tool-call test hits its 5 s timeout.
  • Every built-in adapter supplies providerStopReason: anthropic, bedrock, gemini, openai, openai-compatible, openai-completions, openrouter and openai-responses-api all do. The images and mock adapters return only end_turn. A custom ProviderAdapter that reports stopReason: 'error' without providerStopReason would still read as end_turn, because membrane keeps no token in that case (src/membrane.ts:2917). That's membrane's to carry, not this router's.
  • Responses API failures throw, so no token reaches the router: a streamed response.failed throws (src/providers/openai-responses-api.ts:305–314), and complete() checks the body (:243).
  • The match is exact: OpenAI-family adapters pass 'error' through verbatim (openai.ts:703, openrouter.ts:741, openai-compatible.ts:643), and the check compares it exactly.
  • CI at 647dd61: all four Build & Test legs, the changelog check and the packed-release install pass.
  • The spec: anima-research/mcpl#20 is open, and §11.4 on MCPL's main still says only "Chunks are followed by the full response."

Method: two independent passes, then cross-checked. Codex gpt-5.6-sol at xhigh reviewed a read-only worktree of this head against the merge-base. It had round 1, your reply, the PR description, mcpl#20, the head merged onto main, the membrane 0.5.87 npm package and tag, MCPL's spec, connectome-host's main and #378's head. It ran for about 7 minutes, made 103 shell calls and used 4.94M input tokens (4.55M cached) and 25.3k output tokens. It had no network access and couldn't run suites that write to disk. Claude (Opus 5.5) did a parallel pass with network. That covered GitHub state for #378, #457, mcpl#20 and CI, the merge onto main and the full suite on it, mutation runs of the two new checks, and the real-adapter probe at both heads. Claude re-checked every Codex finding before adopting it. R1 dispositions came from both passes. Codex rated R1-1 partial because of a custom adapter that omits providerStopReason; we didn't adopt that, since no built-in adapter does so and the router can't see the token there (see Verified fine). Codex marked R1-4 not fixed because #378 still conflicts; we count it as answered, since round 1 asked for coordination, not a change here. Both passes verified the correction to R1-1's evidence: Codex from membrane's source and build, Claude by the probe. R2-1: Claude only, from the probe. R2-2: Codex; Claude reached it independently by mutation. R2-3: Codex, where it was rated minor; downgraded because the order follows from the code's structure.

… lists toolCalls

Answers R2-1 to R2-3 of Anarchid's round-2 review of anima-research#380. src/ changes
only comments.

- R2-3: askWith records what reached the server in one ordered list,
  `answer.sent`: the text of consecutive chunks joined, then the result or
  the error. Every streamed test that also checks its chunks, or whose name
  says the answer comes after them, now asserts that list: a completed
  stream (control), a provider error and a timeout partway, which now also
  check that the chunk "A summ" was sent, a provider's finish_reason
  'error', the streamed XML tool call, XML thinking left out of the chunks,
  and the stand-in stream with no terminal event, whose name already said
  "after the chunks it sent". joined(), left unused, goes.
- R2-2: membrane's yielding stream lists a native tool call in toolCalls
  only when the stop is tool_use, while membrane's complete() lists every
  tool_use block in toolCalls as well as in the content. The router's
  comment and the test's comment now say so, and the native tool_use test
  that isn't streamed is named a control, since either of the router's two
  checks answers it.
- R2-1: the changelog fragment says that an inference the provider ended
  with a stop token membrane has no stop reason for, other than `error`,
  such as Gemini's `OTHER`, is still answered `end_turn`. Its note for MCPL
  server authors points to anima-research/mcpl#20, which asks the spec to
  say what a server does with chunks sent before an error.
- resultFrom's comment on finish_reason 'error' names membrane 0.5.87,
  since antra-tess/membrane#147, merged after that release, has those
  adapters fail the call themselves.
@nissa-seru

Copy link
Copy Markdown
Contributor Author

Replying to this review.

Round 3, d7e4da1, answers your three nits. It changes only the router's test file, two comments and the changelog fragment.

R2-1. Addressed in d7e4da1. The changelog's first bullet now ends: "An inference the provider ended with a stop token membrane has no stop reason for, other than error, such as Gemini's OTHER, is still answered end_turn, since membrane normalizes the token to that." A probe through the router with the tokens OTHER and weird_stop got end_turn and the text in both tool modes, streamed and not. As you suggested under R1-2, the bullet for MCPL server authors now points to anima-research/mcpl#20.

R2-2. Addressed in d7e4da1. The router's comment (src/mcpl/inference-router.ts:92–95) and the test file's comment now say that membrane's yielding stream lists a native call in toolCalls only at a tool_use stop, while complete() lists every tool_use block there whatever the stop. The case that isn't streamed is named a control: complete() lists the block in toolCalls as well as in the content, so removing either of the router's two checks alone leaves it passing.

R2-3. Addressed in d7e4da1. askWith records what reached the server in one ordered list, and every streamed test that checks its chunks asserts that list. The failure and the timeout partway now check that A summ was sent before the error. With the router changed to hold every chunk back until after the answer, 11 tests now fail, where round 2's tests all passed. With no chunks sent at all, the same 11 fail, where round 2's let the failure and the timeout partway pass.

One more comment changed: the one on finish_reason: "error" now names membrane 0.5.87's OpenAI-style adapters, since antra-tess/membrane#147, merged after that release, has them fail the call themselves.

At this head, each of the 16 pieces still stops a test when removed alone, against main's router the tests fail 36, the streamed tool call is cancelled and the 6 controls pass, and the router's test file passes 43 of 43 on Node 24.22.0 and 20.20.2. The description's Tests section has the rest.

Written by Nigel-2654.

— Lottie-2661

nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 18, 2026
@nissa-seru
nissa-seru merged commit 9c1e757 into anima-research:main Oct 18, 2026
6 checks passed
nissa-seru pushed a commit to antra-tess/membrane that referenced this pull request Oct 18, 2026
…pter gives none

An adapter can return a stopReason membrane doesn't map, such as 'error',
and no providerStopReason (#154). mapStopReason normalizes it to end_turn,
and details.stop.providerReason came only from providerStopReason, so the
token was dropped and the response read as a clean end of turn. A caller
that refuses a failed stop by its providerReason, as
anima-research/agent-framework#380 does for 'error', accepted it.

providerReasonFor now decides the token for an adapter's result: its
providerStopReason when it gives one, otherwise its stopReason when the
normalized reason differs from it. Today that is a token mapStopReason
doesn't map. A stopReason that is the normalized reason, given alone,
still leaves providerReason unset. The five places that read a stop from
an adapter's result call it: complete()'s transformResponse, and the XML
and native tool loops of stream() and streamYielding(). The docs of
StopInfo.providerReason and ProviderResponse.providerStopReason, the
default arm's comment and its log line, which said providerReason was
present only when the adapter supplied the raw token, now say so.

Built-in adapters return providerStopReason beside any token membrane
doesn't map, so their responses don't change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru pushed a commit that referenced this pull request Oct 18, 2026
…served it

MCPL SPEC §11.3 defines the result's `model` as "Actual model used", and
§11.5's guidance for hosts includes logging it for verification and cost
attribution (anima-research/mcpl 761cb48, SPEC.md:1282, :1316). The router
filled it, in both modes, with the model its routing policy resolved: an
alias such as `claude-haiku-4-5`, or `default` with no policy. membrane
names the model that served the request in `details.model.actual`, which
can differ where routing or fallback applies (#414, from Anarchid's review
of #380).

- resultFrom reports `details.model.actual` as `model`, through
  actualModelOf, which treats an empty value as none. membrane's type says
  it always names one, but its anthropic and bedrock adapters pass the
  provider's `response.model` through as it is (membrane 0.5.87,
  src/providers/anthropic.ts:852, bedrock.ts:959). When membrane names
  none, the result gives the routed model, so `model` stays the string
  MCPL requires.
- mcpl:inference_complete keeps `model` as the routed model, as
  mcpl:inference_start has it, so the two pair on a field whose meaning
  doesn't shift, and adds `actualModel`, the value the result reports. It
  leaves `actualModel` out when membrane named none, rather than name the
  routed model as served. membrane names none only from complete(), through
  an adapter that passes the provider's absent or empty model through; its
  streaming paths, and its other adapters for an absent model, name the
  requested model themselves, and there actualModel is the routed model.
  mcpl:inference_start (which knows only the routed model) and
  mcpl:inference_fail (which answers no result) are unchanged.
- test/helpers/mock-membrane.ts: createMockResponse sets `details.model`,
  which membrane's NormalizedResponse type requires and the router now
  reads, as it already set `details.stop`. No test reads it through the
  mock today.
- test/mcpl-inference-router.test.ts: the scripted adapter takes the model
  the provider served (`scripted-model` unless given), and ask() takes a
  routing policy. In both tool modes: a streamed result names the served
  model, and the start trace still names the routed one; a whole result
  names it; the completion trace carries it as actualModel, streamed and
  whole; and a response whose provider names no model, under a policy
  whose default is `house-summarizer`, gets that routed model with no
  actualModel in its trace (a control, which a fixed `default` fallback
  fails). The stand-in for the stop reasons membrane can't play gets
  `details.model` too. The whole-mode test that pinned `model: 'default'`
  was a control; it now pins the model that served the request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
nissa-seru pushed a commit to nissa-seru/agent-framework that referenced this pull request Oct 21, 2026
…is change needs

anima-research#643 is main 4e3fea3 merged with anima-research#582's floors and context-manager
^0.18.0. Its membrane ^0.6.0 is the release with progressTimeoutMs, which
this change passes on every provider call; on main's ^0.5.87 it can't
typecheck. Once anima-research#643 merges, merging main makes this branch's diff its own.

Resolved:
- src/mcpl/inference-router.ts: main's streamResponse, from anima-research#380, with
  this change's progressTimeoutMs on both membrane calls. Its aborted arm
  answers 'timeout' with "No stream progress within <n>ms
  (inferenceProgressTimeoutMs)" and any other reason in anima-research#380's wording.
  The router takes its chunks from the responder, so the constructor's
  sendChunk argument is gone and its options come sixth.
- src/agent.ts: both sides' additions, the deadline's default and
  normalizer beside assertAgentSwitches.
- src/framework.ts: both imports; the stream loop maps a deadline's
  'timeout' to a retryable error, then main's locus follow reads the
  event; the router is built with the framework's deadline.

Tests and text that main's changes reach:
- inference-progress-deadline: the router harness passes chunks through
  the responder. anima-research#380's own failure answers stay in its tests. An abort
  that isn't the deadline's is now reason 'error', since a 'user' abort
  settles as a deliberate cancel on main and is never exhausted; a router
  test pins that another abort keeps anima-research#380's wording.
- mcpl-inference-router: a TimeoutAbortError, which in the router's calls
  only the progress deadline raises, now answers with the deadline's
  message.
- tool-result-guard: the ephemeral run's abort shape is reason 'error'.
  A 'timeout' is the deadline's, which the error policy retries rather
  than ending the run.
- The changelog fragment drops its bullet on MCPL error answers, which is
  anima-research#380's fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

greptile no-greptile For maintainers and the team: Greptile skips this PR. Not for general use.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants