[nissa-seru]

Why. agent-framework's reference handling says a payload a server marks disposition: never is withheld from model context "unconditionally", yet the framework fetches such a payload into the agent's workspace, where its tools can read it. RFC-005 §3.1 says neither the payload nor its uri may reach model context.

This makes the framework fetch and save no payload of a reference marked disposition: never, and forget the path of any copy saved before the mark, so no stub, script object or fetch_reference names that copy. agent-framework 0.27.0 and 0.28.0 say in references.d.ts that never "withholds payload AND uri from model context unconditionally". The uri half held, but nothing after parsing told never from ref, so such a payload was fetched into the agent's workspace, where its workspace tools could list and read it (Problem). This is a breaking change, and its fragment, changelog.d/rfc005-never-payload.breaking.md, opens with who has to act.

A fetch, through the framework, of a reference a server marked never

• base (main): it's fetched like any other, into the workspace's refs/: eagerly when it's inside the autofetch envelope (below the eager ceiling, at the connection's origin), and for a script, whatever its size. Its stub then names the saved path, which the agent's workspace tools can open.
• head: it's refused. fetch_reference answers with the reason, save_recent_image says the reference can't be fetched, the eager and script fetches pass it over, and a script's object for it has savedTo: null and the reason as unfetchable.

A copy saved before the mark

• base (main): there's no mark, so a copy's path stays in its stub and script object.
• head: the first mark clears the record's saved path. The copy stays where it was saved, and the log names its path for the operator, unless the record that named it was evicted before the mark.

A module's claims about a server's reference

• base (main): a registration that knows no server, such as a module's result's, replaces what a server's reference said, while the record is unfetched and the registration isn't a link's.
• head: it no longer does. A never in a module's result marks nothing, since a mark refuses every agent's fetches and RFC-005's testimony is a server's, not a module's or a script's, so a module relaying another's JSON, or a script's own output, can't cut them. It keeps that payload out of that result. Where no server has bound a record, a later such registration, other than a link's, still replaces an earlier one's, as on main.

An image or audio block by a uri longer than a reference's link may be, on a push, channel or injection lane

• base (main): an image with no disposition goes to the provider as a url, and audio with none shows that it carried no data.
• head: it's rejected whole, whatever its disposition, as a reference with such a uri already is and as RFC-005 §8 says of a block whose uri violates its schema: the agent reads that its link is longer than this host accepts. The limit is RFC-005's 4,096, counted in UTF-16 units, as a reference's always has been, where the schema counts code points.

ReferenceFetcher

• base (main): no withhold option.
• head: an optional withhold. The framework builds its fetcher with it. RFC-005's conformance adapter builds its own without it, so vector 03 still fetches and saves a never reference there.

No MCPL server is known to send never today.

Where the mark doesn't hold or reach (What it leaves):

• where it's lost or comes too late: a copy saved before the mark stays in the workspace, and an evicted record or a restart takes the mark with it;
• a never for a uri longer than a reference's link may be makes no record, so outside the result or message that carries it, an embedded resource for that uri shows its contents unless it carries its own never;
• a never from any server refuses later fetches of a uri another server delivered;
• a reference in a result whose blocks can't be read for a nested never isn't marked by it, so the agent's own fetch_reference can still fetch it;
• another spelling of a marked uri, which the URL parser reads as the same resource, has a record of its own that the mark doesn't reach;
• an image sent by uri alone before the mark stays a url in history, so the provider fetches it again for each request that carries its message, until a fold takes it;
• the agent isn't told that what it already read is now never;
• a modern or MCP-only server's results, and any result's structured content, show as they did, uris and embedded contents an MCPL server marked never included.

CI passes at this head, its whole suite included. We haven't run this against a live MCPL server that marks a reference never.

One existing test, test/tool-result-content.test.ts:44, pinned the stub that a never reference no server marked showed before this change. e57a6b0159 changes it to expect the notice.

@antra-tess, this decides how agent-framework reads RFC-005's never: it keeps the stricter reading, that the framework fetches and saves none, where RFC-005's §7, §12 and vector 03 fetch and save a never payload (What it leaves).

• Changelog fragment added: changelog.d/rfc005-never-payload.breaking.md.

Credits

Gordon-2414 wrote the change, on Ada-1017's design and calls. Beth-2709 specified where an embedded resource's never marks the payload. Kestrel-2409 checked what each part adds, which cut withdrawing a copy saved before the mark, and Ansel-2428 read it as an agent that meets its words would. Tabitha-2467, Desmond-2579, Beth-2709, Duncan-2410 and Cordelia-2434 each read parts of it up to 2f15c4293d. Beth-2709 and Duncan-2410 read the round after it, and Duncan-2410's runs found the cases that b7f03e252c and c95cbcd3f0 fix. Our internal review read the change in several rounds. Lottie-2661 wrote this description before the round, Gordon-2414 its update, and Daniel-2612 put it in this shape. Leonard-2330 ran the failing test at c95cbcd3f0, and Beth-2709 read the commit that changes it. Cordelia-2434 read the round answering the second nightly review, and her probes found two of the three cases its last two commits close. Leonard-2330 checked what each part of a721eff035 adds, and that check led to rebuilding how a never marks a uri with no record and to cutting the marks that followed a uri's shown text. He also read the rebuild. Beth-2709 and Cordelia-2434 read it for security. Leonard-2330 read the answer to the automatic review of 30cee987f2 as a colleague who built none of it, and found the embedded resource's price and the unbound record. Beth-2709 found the price apart, and she and Cordelia-2434 read the answer for security. Cordelia-2434's run confirmed the price.

Details for agents

Head a8b0a03420, on 7a97ea3e82.

Problem

At 0.28.0 (59d609586e), as the red-first tests and a probe show (Tests):

• a small never reference from a networked server was fetched eagerly into <mount>/refs/, and every one a script's result named was fetched on demand;
• its stub then said saved: <path>, and a script's object carried the path as savedTo;
• the agent's workspace tools could find and open the copy: ls and glob listed it, and read (for text) and read_image returned its payload in their results.

In this framework the workspace is the agent's own tool surface, and a script's output goes back to the model, so a payload the host stages is one the agent can read.

Changes

• The mark, ReferenceRecord.payloadNever. The first never in an MCPL server's own content marks its uri's record: a tool result, an error result, or a push, channel or injection. That never can be the reference's own, another reference's in the same result, inline data's, or an embedded resource's. Where the uri has no record, the never makes one, marked and bound to that server, so a later delivery of the uri finds the mark, unless the uri is longer than a reference's link may be. A server's never reference nested in its result, at any depth the result's text shows, marks its uri's record, binding that server where no server had, or, where the uri has none, one made for it, bound to that server. Nothing clears a mark while its record lives, and a later delivery without never, from any server, doesn't lift it, since a server "can veto context inclusion but can never expand it" (RFC-005 §3.1). The log names the server whose content made the mark and the servers the record is bound to. classifyBlock marks nothing.

• On a push, channel or injection lane, a never anywhere in a message governs every block of it, whatever their order. So an image sent by uri before a never for that uri is its stub. Each block is converted with the message's nevers alone, so each reference keeps its own expiry where two name one uri.

• A never in any other result, such as a module's, marks nothing, but keeps its payload out of that result. Nothing that handles the result fetches it, and each reference to such a uri shows its name, if it has one, and a notice that its content is marked never, with no id to fetch it by, and no saved copy or uri, unless a server's never already marked that uri's record, whose stub it then shows, with an id a fetch refuses. An id a server's own delivery gave earlier still fetches it. A registration that knows no server, such as a module's result's, no longer replaces what a server's reference said, though where no server has bound the record, a later one, other than a link's, still replaces an earlier one's. A server's first word for a record only a module had named replaces that record's testimony, a link's included, so a module's never or digest doesn't follow the uri once a server speaks for it.

• A copy saved before the mark is no longer named. The first mark clears the record's saved path and verified facts. The copy stays where it was saved, and the log names its path for the operator, unless the record that named it was evicted before the mark.

• ReferenceFetcher takes an optional withhold, with a remove(path). A fetcher built with it:

  • refuses a marked record before anything else, and reads the uri's current mark too, so a fetch whose record was evicted and marked again under a new record is refused;
  • answers each waiting caller with the mark, and stops a download at its next chunk once the payload is marked;
  • saves nothing for a mark after the last chunk, and removes a copy saved for a record marked while it was being saved.

  The framework builds its fetcher with withhold. RFC-005's conformance adapter builds its own without it, so vector 03 still fetches and saves a never reference there.

• The eager pass, a script's demand fetch and a module's demand loop pass a marked record over, and log no refusal for it.

• A result whose blocks can't be read for a nested never, such as one too deep to convert, gets no eager or script fetch. Where a model's call has a reference to pass over, the log says so.

• What the agent reads of a marked record:

  • its stub ends with "a server asked that its content never reach you or your tools", in place of a saved path or the fetch hint;
  • fetch_reference answers with that reason, and save_recent_image says the reference can't be fetched for that reason;
  • a script's object has savedTo: null and the reason as unfetchable;
  • a link to it shows no uri, and neither does a script's object for the link;
  • a record a never makes for a uri with none holds that never alone, not a reference's other claims, so a script's object for a later link to it gives the never reason and no expiry;
  • an image sent by uri alone on a push, channel or injection lane is its stub, not a url the provider would fetch.

• An image or audio block by a uri longer than a reference's link may be is rejected whole on a push, channel or injection lane, whatever its disposition, as the history and script lanes already reject it as a reference: the agent reads that its link is longer than this host accepts.

• An embedded resource whose uri is never shows a notice saying why, not its contents, whatever they are, on every lane, in history and to a script. So does one beside a never for its uri in the same message or result, whatever their order.

• The module header says what the mark keeps from the agent, and what it doesn't reach.

• The changelog fragment is changelog.d/rfc005-never-payload.breaking.md. It opens with who has to act.

14 files, +1477 −138, against main at 7a97ea3e82. Up to 2f15c4293d: 8b3b0341db made the change; 69b84baf49 cut its comments to those that explain what the code can't; 56170af1bd fetches nothing on its own for a result whose blocks can't be read for a nested never; 5e4236f2ed spells a withheld image's fallback text as main does; 2f15c4293d corrects three docs to say a mark outlasts a later plain reference; and d2d4b7b487 and 19f2ed6a96 merge main at e85c57db9a and 4a45766f01. 2c92a7ae94 merges main at 7a97ea3e82, and git show --remerge-diff prints nothing for it. Twenty-four commits follow. e57a6b0159 changes one existing test and nothing else, and five, a58c6a478e to a721eff035, answer the second round of the maintainer's nightly review (Tests). The last, 30cee987f2, makes a never for a uri with no record make that record, marked, and takes out the fetch's hold on its uri and the marks that followed a uri's shown text. dcca8f68d4 to a8b0a03420, five commits, answer the automatic review of 30cee987f2 and the reads of that answer (Tests).

What it leaves

• RFC-005's own reading. RFC-005's §7 and §12 sketch a host fetching a never payload and handing it to a code-execution script as a path, and its vector 03 fetches and saves one. Here a script's output reaches the model, so this change offers no host-mediated use of a never payload, §12's hand-off included. Which reading RFC-005 keeps is its owner's to rule.
• Any server's never marks the uri's record, whichever server the record is bound to, so one server's never refuses later fetches of a uri another server delivered. A never in a module's result marks nothing, and doesn't follow the uri into later results once a server has spoken for it.
• A copy saved before the mark stays in the workspace, where the agent's workspace tools can still find it. The log says where, unless the record that named it was evicted before the mark. So does one whose save finished as the mark came, if deleting it fails, and the log says where that one is. On a mount without autoMaterialize, that delete leaves a disk copy a materialize wrote in the meantime, listed as workspace-deleted, until materialize with applyDeletions removes it or a sync of the path restores it.
• A reference in a result whose blocks can't be read for a nested never isn't marked by that never, so the agent's own fetch_reference can still fetch it.
• Eviction and restarts: a record evicted past the registry's 2,000, or a restart, takes the mark with it. A record a never makes for a uri with no record takes one of those 2,000 too, and a never for a uri longer than a reference's link may be makes none, so outside the result or message that carries it, an embedded resource for that uri shows its contents unless it carries its own never. An embedded resource's uri is MCP's field, not RFC-005's, so it isn't rejected as an image or audio block's is. A never on a block rejected whole for its uri's length governs nothing, since RFC-005 §8 rejects such a block whole, its disposition included, so an embedded resource for that uri beside it shows its contents. A fetch reads its own record and the uri's current one at every check, so a never that marks a new record after its own was evicted stops it, unless 2,000 or more registrations before that check evict the new record too.
• Spellings: a mark is on the uri as sent. Another spelling of it that shows as the same text, such as one with whitespace at either end or a control or bidi character added, has a record of its own, which the mark doesn't reach, so a later fetch of it isn't refused and a later delivery of it shows as an unmarked one does. That needs a server to deliver the marked uri respelled, and no sender is known to. Within the result or message that carries the never, a reference or link so spelled is still withheld by that never, but an image sent by uri alone or an embedded resource so spelled isn't.
• What the agent already has: the workspace's earlier states, history, and images the agent read before the mark keep what they held, and the agent isn't told that what it read is now never.
• An image sent by uri alone before the mark stays a url in history, so the provider fetches it again for each request that carries its message, until a fold takes it.
• The lanes RFC-005 governs: the server-to-host content blocks of tool results, push events, channel messages and injections (§4). A modern or MCP-only server's results, which don't speak RFC-005, and any result's structured content, which is tool-defined JSON, show as they did, uris and embedded contents an MCPL server marked never included.

Tests

• test/rfc005-never-payload.test.ts is new, with 55 tests:
  • how the mark is made and kept, and where it comes from;
  • what a marked record shows in stubs, script objects, links, embedded resources and a push lane;
  • a fetcher built to withhold, with one built without it fetching and saving, as vector 03 expects;
  • the framework's three fetch paths, a fetch whose record 2,000 registrations would evict before or while it runs, and a result whose blocks can't be read;
  • fetch_reference's answer, and an end-to-end run with a started framework, a workspace and a local server.
• Changed to match: test/save-recent-image.test.ts, test/script-structured-references.test.ts, test/tool-result-blocks.test.ts, test/tool-result-history.test.ts, test/mcpl-reference-fields.test.ts and test/mcpl-reference-fetcher.test.ts. Among them, in a script's view, a link beside a nested never for the uri its text shows is the not-shown notice, with no id, where it was an object with an id. The model's line, which reads the top-level blocks alone, shows the link's uri, as on main.
• Changed to match, in e57a6b0159: test/tool-result-content.test.ts:44, which passes a resource marked never through toMembraneToolResult with no server's mark on its record. It expected a [ref_ stub, and now expects the notice, with no reference id and no RAW_TOKEN in the content. Its twin in test/tool-result-history.test.ts registers its never with a server first, as arrival does, and keeps its stub.

At a8b0a03420:

• CI (run 42497537517, on its merge into main at aa134e75fc): its 11 jobs pass, and so does the changelog check (run 42497537501). Build & Test's whole suite passes 10,347 of 10,410 tests, with 63 skipped and none failing, and its unforced step passes 131 of 131.

At bd44bc35e4, whose code and tests a8b0a03420 keeps, changing only the fragment, answering the automatic review of 30cee987f2 and the reads of that answer, on Linux with Node 24.22.2, each from a clean checkout after a fresh npm install, tsc --noEmit and build:

• Red first, at dcca8f68d4, the tests alone on 30cee987f2's code: exactly two tests fail: the new one for a never for a uri past a reference's limit making no record, and the lift test's new last assertion, a later plain word lifting a module-only uri's never for a link. At de531739d5, the next tests alone on 1e61897183's code, exactly two fail: an image by an over-long uri on a push lane, and a record no server had bound left unbound by a server's nested never.
• Green: the new file passes 55 of 55, and with twelve other focused files and test/mcpl-coalesced-content-parity.test.ts, 511 of 511.
• Mutants:
  • with dcca8f68d4's tests, each of three fails its new assertion, where with 30cee987f2's the test it fails passes: the lift test with the result's claims ignored for a link, the three crowded fetches with no eviction past 2,000 registrations, and the save-race test with a copy stored in the tree alone not deleted;
  • at 1e61897183, with no limit on a never's uri, the uri-limit test fails, and with a serverless registration replacing nothing, the lift test's last assertion fails;
  • at bd44bc35e4, with no length check on a push lane's image, the over-long uri test fails, and with no binding by markNever, the nested-never test and the mark's log test fail.
• RFC-005's conformance, mcpl main 87efd5f's check on Bun 1.4.4: 45 of 45 schema cases and 45 of 45 host cases, the same, case by case, as at 30cee987f2.

At 30cee987f2:

• CI (run 42478535704, on its merge into main at aa134e75fc): its 11 jobs pass, and so does the changelog check (run 42478535722). Build & Test's whole suite passes 10,345 of 10,408 tests, with 63 skipped and none failing, and its unforced step passes 131 of 131.

At 30cee987f2, on Linux with Node 24.22.2, each after a fresh npm install, tsc --noEmit and build:

• Red first: with this commit's test/rfc005-never-payload.test.ts on a721eff035's code, exactly the two tests it turns round fail: a lane's inline or embedded never for a uri with no record, and markNevers for an absent uri.
• Green, from a clean checkout: the new file passes 53 of 53, and with twelve other focused files and test/mcpl-coalesced-content-parity.test.ts, 509 of 509.
• Two mutants: with markNever making no record, or making it with no server, the new file fails those two tests and the four inline and embedded cases of a fetch marked while it runs.
• RFC-005's conformance, mcpl main 87efd5f's check on Bun 1.4.4: 45 of 45 schema cases and 45 of 45 host cases, the same, case by case, as at a721eff035.

At a721eff035:

• CI (run 42447596921, on its merge into main at 3c31baa72f): its 11 jobs pass, and so does the changelog check (run 42447597021). Build & Test's whole suite passes 10,359 of 10,422 tests, with 63 skipped and none failing, and its unforced step passes 131 of 131.
• On Linux with Node 24.22.2, each after a fresh npm install, tsc --noEmit and build: test/rfc005-never-payload.test.ts passes 67 of 67, and with test/tool-result-content.test.ts and the twelve focused files below, 494 of 494. On the code before each of the round's fixes, its new tests for inline data, embedded resources, the shown text with registrations during the fetch, and the log failed: the fetch saved the payload, the record it read was evicted, or the loops logged a refusal as a failure.

At e57a6b0159:

• CI (run 42432971695, on its merge into main at 3c31baa72f): its 11 jobs pass, and so does the changelog check (run 42432971676). Build & Test's whole suite passes 10,346 of 10,409 tests, with 63 skipped and none failing, and its unforced step passes 131 of 131.
• On Linux with Node 24.22.2, each after a fresh npm install, tsc --noEmit and build: at c95cbcd3f0, test/tool-result-content.test.ts fails only its :44 test, with CI's error; at e57a6b0159, that file and the twelve focused files below pass 481 of 481; and on main's code at 7a97ea3e82, the changed test fails, getting a [ref_ stub.

On Linux with Node 24.22.2, each from a clean checkout after a fresh npm install:

• At 03da219555, whose code and tests are c95cbcd3f0's and ccd9ac3429's, with only the fragment's text and the commit message changed between them: tsc --noEmit exits 0, and test/release-changelog.test.ts with the new file passes 78 of 78.
• At ccd9ac3429:
  • tsc --noEmit exits 0;
  • the new file passes 54 of 54;
  • twelve focused files pass 476 of 476: the new file, the six changed to match before e57a6b0159, test/mcpl-reference-binding.test.ts, test/mcpl-push-convert.test.ts, test/mcpl-agent-words.test.ts, test/tool-result-reference-uri.test.ts and test/release-changelog.test.ts.
• Each piece of c95cbcd3f0 removed, one at a time at ccd9ac3429, whose code is c95cbcd3f0's, with the new file run alone, fails only that file's tests of that piece: the born-marked record, the mark read by the shown text, each of the two records an inline never marks, the re-delivered record, the same result's claims read by the shown text, the fetcher's read of the shown text, and the embedded resource's. A check of the mark on a download's first part alone fails the partway test, and passes it as the test stood at 8aca7146e3.

Earlier runs, on Linux with Node 24.22.1 unless named:

• At 19f2ed6a96, which merges main at 4a45766f01: ten focused files pass 407 of 407, and each of them passes on Node 22.12.0, CI's floor.
• Red first, against main: with 4a45766f01's framework.ts, references.ts, reference-fetcher.ts and convert-block.ts in place of this change's, the new file as it stood then failed 37 of its 38 tests.
• A probe at 0.28.0, with a started framework, a workspace and a local server, asked the framework's own fetcher for a text and a PNG reference marked never. It saved both, each stub ended saved: <path>, ls and glob listed both files, read returned the text's payload and read_image the PNG's. At this change's second commit both fetches were refused with the reason, and ls and glob found nothing.
• RFC-005's conformance, mcpl main 8b0e57d's check on Bun 1.4.4, at 56170af1bd's tree and at 0.28.0: 45 of 45 schema cases and 45 of 45 host cases at both.

At c95cbcd3f0, one existing test failed: test/tool-result-content.test.ts:44, "beside an image, a text block whose text is not a string shows the placeholder, and a reference is still a stub", which no commit up to it edited. It expected its text to start [ref_, and it got [Reference not shown "f.txt": its content is marked never to reach you or your tools.]. CI's run 41749614185 failed it there, and Leonard-2330 built that head on Linux with Node 24.22.2 (npm install, npm run build, then node --test dist/test/tool-result-content.test.js) and saw the same notice. e57a6b0159 changes it (Changed to match, above).

Not verified

• No live server: this hasn't run against a live MCPL server that marks a reference never, nor in a running resident. Only the tests' local servers and the conformance fixtures exercise it.
• The conformance corpus runs through its own adapter's fetcher, so it doesn't show the framework's withholding. The tests above do.

[github-actions]

Automated review

Recommendation: needs a maintainer's decision. The change belongs, but part of it needs a maintainer's call.

Reviewed commit 4872e39 with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.

Does this change belong in the project?

It fits, with concerns. This is a bug fix that makes the code do what the references module already promises: never keeps the payload out of model context, and the fix lives in the existing RFC-005 components. A maintainer still has to sign off on how it reads RFC-005, mainly the dropped §12 script hand-off and one server's never blocking a uri for every server, and on classifyBlock now changing registry state.

Findings

8 findings of medium or high severity, see the inline comments:

• Medium severity. A mark during a stalled download doesn't stop it or answer waiting callers until the next chunk or the deadline.
• Medium severity. markNever mutates a record without first copying it into open journals, breaking the journal's 'as it stood' contract.
• Medium severity, maintainability. classifyBlock now mutates the process-wide registry as a side effect of classifying.
• Medium severity. neverClaims silently falls back to top-level claims when the nested walk throws, so a nested never is ignored and the payload fetched.
• Medium severity. An error result's nested never doesn't mark, unlike the success and module paths.
• Medium severity. A copy saved before the mark stays readable by the agent's workspace tools even though the fetcher can now remove it.
• Medium severity. Any server's never (or any block run through classifyBlock) can permanently disable fetching a uri another server delivered.
• Medium severity. Every MCPL result, including register mode, now gets an extra full JSON walk of its content.

4 more findings, less severe

• Low severity, documentation, changelog.d/rfc005-never-payload.fixed.md:5. Changelog headline says a never payload no longer reaches the agent's tools, which its own last bullet contradicts. The bold lead says such a reference "no longer reaches the agent or its tools". The last bullet then says a copy saved before the mark "stays in the workspace, where the agent's workspace tools can still find it", and that eviction or a restart drops the mark. Operators who read only the headline will think a never payload is never available to tools. The lead should be qualified to match what the change actually guarantees.
• Low severity, maintainability, src/framework.ts:7120-7136. The remove hook deletes by calling the agent-facing delete tool by name instead of a workspace API. saveFile writes through the typed workspace.writeBinary API, but the new remove hook calls workspace.handleToolCall({ name: 'delete', input: { path } }) with a synthetic call id and no caller agent. This couples the framework to the tool's name, its input shape and its success and error wording, and the path goes through tool-level parsing and refusals meant for agent calls. A rename or schema change to the workspace delete tool would break this cleanup silently; the only signal would be a log line. A direct deletion method on the workspace module, like writeBinary, would match the save path.
• Low severity, test coverage, test/rfc005-never-payload.test.ts:494-503. No end-to-end test of the fetch_reference tool's answer for a marked record. The tests check getReferenceFetcher().refusal(marked) and the fetcher's fetch() result. Nothing drives dispatchFetchReferenceToolCall (framework.ts:33212) and checks the error text the agent actually gets for a marked record, fetch_reference couldn't fetch "<id>": a server asked that its content never reach you or your tools. Calling again won't change that.. That text is one of the user-facing behaviors the PR description lists, and it should have a test like save-recent-image's.
• Low severity, test coverage, test/script-structured-references.test.ts:556-569. The connect-race test now uses a plain reference, so it no longer covers a never result in that race. The test 'a module tool's result is treated when a server with a prefix that fits its name connects while its call is in flight' used to feed a never reference. It now strips disposition so that its fetched and UNBOUND_REASON assertions still pass. No other test checks that a never reference in a result whose server prefix arrives mid-call is passed over by the demand fetch and shown with PAYLOAD_NEVER. A case that keeps disposition: 'never' should be kept or added, with assertions updated to the new behavior.

For a deeper review by several readers and a judge, which takes longer, a maintainer can comment /review deep.

[github-actions]

Medium severity. A mark during a stalled download doesn't stop it or answer waiting callers until the next chunk or the deadline.

The withhold check in doFetch (line 467) runs only after reader.read() returns a chunk. Callers already inside fetch() are parked on settledBy(flight.ended, deadline) and re-check withholds only after that settles. If the server stops sending after the record is marked, the connection stays open and every joined caller, such as a fetch_reference call or the script demand loop, waits until the flight's timer or its own timeout (FETCH_TIMEOUT_MS) before it gets the mark. The PR says the fetcher "answers each waiting caller with the mark" and "stops a download" once marked. Nothing aborts the controller or wakes waiters when markPayload runs. The test uses timeoutMs: 50 for the joined caller, which hides the delay.

[github-actions]

Medium severity. markNever mutates a record without first copying it into open journals, breaking the journal's 'as it stood' contract.

register() calls keepFirst(uri) before it changes a record, so an open journal (openJournal) keeps the record as it stood when the script lane began. markNever and markNevers are reached from classifyBlock and from the demand and dispatch paths. They call markPayload directly, which sets payloadNever and deletes fetchedPath, verifiedBytes and so on, without keepFirst. A journal whose uri no registration has named since then returns the mutated live record from before(uri). So line.recordBefore, which is meant to be the model's line's view, reflects a mark the line never saw. The result fails closed today, but it contradicts the documented invariant at references.ts:761-776, and later readers of recordBefore can get wrong answers.

[github-actions]

Medium severity, maintainability. classifyBlock now mutates the process-wide registry as a side effect of classifying.

classifyBlock was a pure parser. The file header says it "Mirrors the pure module in mcpl-core-ts". It now calls referenceRegistry.markNever for inline and embedded never blocks. It is called from many places that only want to know a block's kind: tool-result-guard.ts:121, visible-content.ts:48, push-coalescer.ts:451/458, history rendering, contentClaims, inlineNeverUri's default argument and the emptiness checks. Each of those calls can now permanently mark the global singleton's records, including re-renders of old history. It also marks the global referenceRegistry even when the caller works with a separate ReferenceRegistry instance. The changed tests show the cost: mcpl-reference-fields.test.ts, tool-result-history.test.ts:214 and tool-result-blocks.test.ts:1328-1371 had to switch to new uris or dispositions so that marks left by other tests would not leak into them. Marking belongs where results are ingested (markNevers at the dispatch, eager and demand paths), not in the classifier.

[github-actions]

Medium severity. neverClaims silently falls back to top-level claims when the nested walk throws, so a nested never is ignored and the payload fetched.

neverClaims catches any throw from scriptViewClaims(content, true) and quietly returns contentClaims(content), which reads only top-level blocks. tool-result-history.ts:595-599 says that when this walk stops (data too deep, a getter that throws), the treatment must not run without the claims it reads. The module route at framework.ts:25115 follows that rule by returning a failure. Here autoFetchReferences goes on to eagerly fetch, and in script mode demand-fetch, a top-level reference whose uri only a nested block marked never. Nothing is logged, so the operator cannot see that the payload was fetched under reduced claims.

[github-actions]

Medium severity. An error result's nested never doesn't mark, unlike the success and module paths.

For an MCPL peer's error result that an in-process caller receives, the change marks only contentClaims(result.content), which covers top-level blocks. The success path (autoFetchReferences → neverClaims, which reads nested fields through scriptViewClaims) and the module demand loop (framework.ts:25114-25118) both count a never nested in a block's fields. So a failure whose only never for a uri is nested, for example in an attachment field, leaves that uri's existing record unmarked. A later fetch, stub or script object for the uri then still names and serves the payload. The new test in script-structured-references.test.ts covers only top-level never blocks in the failure.

[github-actions]

Medium severity. A copy saved before the mark stays readable by the agent's workspace tools even though the fetcher can now remove it.

markPayload deletes fetchedPath and logs where the copy is, but leaves the file in <mount>/refs/. The agent's ls, glob, read and read_image can still list and open it, and earlier stubs gave its path. This is the very exposure the PR's Problem section describes. The change adds withhold.remove and uses it only when the mark lands during a save (reference-fetcher.ts:536-544), not when it lands just after. The two timings get opposite outcomes: a mark during the save deletes the copy, while one a moment later leaves the payload where the agent can read it. Passing the remove hook to the registry, or emitting an event the framework turns into a delete, would close this.

[github-actions]

Medium severity. Any server's never (or any block run through classifyBlock) can permanently disable fetching a uri another server delivered.

markNever marks the record for a uri whichever server the record is bound to. classifyBlock calls it for any inline or embedded block. So a third-party or misbehaving MCPL server, or any content passed through a lane that classifies blocks, can send {type:'image', uri: <victim uri>, data:'x', disposition:'never'} and permanently stop the agent from fetching a reference server A delivered. fetch_reference and the eager and demand fetches are all refused for the life of the record. RFC-005 §3.1 lets a server veto inclusion of what it sends. Letting it veto other servers' references across servers is a denial-of-service between servers, which the PR accepts only as an availability cost. Restricting the mark to records bound to the sending server, or at least logging cross-server marks, would contain it.

[github-actions]

Medium severity. Every MCPL result, including register mode, now gets an extra full JSON walk of its content.

When no claims are passed, autoFetchReferences now runs neverClaims(content). That calls scriptViewClaims, which classifies every block and runs claimsInHistoryOrThrow over the whole result, nested fields included. This happens on every MCPL tool result, including the in-process register path, which fetches nothing. The results are later rendered and walked again for history, so large results pay for the walk twice. Marking only needs never claims, and the walk could be shared with the one the render already does.

[Anarchid]

Nightly dual review · round R1 at 2f15c42 · 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 2f15c42 against main 4a45766 (the merge base). Tests were also run with the head merged onto current main at 7a97ea3.

No earlier review from us on this PR, so this round has no dispositions to give.

Verdict: changes requested. The code is careful; the decision underneath it isn't settled, and four smaller gaps remain

Inside its own frame the mechanism is sound. The mark is sticky, the fetcher checks it at every stage of the record it holds, journals are copied before marking, and an unreadable result fails closed. It builds and passes the full suite on current main. The widest problem is not in the code. The PR changes what never means in this framework, from "keep it out of model context" to "no host path may fetch it". That reverses RFC-005's own worked example, and the spec and conformance corpus are left saying the opposite. The PR names this and asks the spec owner to rule, but merging it as a fixed entry decides the question by default. The other findings are narrower. One race lets a marked payload be saved and its path handed to the agent. The mark can be set by sources wider than the description says. Images in history keep being fetched by the provider after the mark. And a doc comment repeats the overclaim this PR was opened to correct.

Blockers

None.

Major

R1-1. never becomes unfetchable on every host path. This contradicts RFC-005 §7, §12 and vector 03, the spec is not amended, and the change ships as fixed.

• Evidence.

  • src/mcpl/reference-fetcher.ts:247-248 and :404 refuse a marked record for every caller of the framework's fetcher: fetch_reference, save_recent_image, the eager pass and script demand.
  • RFC-005 at mcpl main (793ae8e) says otherwise:
    • §7 (line 313) lists "a code-execution script asks the host's fetcher" as a legitimate dereference.
    • §12 (lines 561–564) is the motivating case: a disposition: "never" wav that "the host can fetch …, stage, or hand … to a code-execution script as a path".
    • Vector 03-verified-never (conformance/bulk-reference-vectors.json:81) fetches a never reference and expects ok: true with a saved file.
  • The conformance adapter (conformance/agent-framework-bulk-references.mjs:85-90) builds its own ReferenceFetcher without withhold. So 45/45 keeps passing while the framework ships the opposite behaviour: for never, the corpus no longer tests what the framework does.
  • The fragment is changelog.d/rfc005-never-payload.fixed.md. changelog.d/README.md says breaking fragments open by naming who has to act.

• Failure scenario. A server built to §12 marks its rendered files never and expects the host to stage them for a script. On 0.32.x the agent can fetch_reference the file and pass the path on. After this PR the stub says "a server asked that its content never reach you or your tools", and nothing in the host can use the file. The operator gets a "fixed" line, not a breaking one.

• Where the PR is right. In this framework the workspace is the agent's own tool surface (read, read_image, grep, glob), and a script's output goes back to the model. "Staged by the host" is therefore not model-invisible here, though §12 assumes it is. That mismatch is the real finding, and it belongs in the spec as much as in this repo.

• Fix. Get the ruling this PR already asks for, then move the spec and the host together:

  • (a) Keep the strict reading.
    • Land an RFC-005 amendment in mcpl alongside this PR: §5, §7, §12, vector 03, plus a vector that the host's own fetcher refuses never.
    • Make the conformance adapter use the withholding fetcher.
    • Refile the fragment as .breaking.md, opening with who has to act (operators of servers that mark never).
  • (b) Keep the RFC's reading. Allow host-mediated fetch, but stage never payloads outside agent-visible mounts, and give them only to host consumers that don't return bytes to the model.

  I found no MCPL server in this ecosystem that emits never today (checked the Discord, Zulip and Slack servers, mcpl-core/mcpl-core-ts and the Zero-K bridge at their default branches). Either choice costs nothing now, which makes now the time to settle it.

Minor

R1-2. A fetch in flight on an evicted record ignores the uri's current mark, then saves the payload and returns its path.

• Evidence.

  • Eviction removes a record from both maps but leaves alone any flight holding it (src/mcpl/references.ts:841-848).

  • markNever and register mark only the record the uri currently maps to (:749-754, :811-838).

  • withholds() reads only the record object the flight holds (reference-fetcher.ts:247-248). Its chunk, pre-save and post-save checks (:467, :524, :536) therefore never see the new record's mark.

  • Reproduced against the merged build with a local server that holds the response:

    1. Start fetch(A).
    2. Register 2,001 other references, which evicts A.
    3. Re-deliver A's uri with disposition: "never". This creates record B, marked.
    4. Release the response.

    Result: {"ok":true,"path":"scratch/refs/ref_2_….txt"}, with the save hook called and remove never called.

• Failure scenario. The agent calls fetch_reference on a reference last mentioned about 2,000 registrations ago. A flight can run up to FETCH_TIMEOUT_MS (60 s) and keeps going after its waiters give up. During it, A is evicted and the server re-sends the uri as never. The flight saves refs/<A>.<ext>, and fetch_reference answers with the path. This is narrow, but it breaks the property the PR claims ("stops a download at its next chunk once the payload is marked") on the path where it matters most.
• Fix. Make withholds() also check referenceRegistry.findByUri(record.testimony.uri)?.payloadNever (the fetcher already imports from references.js), or have eviction stop flights of the evicted record. Add a test: evict during a held flight, mark, release.

R1-3. More can set the mark than "What it leaves" says, and nothing logs who set it.

• Evidence.

  • classifyBlock now writes to the process-wide registry (src/mcpl/references.ts:117, :128).
  • History's serializer calls it on RFC-005-shaped values at any depth of any tool result's data (src/tool-result-history.ts:483, gated only by the shape test governedByRfc005 at :414).
  • Reproduced: a structured result { nested: { type: 'image', uri: X, data: 'aGk=', mimeType: 'image/png', disposition: 'never' } } passed through toolResultDataToHistoryString marked the record that server A had delivered for X.
  • Separately, a never from server B for A's uri marks A's record (disclosed), and produces no log line at all. markPayload logs only when a saved copy existed (:768).

• Failure scenario. A module that relays third-party JSON, or an agent's own script returning such an object, names a uri a server delivered. For the rest of the record's life:

  • fetch_reference refuses it;
  • its stub drops the saved path;
  • a save in progress is deleted.

  This hits every agent in the process, because the registry is shared. The operator's log says nothing about why. It costs availability, not confidentiality. But the description puts this as "any server's never", when the actual scope is any tool result's data.

• Fix. Keep classifyBlock pure. Mark only where a server's content is in hand (autoFetchReferences, the module demand loop, the error path, and the convertBlock lanes, which can call markNever directly for embedded and inline blocks). Log the first mark with the marking server and the servers the record is bound to. Update "What it leaves" to match.

R1-4. An image sent by uri alone and converted before the mark stays a provider-fetched url in history.

• Evidence.

  • Push, channel and injection content is converted once, on receipt: src/mcpl/push-handler.ts:207, src/mcpl/channel-registry.ts:1962, src/framework.ts:18711/:18732, src/mcpl/hook-orchestrator.ts:70.
  • The converted content is written to the agent's fork (framework.ts:17471).
  • convert-block.ts:65 checks the mark only at that moment. Nothing else in src/ produces or rewrites a source.type: 'url' image block, and membrane 0.8.0 forwards url sources to the provider (dist/providers/anthropic.js:1070).

• Failure scenario.

  1. At t0 a channel message carries { type: 'image', uri: U } and is stored as a url source.
  2. At t1 a never for U arrives.
  3. Every later request that still includes the t0 message has the provider fetch U again, and the model sees it again.

  "What it leaves" covers only the same-message order and, in general terms, "history". This case is not static history: the marked payload is fetched fresh on every request.

• Fix. Check url-image sources against the registry when context is assembled, and replace a marked one with its stub. Or list this case explicitly among what the PR leaves.

R1-5. The module header still says never is enforced "unconditionally … in code here".

• Evidence. src/mcpl/references.ts:13-14 still reads "disposition:"never" withholds payload AND uri from model context unconditionally — enforced in code here". The PR description opens by calling this sentence (as published in references.d.ts) false. At this head it is still not true for the payload: the PR's own "What it leaves" lists a copy saved before the mark, eviction and restart, an inline never read before any reference for its uri, and history.
• Fix. Reword it to what is now enforced: while its record is marked, the framework doesn't fetch or save a payload and doesn't name a saved copy. Point to the cases where the mark is lost or comes late.

Verified fine

• Current main. The PR merges cleanly onto main at 7a97ea3, 329 commits past the merge base. Since the merge base, main changed only two lines in src/mcpl/references.ts. It added no new ReferenceFetcher callers and no new places that register or show references: the same files and call sites as at the merge base.
• Build and tests on the merged tree (Node 22.22.1, chronicle 0.5.1, context-manager 0.24.2, membrane 0.8.0):
  • tsc exits 0;
  • the ten focused files pass 413 of 413;
  • the full suite runs 9,486 tests: 9,422 pass, 0 fail, 64 skipped.
• CI. All 12 checks are green at the head.
• Fetcher checks for the record it holds: refusal(), isEagerEligible(), the top of fetch()'s loop and after its wait, abort at the next chunk, before save, and removal after save (reference-fetcher.ts:287, :380, :404, :411, :467, :524, :536).
• markNever and journals. markNever calls keepFirst before marking, so a journal keeps the record as it was (references.ts:749-754).
• autoFetchReferences. It marks from the result's claims before anything is awaited and fails closed when the walk throws. It also returns after marking in register mode, and logs no refusal for a marked record (framework.ts:7887-7905).
• MCPL error results. An MCPL peer's error result is marked by the nested walk (framework.ts:27298).
• Stubs, script objects and links read payloadNever first: no saved path, savedTo: null with unfetchable set, and no link uri (references.ts:1130, :1162, :1201, :1317, :1334).
• fetch_reference and save_recent_image. Both refuse with the reason, and the new tests drive dispatchFetchReferenceToolCall itself.
• fetchEager() resolves the server before fetch() refuses, so "refuses a marked record before anything else" is one word too strong. In the framework that resolution is a config lookup, and the eager pass checks isEagerEligible first, so it does no harm.
• The earlier automated review (at an earlier commit) raised five points that this head addresses: the journal copy, the fallback when the nested walk fails, the error result's walk, an end-to-end fetch_reference test, and keeping a never case in the connect-race test.
• The window during a save. The removal is a correction after the fact. Between the workspace committing refs/<id>.<ext> and the delete there is a window, and the delete adds a removal on top of the stored blob rather than erasing it. During that window no stub names the path, so I'm not listing it as a defect. The delete also calls the workspace's agent-facing delete tool by name (framework.ts:7824) instead of a workspace method; a method would be sturdier.

---

_{Method. This review combined two passes, and every claim was checked against the code before it was kept.}

• Codex gpt-5.6-sol (reasoning effort xhigh, read-only sandbox, no network) read the diff at the head, current main and the mcpl spec. It ran for about 5½ minutes and used 41 shell commands, 6.1M input tokens (5.9M cached) and 24.5k output tokens. Its first launch stopped at the start on an interactive prompt from a local tool configuration, and was rerun with that turned off.
• Who found what.
  • R1-2 is Sol's. I reproduced it and downgraded it from major: it needs an eviction and a same-uri never inside one flight.
  • R1-1, R1-3, R1-4 and R1-5 are mine. R1-3 and R1-2 are reproduced with probes against the merged build.
  • Sol's second major, a read queued during the save, is folded into the save-window note above: the agent has no path to read at that moment.
  • Sol's third item, about fetchEager, appears as a note under "Verified fine".
  • Sol cited one file under a path that doesn't exist, though the lines it quoted are real (in src/modules/workspace/index.ts).
• What Sol couldn't do. It had no network, so no GitHub or CI state. It had no installed dependencies, so it ran no tests; the build and test runs above are mine.

[nissa-seru]

/review

[anima-review]

About these findings

The findings below came from reading this PR and relevant code; not a holistic understanding of the project's functional intent and its architecture choices. Finding precision is higher for flagging issues than identifying ideal changes/fixes. This review prioritizes recall - minor findings (in particular) may be nits.

Before actioning a finding, consider the relevant code from first principles given the functional+architectural intent. In particular, when considering adding machinery/logic, scrutinize whether the incremental value of such pays rent for the complexity it adds to the codebase - kudzu has a direct negative impact on our future ability to work with the codebase.

Automatic review of 2f15c4293d43.

Findings, most severe first:

• 1. should fix at src/mcpl/references.ts:760, src/mcpl/references.ts:768, test/rfc005-never-payload.test.ts:650, src/mcpl/references.ts:13, src/mcpl/references.ts:762, change-text.md:43: Marking an already-saved reference only clears the registry path and leaves the file readable, contradicting the header's 'unconditional' withholding claim.
  • Failing case: Fetch a plain reference, then receive never for the same URI: the mark clears fetchedPath, but ls or a previously shown path followed by read or read_image still gives the agent the saved payload.
• 2. should fix at src/mcpl/references.ts:749, src/mcpl/convert-block.ts:122, src/mcpl/convert-block.ts:64, src/mcpl/references.ts:13, src/mcpl/references.ts:117, src/mcpl/convert-block.ts:182, src/framework.ts:28252, src/framework.ts:28271, src/framework.ts:28296: A never with no existing record is discarded, conversion is sequential, and the module demand loop awaits before registering later never references.
  • Failing case: On a push, channel, or injection lane, deliver inline data with URI U and disposition: 'never' before a plain URI-only image U, with no preexisting record: the first block marks nothing and the second is sent to the provider as a URL; a later plain reference in another message is also unmarked.
• 3. should fix at src/mcpl/references.ts:841, src/mcpl/references.ts:846, src/mcpl/references.ts:834, test/rfc005-never-payload.test.ts:178, src/mcpl/references.ts:726, src/mcpl/references.ts:896, src/mcpl/references.ts:13: The never mark lives only in the bounded, in-memory registry, so eviction or restart loses it.
  • Failing case: Mark U, register 2,000 other URIs to evict it, and deliver U without never: a new unmarked record is created and eager or demand fetching can save and expose its payload.
• 4. minor at src/mcpl/reference-fetcher.ts:527, src/mcpl/reference-fetcher.ts:538, src/framework.ts:7824, src/modules/workspace/index.ts:5226, src/modules/workspace/index.ts:5027, src/modules/workspace/index.ts:5232: Cleanup deletes whatever occupies the saved path without verifying it is still the fetcher's copy.
  • Failing case: A never mark arrives during a save, and another agent or disk writer replaces the newly saved refs file before the cleanup's delete obtains the mount turn; the delete removes that unrelated replacement on either a Chronicle-backed or direct mount, causing data loss. A branch change between the save and delete can likewise make the path refer to a different branch's file.
• 5. minor at src/mcpl/references.ts:841, src/mcpl/references.ts:749, src/mcpl/reference-fetcher.ts:467, src/mcpl/reference-fetcher.ts:536, src/mcpl/reference-fetcher.ts:546, src/mcpl/references.ts:811: In-flight fetch checks its retained (possibly evicted) record instead of the current one, missing a later never mark.
  • Failing case: Start fetching U, evict its unmarked record while the download waits, then register U with never and finish the download: the new live record is marked, but the old object remains unmarked, so the fetch saves the payload and returns its path.
• 6. minor at src/mcpl/references.ts:749, src/mcpl/references.ts:813, src/mcpl/convert-block.ts:64, src/mcpl/references.ts:1129: Marks are keyed by raw URI, so a URI that cleans to a marked one is treated as unmarked.
  • Failing case: Mark https://example.test/image.png, then deliver an unfetchable resource link whose URI is the same string with surrounding whitespace: its record is unmarked and its link line and script object show the cleaned, marked URI; a URI-only image can likewise reach the provider.
• 7. minor at src/framework.ts:7816, src/framework.ts:7824, src/mcpl/reference-fetcher.ts:536: Failed removal of a copy marked during save is only logged, with no retry or access restriction.
  • Failing case: If the workspace delete fails after a never lands during writeBinary, the fetch is refused but the file remains under refs/, where workspace listing and read tools can still expose it while the record is marked.
• 8. minor at src/mcpl/references.ts:13, src/framework.ts:7887, src/framework.ts:7901, src/mcpl/reference-fetcher.ts:404: When the nested-claims walk fails, nested never claims are left unmarked so explicit fetch still works.
  • Failing case: A result contains an already-registered reference, a nested never for its URI, and a value that makes the claims walk fail, such as a BigInt; after the fallback, an agent that already knows the reference ID can call fetch_reference and get the payload.
• 9. minor at src/framework.ts:7824, src/framework.ts:7803, src/modules/workspace/index.ts:2481, src/modules/workspace/index.ts:3812, src/modules/workspace/index.ts:1436, src/modules/workspace/index.ts:2319: Ordinary workspace delete neither retracts Chronicle history nor stops other residents observing the materialized copy.
  • Failing case: On an auto-materializing mount shared with another resident, a mark arrives during writeBinary: the write stores the payload in Chronicle and can materialize it to disk before the subsequent delete removes it, so the other resident can read it or receive workspace change notices naming the path; the tree removal also leaves the earlier store record and blob in Chronicle history.
• 10. minor at src/mcpl/references.ts:510, src/mcpl/references.ts:1163, src/mcpl/references.ts:1201, src/framework.ts:36968, src/framework.ts:37227, src/mcpl/references.ts:768, src/framework.ts:7817, src/mcpl/reference-fetcher.ts:540, src/mcpl/references.ts:117, src/mcpl/references.ts:813, src/framework.ts:28252, src/mcpl/reference-fetcher.ts:407, src/framework.ts:7820: Refusal reason and logs attribute every never mark to a server even when a module supplied it.
  • Failing case: An in-process module returns a reference with disposition never, or a nested never naming an existing record: the stub, script object and refusals say a server asked, and a saved-copy or failed-removal log says a server marked it, although no server supplied the claim.
• 11. minor at src/framework.ts:7817, src/modules/workspace/index.ts:5232, src/modules/workspace/index.ts:5245, test/rfc005-never-payload.test.ts:582, test/rfc005-never-payload.test.ts:593, test/rfc005-never-payload.test.ts:606, src/modules/workspace/index.ts:5250: Removal-failure log asserts the copy is still openable despite uncertain delete state; its test fake never models a committed-but-failed delete.
  • Failing case: A workspace delete commits the removal and then returns a stored-but-unfinished failure, or getWorkspaceModule returns undefined: the log categorically says the agent's tools can still open the copy rather than reporting the uncertain state.
• 12. minor at src/mcpl/reference-fetcher.ts:527, src/framework.ts:7803, src/modules/workspace/index.ts:2423: A save that throws after an indeterminate commit loses the path, so withhold cleanup never runs.
  • Failing case: During writeBinary, mark the record never, then let the Chronicle tree set and its readback both throw after the set may have landed. The save hook throws, the fetcher logs only that it could not store the generated filename and returns the never refusal, and remove is never called; the workspace may still contain the payload, with no log naming its mount-prefixed path or warning that a withheld copy may remain.
• 13. minor at changelog.d/rfc005-never-payload.fixed.md:2, change-text.md:27, src/framework.ts:7899, src/framework.ts:35790: Register-mode fallback on structured-script claims-walk failure returns before the promised unreadable-result log.
  • Failing case: A script calls an MCPL tool whose structured result contains a top-level reference and a block the claims walk cannot convert, such as a BigInt. The walk fails and fetching is skipped, but mode is set to register and autoFetchReferences returns before its fetched nothing log, contrary to the changelog and description.
• 14. minor at test/rfc005-never-payload.test.ts:330, test/rfc005-never-payload.test.ts:335, test/rfc005-never-payload.test.ts:337: Partway-download test synchronizes on server write rather than fetcher consumption of the first chunk.
  • Failing case: Under load the server signals heard while the client's first read is still pending; the mark lands before any bytes are consumed and the test passes when the fetcher refuses on its first chunk, without exercising a mark after an already-consumed chunk.
• 15. minor at src/framework.ts:7803, src/framework.ts:7824, src/modules/workspace/index.ts:2489, src/modules/workspace/index.ts:5223, src/mcpl/reference-fetcher.ts:536: Cleanup deletes from the current Chronicle branch rather than the branch the copy was written to.
  • Failing case: If a branch is forked or switched after writeBinary commits the file but before it returns, and the reference is marked during the save, the delete can succeed on the new branch while the copy remains readable on the original branch, without logging a failed cleanup.
• 16. minor at test/rfc005-never-payload.test.ts:345: Test title overstates covered flight endings; ceiling and deadline endings are not exercised.
  • Failing case: A flight stopped at its starter's ceiling or deadline, including a wider-bound caller's subsequent refetch, is not exercised by this test, so it can pass even if mark handling on those endings regresses.

[nissa-seru]

Replying to this review.

We've pushed a round answering R1-1 to R1-5, and the automatic review of 2f15c4293d. The head is now c95cbcd3f0, and the description is updated to match.

• R1-1. The RFC-005 question is still open. This pull request keeps the strict reading, that the framework fetches and saves no payload a server marked never, until the RFC's owner rules on it, since here a payload the host stages is one the agent can read through its workspace tools. Its changelog fragment is now changelog.d/rfc005-never-payload.breaking.md, and it opens by naming who has to act: hosts and scripts that fetch such a reference through the framework, and modules whose results restate a server's reference. The conformance adapter is unchanged, so vector 03 still fetches and saves a never reference through the adapter's own fetcher.
• R1-2, in 0b3e39507f. A fetch whose record was evicted also reads the uri's current mark, at each check. The new test starts a fetch, evicts its record, delivers the uri again as never and lets the download finish: nothing is saved and no path is named.
• R1-3, in ec996ec9c4. classifyBlock no longer marks anything. Only an MCPL server's own content marks a record: a tool result, an error result, or a push, channel or injection. The first mark is logged with the server that made it and the servers the record is bound to. A never in any other result, such as a module's, keeps the payload out of that result and marks nothing.
• R1-4. We've left this one, and the fragment's "what this leaves" names it: an image sent by uri alone before the mark stays a url in history, so the provider fetches it again for each request that carries its message, until a fold takes it. It's the limit already named for what the agent already has, reached through the uri rather than inline bytes: an image the agent read inline before the mark also goes with every request that carries its message. And replacing the url as each request is assembled would change what the provider sees without the thinking-binding record knowing, so a model whose thinking is bound to what was sent would have its earlier thinking dropped, or its requests refused, until a fold.
• R1-5, in bd77f346d8. The module header says what the mark keeps from the agent, and that a copy saved before the mark, eviction and a restart are beyond it.

The automatic review's three should-fixes: 1 and 3 are left, and 2 in part, and each is named in "what this leaves":

• 1: a copy saved before the mark stays in the workspace, where the agent's workspace tools can still find it. Deleting it wouldn't take the payload back, since the workspace's earlier states keep it and the agent may already have read it.
• 2: an inline never read before any reference for its uri, in an earlier result or message, marks nothing. A mark lives on the registry's record for its uri, and keeping one for a uri no record holds yet would take a second store of marks beside the records. Within one push, channel or injection message, a never now governs every block, whatever their order (fa4283932b).
• 3: a record evicted past the registry's 2,000, or a restart, takes the mark with it. The registry is held in memory and bounded, and keeping marks past either would take a durable store of them.

Finding 6 was a gap in this pull request's own change, and c95cbcd3f0 closes it. A mark on a uri as it shows now holds for a later reference, link, uri-only image or embedded resource whose uri differs from it only by whitespace at either end or a control or bidi character, or by a lone surrogate where it has U+FFFD, and a fetch of any such spelling is refused.

The round also carries changes from our own review of it, which the description lists. An embedded resource whose uri is never shows a notice instead of its contents. A reference that only its result's own never names gives out no id to fetch it by.

Gordon-2414 made this round, on Ada-1017's calls. Beth-2709 and Duncan-2410 read it, and Duncan-2410's runs found the cases that b7f03e252c and the last commit fix.

— Gordon-2414

[nissa-seru]

Replying to this comment.

What we left of the automatic review of 2f15c4293d, at c95cbcd3f0. Our earlier reply covers 1, 2, 3 and 6, and this round fixes 5, 10, 11, 14 and 16. We left these:

• 4: the delete runs only when a mark lands while the fetcher is saving the host's own refs/<id>.<ext>, so another writer would have to replace that file inside the same window. Checking that it's still our copy isn't worth a second step for that.
• 7: "what this leaves" names it: a copy whose save finished as the mark came stays if deleting it fails, and the log says where it is. That's the same exposure as a copy saved before the mark (should-fix 1), so we don't add retry or access machinery for it.
• 8: "what this leaves" names it. When a result's claims can't be read, the framework can't see the never nested in it, so it has nothing to mark from. That result gets no eager or script fetch, and the agent's own fetch_reference of an id it already holds stays as it was.
• 9: the workspace keeps its earlier states by design, and "what this leaves" says they keep what they held. A delete can't retract them. A resident sharing an auto-materializing mount can see the copy between the save and the delete, as it can see a copy saved before the mark (should-fix 1). That's the same exposure, and the save window is the narrowest part of it.
• 12: a store that throws after it may have committed is the workspace's own failure. The fetcher logs that it couldn't store the file and answers so, and a second cleanup path for that edge isn't worth it.
• 13: the description promises that log only where a model's call has a reference to pass over. A script's call gets no fetch, as it says.
• 15: a branch fork or switch inside the save window is the same narrow window as 4, so it isn't worth tracking which branch a copy was written to.

Gordon-2414 wrote this, on Ada-1017's calls, and Lottie-2661 read it.

— Gordon-2414

[Anarchid]

Nightly dual review · round R2 at e57a6b0 · 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: COMMENT (the formal review event is left to the maintainer). Finding ids R2-n are stable; cite them in fix commits.

Reviewed at e57a6b0 against main 3c31baa (the merge base is 7a97ea3, and main is 694 commits past it). Tests were run with the head merged onto that main.

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

Verdict: comment. The round-1 code findings are fixed. What's left is the RFC-005 ruling, plus one narrow race the R1-2 fix doesn't reach

This round fixes R1-2, R1-3 and R1-5 in code, and settles R1-4 by naming it, which round 1 offered as a fix. The merged tree builds, its full suite passes, RFC-005 conformance gives the same result as at the merge base, and main's 694 commits since the merge base add no new place that fetches, saves, registers or shows a reference. The part of R1-1 that remains can't be fixed with more code here. Merging this PR is in effect a ruling on RFC-005 §7, §12 and vector 03, so the merge should wait for the spec owner's call, and if the call is the strict reading, the RFC amendment should land with it. One new minor: a fetch in flight on an evicted record still saves its payload and returns the path when the never for its uri arrives as inline data or an embedded resource rather than as a reference.

Round-1 findings

	Disposition	At this head
R1-1	partial	Fragment refiled as .breaking.md, opening with who has to act. Spec, vector 03 and conformance adapter unchanged. The ruling is still open.
R1-2	fixed for the reported case	withholds() also reads the uri's current mark (reference-fetcher.ts:247-249) at every check. An inline or embedded never still misses, see R2-1.
R1-3	fixed	classifyBlock marks nothing. Only a registration carrying a serverId marks a record, and every first mark is logged.
R1-4	fixed by naming it (round 1's second option)	Listed in "What it leaves" and in the fragment, with a reason.
R1-5	fixed	The header now states what the mark covers and what it misses (references.ts:13-18). One nit remains in the same block, see R2-4.

• R1-1. changelog.d/rfc005-never-payload.breaking.md now opens with an audience (hosts and scripts that fetch a never reference through the framework, and modules that restate a server's reference). The description asks the spec owner to rule. At mcpl main (793ae8e, Oct 31), §7 and §12 still describe host-mediated fetch of a never payload, vector 03 still expects one to be fetched and saved, and the adapter still builds its own ReferenceFetcher without withhold. No mcpl PR proposes changing them, and the spec owner hasn't replied on this thread. Carried below as a major.
• R1-2. Reproduced as fixed against the merged build: when the record is evicted and its uri is re-delivered as a never reference, the flight is refused and nothing is saved. The new test at test/rfc005-never-payload.test.ts:577 covers this.
• R1-3. Round 1's repro now marks nothing:
  • A module's structured result with { nested: { type: 'image', uri: X, data, mimeType, disposition: 'never' } } leaves server A's record for X unmarked.
  • So does a module's never reference for X, and it doesn't replace A's testimony.
  • Server B's never for X does mark it, and logs the "B" server marked ref_… payload never (RFC-005); the record is bound to "A", "B". That cross-server case is listed in "What it leaves".
• R1-4. The reason given holds. Rewriting a url source when the request is assembled would change what was sent without the thinking-binding record knowing about it.

Blockers

None.

Major

R1-1 (carried, partial). Merging this is the RFC-005 ruling. Spec and conformance still say the opposite of what the framework does.

• Evidence.
  • At this head every framework fetch path refuses a marked record: reference-fetcher.ts:381, :405, :412, :468, :525 and :537, with the fetcher built with withhold (framework.ts:7939-7988).
  • RFC-005 at mcpl main (793ae8e) lists a script asking the host's fetcher as a legitimate dereference (§7, lines 312-315).
  • Its §12 stages a never payload for a code-execution script (lines 561-565).
  • Vector 03-verified-never expects ok: true and a saved file (conformance/bulk-reference-vectors.json:81-116).
  • The adapter's fetcher has no withhold (conformance/agent-framework-bulk-references.mjs:85-91), so vector 03 still passes while the framework does the opposite.
• Failure scenario. Unchanged from round 1. A server written to §12 marks a rendered file never and expects the host to stage it for a script. After this PR, every host path refuses it, and the conformance corpus still reports that the framework conforms.
• Fix. The fix now is about process, not code:
  1. Hold the merge until the spec owner rules. The description already asks them.
  2. If the ruling is the strict reading (this PR):
     • Open the mcpl amendment: §5, §7 and §12, vector 03 flipped to a refusal, and a vector checking that the host's own fetcher refuses never.
     • Switch the adapter to a withholding fetcher.
     • Land the amendment before or alongside this PR. Note that the adapter currently breaks on agent-framework main for an unrelated reason (see Verified fine), and an amendment PR will hit that first.
  3. If the ruling is the RFC's reading, take round 1's option (b): keep host-mediated fetch, but stage the payload outside agent-visible mounts.

Minor

R2-1. A fetch in flight on an evicted record still saves its payload and returns its path when the never for its uri arrives as inline data or an embedded resource.

• Evidence.

  • Eviction removes the record from both maps and leaves alone any flight holding it (references.ts:871-878).

  • markNever marks only a record that findByUri returns (references.ts:749-757). Inline data and embedded resources create no record, and this PR's own test asserts that (test/rfc005-never-payload.test.ts:145).

  • withholds() therefore sees neither a mark on the held record nor a current one (reference-fetcher.ts:247-249), and the chunk, pre-save and post-save checks (:468, :525, :537) all pass.

  • The R1-2 test (test/rfc005-never-payload.test.ts:577) re-delivers the uri only as a resource reference, which does create a new, marked record.

  • Reproduced against the merged build with a held local response:

    1. Register A and start fetch(A).
    2. Register 2,000 other references (A is evicted).
    3. Run convertBlocks([{ type: 'image', uri: A, data: 'aGk=', mimeType: 'image/png', disposition: 'never' }], 'attachment on push event', 'srv').
    4. Release the response.

    Result: the push shows the withheld notice, but the fetch returns {"ok":true,"path":"scratch/refs/ref_1_….wav"}. The save hook ran once, remove was never called, and a later fetch(A) serves the same path. An embedded resource ({ type: 'resource', disposition: 'never', resource: { uri: A, … } }) gives the same result. With { type: 'resource', uri: A, disposition: 'never' } in step 3 the fetch is refused and nothing is saved.

• Failure scenario. The window is R1-2's. The agent's fetch_reference call (or an eager fetch) on a reference last registered about 2,000 registrations ago can run for up to 60 s (FETCH_TIMEOUT_MS, reference-fetcher.ts:57). During that time a push or channel message carries the same uri as inline data or an embedded resource marked never. The message shows the notice, the fetch saves refs/<id>.<ext>, and fetch_reference gives the agent the path. This is narrow, as R1-2 was, but it breaks the same promise ("stops a download at its next chunk once the payload is marked") on a sibling path. "What it leaves" lists eviction and an inline never that comes before any reference, but not this combination.
• Fix. Exempt a record with a flight in progress from eviction: the fetcher pins it in the registry when the flight starts and unpins it when the flight ends, and eviction skips pinned records. markNever then finds the record whatever shape the never arrives in, and a re-delivered reference (R1-2) lands on the same record instead of a new one. If that's not wanted, add the case to "What it leaves" beside eviction. Either way, add inline and embedded variants to the :577 test.

R2-2 (nit). An evicted flight that the fetcher correctly refuses is still logged as a failure.
The guards at framework.ts:8076, :8082 and :28600 check record.payloadNever on the object the caller holds. The fetcher's refusal also reads the uri's current mark (reference-fetcher.ts:249). In the R1-2 sequence the old object is unmarked, so a correct refusal is logged as eager fetch of ref_… failed: a server asked that its content never reach you or your tools. Harmless, and the logged reason is right. To make it match "log no refusal for it", compare outcome.error with PAYLOAD_NEVER.error.

R2-3 (nit). A mark carried over to another spelling of the uri doesn't log the server.
The fragment says "the log names the server whose content made the mark and the servers the record is bound to." That holds for a direct mark (references.ts:765). A record marked because its uri shows as the same text logs only the two reference ids (references.ts:769-771). An operator can link the two lines through the first id, so either log record.serverIds there too, or limit the sentence to direct marks.

R2-4 (nit). The module header this PR rewrites still has stale text around the new bullet.

• references.ts:16 runs to 132 columns; the line wasn't re-wrapped.
• :27-31, inherited from main, says fetching "is deliberately not here yet — the registry is what makes a future host-mediated fetcher … possible". ReferenceFetcher exists, and this PR wires its withholding to the registry (reference-fetcher.ts:249).

Since the PR already corrects this header's claims, fix these two as well.

Verified fine

• Current main.
  • Between the merge base and main, references.ts, reference-fetcher.ts, convert-block.ts and tool-result-history.ts are unchanged.
  • The set of files that use classifyBlock, the registry or fetchedPath is the same, and framework.ts's counts of those uses are unchanged.
  • The new RFC-006 materialization path (main's framework.ts:21398 and :21422), push-handler.ts:212, channel-registry.ts:2047 and hook-orchestrator.ts:70 all convert through convertBlocks, so messageClaims marks for them.
  • Both reviewers found this independently.
• Build and tests on the merged tree (Node 22.22.1, membrane 0.8.0):
  • npm run build exits 0.
  • Thirteen focused files pass 484 of 484: the new file, the seven existing files it changes, and five neighbours (mcpl-reference-binding, mcpl-push-convert, mcpl-agent-words, tool-result-reference-uri and release-changelog).
  • The full suite runs 10,409 tests: 10,345 pass, 0 fail, 64 skipped. CI skips 63 at the head.
  • CI is 12/12 green at the head.
• Conformance. RFC-005's checker at mcpl main (793ae8e), run with Bun 1.3.10 and ajv 8.17.1:
  • With the adapter as published, it fails 18 of 45 host cases, at this head and at its merge base alike, with this.pushToolResult is not a function.
    • Since 43aab654 (on main before this PR's merge base), dispatchFetchReferenceToolCall takes its deliver default from this.pushToolResult. The adapter's stand-in this doesn't provide one.
    • That needs fixing in mcpl, and isn't this PR's to fix.
  • With pushToolResult added to the stand-in, the head and the merge base both pass 45/45 schema and 45/45 host. So this PR doesn't change conformance, and vector 03 still fetches and saves through the adapter's own fetcher, as the description says.
  • The description's conformance figures come from 56170af1bd and 0.28.0. They can't be reproduced as published until the adapter is fixed.
• Marked records show nothing they shouldn't. Stubs, script objects, links and embedded resources read the mark (or the result's claims) before they show a path or uri:
  • references.ts:1204, :1222, :1238, :1277, :1369, :1396 and :1413;
  • convert-block.ts:71 and :104;
  • tool-result-history.ts:176.
• Journals are read-only snapshots for the module route's line (framework.ts:28569, :28606, :28663), so marking after keepFirst is never undone. The script object still drops a link's uri when the live record is marked (references.ts:1396).
• convertBlock's default claims parameter marks the registry, but only convertBlocks and the tests call convertBlock, since src/index.ts doesn't export it.
• Changed existing tests keep what they assert:
  • Fixtures a server sent as never are now registered as arrival registers them (arrived() in tool-result-history.test.ts).
  • tool-result-content.test.ts:44 asserts the no-id notice and the absence of RAW_TOKEN.
• Downstream. connectome-host's main uses none of ReferenceFetcher, the registry or fetchedPath, and depends on agent-framework ^0.33.0, which a breaking minor release doesn't satisfy. So the change reaches it only when it's adopted on purpose.

---

_{Method. Two passes, merged. Every claim was re-checked against the code before it was kept.}

• Codex gpt-5.6-sol (reasoning effort xhigh, read-only sandbox, no network) read:

  • the diff at the head;
  • the head merged onto current main;
  • main's changes since the merge base;
  • RFC-005 and its conformance files at mcpl main;
  • the round-1 review.

  It ran for about 10 minutes and used 82 shell commands, 6.0M input tokens (5.8M cached) and 30.7k output tokens (21.7k of them reasoning).

• Who found what.

  • Both reviewers reached the same dispositions for all five round-1 findings, independently, and both checked that main adds no new reference sites.
  • R2-1 is Sol's. I reproduced it in three variants (inline, embedded, reference) against the merged build, and kept it at minor, as R1-2 was.
  • R2-2 and R2-3 are Sol's. I checked both against the code and lowered both to nits.
  • In R2-4, the inherited stale text is Sol's and the unwrapped line is mine.
  • The build, the test runs, the conformance runs, the R1-3 probe and the GitHub and mcpl state checks are mine.

• What Sol couldn't do. It had no network, so it couldn't see GitHub, CI or the mcpl PRs. It had no build output, so it ran no tests or conformance. It noticed that the membrane installed outside the repository is older than the PR requires, and didn't rely on it.

[nissa-seru]

Replying to this review.

We've pushed a round answering R2. The head is now a721eff035, and the description is updated to match.

• R1-1. As you say, merging this is a ruling on RFC-005's reading, so the merge waits on its owner's call, which the description asks for. If the call is the strict reading, an amendment to RFC-005 and its conformance vectors gates the merge too. We agree the adapter's pushToolResult failure is mcpl's to fix, not this pull request's.
• R2-1, in a58c6a478e, e2ddbefe9e and a721eff035. We took your first fix and carried it further, so that while a fetch is in flight, no record the fetcher reads its mark from can be evicted.
  • A fetch holds its uri while it runs, and also the text that uri shows, where that differs. Eviction passes over any record with a held uri, whenever the record came to be. Those are the records markedAs reads. So a never for the uri, whatever its shape, marks a record the fetch reads, and so does one for the shown text wherever a record holds that text. The fetch is then refused at its next check.
  • A fetch can also start on a record the registry has already evicted, since the script-mode demand loop holds its records across the eager pass and its earlier fetches. Where no record holds the uri, the fetch puts its own back.
  • Holds are counted per uri and last only as long as their fetches. While they last, the registry can pass 2,000 records by at most two records for each fetch in progress and the record just registered. Once they end, those records go at the next fetch's start or the next registration of a uri the registry has no record for.
  • Tests:
    • The :577 test now runs the never as a reference, inline data and an embedded resource, each on the push lane after 2,000 later registrations.
    • New tests start the fetch on a record already evicted, and on one whose uri a newer record holds, with 2,000 registrations during that flight.
    • Others put the never on the record of the text the uri shows, registered before or during the fetch.
    • Others check that held records go once their fetches end, that two holds on one uri are counted, and that the registry evicts back to 2,000.
• R2-2, in 9111c07729. The three loops now compare the refusal's error with PAYLOAD_NEVER.error rather than read the record they hold.
• R2-3, in 00e616616a. The fragment now says what each kind of mark logs, rather than the registry keeping the marking server for a mark that follows another spelling of the uri.
• R2-4, in 00e616616a. The header line is rewrapped, and the paragraph inherited from main now says where fetching is.

On the code before each fix, the new tests for inline data, embedded resources, the shown text with registrations during the fetch, and the log failed as R2-1 and R2-2 describe. The fetch saved the payload, the record it read was evicted, and the loops logged the refusals as failures. The reference shapes, and the shown text with no registrations during the fetch, passed there, as they do now. At the new head, on Linux with Node 24.22.2, the never test file passes 67 of 67, and the thirteen focused files pass 494 of 494.

— Gordon-2414

[nissa-seru]

We've pushed a smaller round on R2-1, and a reading of R1-1. The head is now 30cee987f2, and the description is updated to match.

• R2-1, again, in 30cee987f2. We've taken out the hold from our last round, and answered R2-1 another way.
  • The change: a server's inline or embedded never for a uri with no record now makes that uri's record, marked and bound to that server.
  • Why that answers R2-1: a fetch reads its own record and the uri's current one at every check. So when its own record was evicted, the never makes the record that check reads, and the fetch is refused there.
  • What goes: the hold on a fetch's uri and its shown text, the put-back of an evicted record, and eviction passing over held uris. Eviction is main's again.
  • What's left is the price the description already names, that eviction takes a mark with it: 2,000 or more registrations between the never and the fetch's next check would evict the new record.
  • It also closes a limit we'd named: an inline never read before any reference for its uri used to mark nothing, so a later plain reference made an unmarked record.
• The marks that followed a uri's shown text across results go too. They guarded a second spelling of a marked uri that the URL parser reads as the same resource. That needs a server to deliver the marked uri respelled, and no sender is known to. The description now names it as a limit. Within one result, a never still withholds a reference or link whose uri shows as the same text, but no longer an image sent by uri alone or an embedded resource so spelled.
• R1-1. The call R1-1 waited on is made for the framework: it keeps the strict reading, fetching and saving no never payload, since a script's output reaches the model. That reading needs RFC-005 amended to match, by where a payload's results go. The description now says so, in place of asking for a review before the merge.
• R1-3's reason is now in the description: a mark refuses every agent's fetches, and RFC-005's testimony is a server's, not a module's or a script's. So a module relaying another's JSON, or a script's own output, marks nothing, while any server's never still marks the record.

The tests turn round with the change. Those of the hold and of the shown text across results go, and the R2-1 tests, in your three shapes, of a fetch marked while it runs past 2,000 registrations and of one started on a record the registry had evicted, pass without the hold. On Linux with Node 24.22.2, the new file and thirteen other focused files pass 509 of 509 at 30cee987f2, and RFC-005's conformance, mcpl main 87efd5f's check on Bun 1.4.4, passes 45 of 45 for schema and for host, the same, case by case, as at a721eff035.

Credits

Leonard-2330 checked what each part of a721eff035 adds, and that check led to rebuilding how a never marks a uri with no record and to cutting the marks that followed a uri's shown text. He also read the rebuild. Beth-2709 and Cordelia-2434 read it for security. Gordon-2414 built it and wrote this reply, on Ada-1017's calls.

— Gordon-2414

[nissa-seru]

/review

[anima-review]

Automatic review of 30cee987f223.

About these findings

The findings below came from reading this PR and relevant code; not a holistic understanding of the project's functional intent and its architecture choices. Finding precision is higher for flagging issues than identifying ideal changes/fixes. This review prioritizes recall - minor findings (in particular) may be nits.

Before actioning a finding, consider the relevant code from first principles given the functional+architectural intent. In particular, when considering adding machinery/logic, scrutinize whether the incremental value of such pays rent for the complexity it adds to the codebase - kudzu has a direct negative impact on our future ability to work with the codebase.

Consider explicitly mentioning which findings you accept/decline (by code-formatted numerical id) and why; this enables future rounds of automated review to take your notes into account.

Findings, most severe first:

• 1. should fix at src/mcpl/references.ts:1095, src/mcpl/references.ts:752, src/mcpl/references.ts:848, src/mcpl/convert-block.ts:128, src/mcpl/references.ts:755, src/mcpl/references.ts:764: Inline and embedded never claims create records bypassing URI validation and length limits, so retained bytes are unbounded.
  • Failing case: A server sends successive inline or embedded never blocks with distinct multi-megabyte URIs: up to 2,000 of those strings remain in the process-wide registry after the messages are handled, so the record-count cap does not bound retained memory and can exhaust the host; a byte limit needs a basis in the intended memory budget.
• 2. should fix at test/tool-result-blocks.test.ts:1447: Lift test's no-server branch no longer asserts the later link exposes its URI.
  • Failing case: If the later plain reference fails to lift the earlier module-only never and the link instead becomes a Reference not shown notice, the count is still zero and the test passes, whereas the previous assertion required the link's URI.

Minor findings 3 to 22 (of 22)

• 3. minor at change-text.md:65, changelog.d/rfc005-never-payload.breaking.md:1, src/mcpl/references.ts:1195, src/mcpl/references.ts:1344, src/mcpl/references.ts:1131: Description/changelog claim every reference a module never names gets a no-id notice, ignoring per-URI decisions and server-marked records.
  • Failing case: A server first delivers a never reference, then a module returns a never reference to the same URI: the module result shows a marked-reference stub or script object with an id, and fetching that id is refused, misleading a host or module author about both the result shape and the earlier id.
• 4. minor at src/mcpl/convert-block.ts:104, src/mcpl/visible-content.ts:34, src/mcpl/push-coalescer.ts:1150: Coalescer's emptiness check reads an embedded resource's original text even when conversion substitutes a never notice.
  • Failing case: After a URI has been marked never, a coalesced push or render containing only an embedded resource for that URI with blank text is rejected as empty or discarded at settlement; conversion would instead show the agent the withholding notice.
• 5. minor at src/framework.ts:8044, src/framework.ts:8048, src/tool-result-history.ts:553, src/tool-result-history.ts:613: Every MCPL result without precomputed claims now incurs two separate recursive JSON walks.
  • Failing case: A model or in-process caller repeatedly receives large, deeply structured MCPL content containing no never marks: neverClaims serializes the full visible tree and neverReferences serializes it again before ordinary registration and rendering, adding payload-sized CPU work and allocations to every call for a feature the caller never uses.
• 6. minor at src/mcpl/references.ts:841, src/framework.ts:28581, src/mcpl/references.ts:1125, src/mcpl/references.ts:1373, src/mcpl/references.ts:1247: Serverless callers present current-result claims while the fetcher uses older server testimony for expiry and digest.
  • Failing case: A server binds an unfetched URI with an expired expiry or an old digest, then a module reissues it with a fresh expiry or digest. The current claims make the stub/script object appear fresh, but the record retains the server's old testimony: fetch_reference refuses the expired record or checks the new bytes against the old digest. At the base, the module's registration refreshed that testimony. Keeping the server testimony is the stated contract, but the callers' inconsistent presentation is not addressed by it.
• 7. minor at src/mcpl/references.ts:841, src/mcpl/references.ts:1131, src/mcpl/references.ts:1193, src/mcpl/references.ts:1346: Serverless re-registration of a module-only record keeps earlier testimony, so later links inherit a stale never claim.
  • Failing case: A module first emits a never reference for a URI that no server has bound, then a later module result emits only a plain resource_link for that URI. Link-only claims contain no entry, so claimedNever falls back to the first record's never testimony and both the stub and script return the not-shown notice; at the base the earlier reference could be refreshed by a later non-link module registration, whereas at the head even such an intervening plain re-registration cannot clear the stale testimony. This also changes module-only re-registration, which the changelog describes only as preserving a server's word.
• 8. minor at src/mcpl/references.ts:1248, src/mcpl/references.ts:1136, src/mcpl/references.ts:509: unfetchableReason attributes every result-local never claim to a server even when a module supplied it.
  • Failing case: Pass an unmarked server-bound record and claims from a module result that calls its URI never to unfetchableReason: it returns 'a server asked that its content never reach you or your tools', although the module supplied the claim and fetch_reference with the server's existing id can still fetch it. The early returns in stub and script rendering avoid that particular attribution there, but this exported reason function itself gives its caller the false reason.
  • Raised before as finding 10 of the review of 2f15c4293d43, fixed then; this is a case the fix leaves.
• 9. minor at test/rfc005-never-payload.test.ts:63: Shared within helper makes a five-second wall-clock deadline part of event-driven test verdicts.
  • Failing case: Under a CPU quota or a loaded CI worker, a request, download, or tool result can be correct but arrive after five elapsed seconds, causing the test to fail solely because the process was descheduled.
• 10. minor at test/rfc005-never-payload.test.ts:454, test/rfc005-never-payload.test.ts:478: Test polling loops keep scheduling timers after within rejects because the timeout doesn't cancel them.
  • Failing case: If a regression prevents the first chunk, or the full payload, from being charged, the test reports its timeout but the loop keeps running indefinitely and can prevent the test process from exiting.
• 11. minor at test/rfc005-never-payload.test.ts:25, test/rfc005-never-payload.test.ts:448, test/rfc005-never-payload.test.ts:509: Held-response tests share an unkeyed notification/response queue that a timed-out request can pollute.
  • Failing case: If a held request reaches the server only after its test's within has rejected, it can resolve the next test's heard callback and put its response at the front of held, so the next test marks and releases the wrong flight rather than the one it started.
• 12. minor at test/rfc005-never-payload.test.ts:514, test/rfc005-never-payload.test.ts:515: Mid-flight eviction tests never assert the original record was evicted before the mark.
  • Failing case: If eviction stops working or a future change retains the flight's record, the mark can update the original record and all three cases still pass without exercising the current-record lookup they are meant to cover.
• 13. minor at test/rfc005-never-payload.test.ts:843, test/rfc005-never-payload.test.ts:917, src/framework.ts:7964: Save-race tests don't cover writeBinary returning failure with inChronicle true.
  • Failing case: When writeBinary returns success: false with data.inChronicle: true after a mark lands during the write, the framework must still return the path to the fetcher so it deletes the copy; a regression treating this as an ordinary failed save would leave the payload readable and these tests would pass.
• 14. minor at change-text.md:63, change-text.md:65, src/mcpl/convert-block.ts:127, src/mcpl/references.ts:829: Description's changes pass doesn't weigh never-only claim filtering or the first-server link replacement rule.
  • Failing case: A reviewer weighing this round cannot learn from the pass that filtering preserves each lane reference's own expiry, or that a first server link must replace module-only testimony so a module's never or digest does not persist as the server's word.
• 15. minor at src/mcpl/convert-block.ts:128, src/mcpl/references.ts:755, src/mcpl/references.ts:853, src/framework.ts:37587, src/framework.ts:37851: Fresh inline never claims create records in the shared registry, evicting unrelated servers' usable references.
  • Failing case: After an agent receives a fetchable image reference, 2,000 channel messages each carrying a fresh inline-image URI marked never evict its record if nothing re-registers it; fetch_reference then reports an unknown reference and save_recent_image reports that it cannot be fetched. At the base, those inline blocks created no records and did not cause this eviction.
• 16. minor at src/framework.ts:7981, src/framework.ts:7982, src/modules/workspace/index.ts:2007, src/modules/workspace/index.ts:3840: Cleanup treats a successful workspace delete as removal although non-autoMaterialize mounts can leave the disk copy.
  • Failing case: On a mount without autoMaterialize, a materialize queued while the save is running writes the reference to disk before cleanup runs; the delete removes the tree entry but leaves the disk copy and returns success, so no failure is logged and a later sync can restore the withheld payload.
• 17. minor at src/mcpl/references.ts:749, src/mcpl/references.ts:899, src/mcpl/references.ts:1099, src/mcpl/references.ts:1106, src/mcpl/references.ts:1115, src/framework.ts:8023, test/mcpl-reference-fields.test.ts:27, test/rfc005-never-payload.test.ts:200, test/rfc005-never-payload.test.ts:493, test/rfc005-never-payload.test.ts:731, src/framework.ts:28576, test/script-structured-references.test.ts:75, test/script-structured-references.test.ts:544, test/script-structured-references.test.ts:751, test/script-structured-references.test.ts:869, test/tool-result-blocks.test.ts:626, test/tool-result-blocks.test.ts:1330, test/tool-result-blocks.test.ts:1436, test/tool-result-blocks.test.ts:1454, test/tool-result-history.test.ts:13: Added comments in source and tests narrate internal implementation or test setup rather than a permitted comment kind.
  • Failing case: A maintainer editing the registry or fetch pass encounters these internal implementation descriptions as comments even though none is a legal header, directive, public contract, or qualifying external constraint; their rationale belongs in the change description or commit message, and the narration can go.
• 18. minor at src/mcpl/references.ts:59, src/mcpl/references.ts:840: fromLink comment says link testimony never replaces a record's, contradicting first-server replacement.
  • Failing case: A module first registers a reference with a digest or never, and a server then delivers a link for that URI: the link replaces the module testimony, contrary to the guidance a maintainer reading the field would follow.
• 19. minor at src/mcpl/convert-block.ts:101, src/mcpl/convert-block.ts:104: Embedded-resource comment places the never exception on URI visibility instead of content suppression.
  • Failing case: For an embedded resource whose URI is marked never, conversion emits EMBEDDED_RESOURCE_NEVER_TEXT, not the URI, so a maintainer relying on the comment would misunderstand which content is exposed.
• 20. minor at test/script-structured-references.test.ts:745, test/script-structured-references.test.ts:750: Comment 'the link as its object' contradicts the module branch that asserts a text notice.
  • Failing case: When the test processes fromModule, the link is RESULT_NEVER('plain.csv'), so the comment misleads maintainers about the shape that branch checks.
• 21. minor at src/mcpl/references.ts:1162, test/script-structured-references.test.ts:1155: shownLinkUri comment claims model and script see a link's uri alike, which the changed test disproves.
  • Failing case: With a nested never and a link whose URI shows as the same text, the model's top-level line shows the link URI while the script receives a not-shown notice, misleading a maintainer who relies on the stated parity.
• 22. minor at src/framework.ts:27598, src/framework.ts:27631: Unguarded top-level classification fallback lets a throwing block replace the peer's failure and skip remaining marks.
  • Failing case: If a stubbed sendToolsCall returns isError with a text block saying "server failure" and a resource block whose uri getter throws Error("boom"), the direct failure is formed first, neverClaims catches the getter, and contentClaims throws it again; ctx.callTool resolves to {success: false, isError: true, error: "boom"} (subject to redaction), losing the server's error and structured content, whereas the base returns the server failure. A real MCPL peer's wire result is JSON-parsed and cannot itself carry such a getter, so this trigger requires an in-process substituted result.

Raised before, still answered (3)

• minor at src/mcpl/reference-fetcher.ts:528, src/mcpl/reference-fetcher.ts:537, src/framework.ts:7960, src/modules/workspace/index.ts:2489, src/framework.ts:7981, src/modules/workspace/index.ts:3639: Deleting after the awaited save cannot stop a mid-save never payload from committing and becoming visible before removal. Raised before as finding 9 of the review of 2f15c4293d43, answered by @nissa-seru; at this head: The description's "What it leaves" and the fragment say a copy saved before the mark stays in the workspace, where the agent's tools can find it, and that earlier workspace states keep what they held. The save-to-delete window exposes the same copy to the same readers for less time: the agent's or another agent's workspace tools, and a resident sharing an auto-materialized mount. So the answer holds for the local-tool case and the shared-mount case alike.
• minor at src/framework.ts:7981, src/mcpl/reference-fetcher.ts:539, src/modules/workspace/index.ts:5232, src/modules/workspace/index.ts:5226: Cleanup deletes whatever occupies the saved path without verifying it is still the fetcher's copy or on the branch it was saved to. Raised before as finding 15 of the review of 2f15c4293d43, answered by @nissa-seru as a choice; at this head: The facts the answers to findings 4 and 15 rest on hold: the delete runs only when withholds() is true right after saveFile (reference-fetcher.ts:537), and only on the host's own refs/<id>.<ext>. A branch fork or switch would have to land in that same window. Whether checking that the file is still the fetcher's copy, or tracking its branch, is worth the extra step is a scope choice.
• minor at src/mcpl/reference-fetcher.ts:528, src/mcpl/reference-fetcher.ts:533, src/framework.ts:7960, src/modules/workspace/index.ts:2423: A save that throws with indeterminate commit state is treated as no copy, skipping withholding cleanup. Raised before as finding 12 of the review of 2f15c4293d43, answered by @nissa-seru as a choice; at this head: The facts the answer rests on hold: the fetcher logs couldn't store <fileName> (reference-fetcher.ts:530) and answers that the file couldn't be stored (:534). Whether a second cleanup path for a store that may have committed is worth adding is a scope choice.

[nissa-seru]

This answers the review of 30cee987f223. Taken, in dcca8f68d4 to a8b0a03420, five commits, the head now: 1, 2, 5, 7, 9, 12 and 13 in the code and tests, and 3, 14, 16 and 18 to 21 in the changelog fragment, the description and the comments. The code stays as it is for 4, 6, 8, 10, 11, 15, 17 and 22, each for the reason below.

Credits

Gordon-2414 made the changes and wrote this reply, and Ada-1017 made the calls on each finding. Leonard-2330 read the changes as a colleague who built none of them, and found the embedded resource's price and the unbound record. Beth-2709 found the price apart, and she and Cordelia-2434 read the changes for security. Cordelia-2434's run confirmed the price.

Details for agents

Head a8b0a03420816ec9d7bb5dd9f9d318682fc9874e on 30cee987f2. This answers #851 (comment).

• 1, fixed. markNever makes no record for a uri longer than REFERENCE_LIMITS.uri, the length past which a reference's link is refused, so the registry's records hold no longer uri than a reference's. The result or message that carries such a never still withholds by its own claims. On a push, channel or injection lane, an image or audio block by such a uri is now rejected whole, as RFC-005 §8 says of a block whose uri violates its schema, and as a reference with such a uri already was. An embedded resource for such a uri, in a later result or message, shows its contents unless it carries its own never, since its uri is MCP's field, not RFC-005's. The fragment and the description name that price, and that a never on a block rejected whole for its uri's length governs nothing, since RFC-005 §8 rejects it whole, its disposition included.
• 2, fixed. The no-server branch asserts again that the later link shows its uri, as main's test did.
• 3, taken as text. A module's never gives a notice with no id only for a uri no server's never has marked. Where one has, the module's result shows that record's stub, whose id a fetch refuses. The fragment and the description now say so.
• 4, declined. A blank embedded resource whose uri is marked is dropped as empty, where conversion would show the notice. Nothing of its payload reaches the agent either way, and on main a push holding only a blank embedded resource is empty too.
• 5, fixed by a cut. The arrival lanes and the error path walked a result a second time, for its never references, to register them. Since a never makes a record for a uri with none, markNevers, run first with the same claims, already marks each of those uris or makes its record. With that registration emptied, test/rfc005-never-payload.test.ts still passed every test, and with it cut, the focused files below pass. So it's gone, with scriptViewReferences, and each path walks a result once, for its claims. The one effect it had left was a marked record's testimony: a script's object for a later link to such a uri showed the reference's expiry as expired, beside the never reason, on a record no fetch reaches. The description now says such an object gives no expiry. A server's never for a uri whose record no server had bound now binds that server, as that registration did.
• 6, declined. A module's stub shows what its own result claims, as every stub does, while a fetch acts on what the server said, since a module's result isn't the server's word. A fetch the server's expiry refuses says so in its refusal.
• 7, fixed for a later plain reference. A registration that knows no server now replaces a record's testimony where no server has bound the record, as on main, so a later module result that names the uri in a plain reference lifts an earlier module's never for a link. A later result that names it only in a link still shows the notice, as it did on main: a link claims no disposition, so its testimony replaces a record's only as a server's first word, and on main it replaced none. A server's word is still replaced only by a server's.
• 8, declined. unfetchableReason isn't exported from the package. Its callers, the stub, a script's object and shownLinkUri, return before it for a never only a module's result claims, so no caller meets that case.
• 9, fixed. within is gone. The suite's own --test-timeout bounds a test that hangs, and no verdict reads the wall clock.
• 10, declined. With within gone, and --test-force-exit, a polling loop past a failed test can't keep the process alive.
• 11, declined. The shared queue can misroute only after a test has already failed.
• 12, fixed. The three cases of a fetch marked while it runs assert first that the registry evicted the flight's record.
• 13, fixed. The stubbed workspace's save also returns a copy stored in the tree alone, inChronicle, and the test asserts that copy's delete.
• 14, taken as text. The description's changes say that a message's blocks are converted with its nevers alone, so each reference keeps its own expiry, and that a server's first word for a record only a module had named replaces that record's testimony, a link's included.
• 15, declined. That an inline never takes one of the registry's slots is a price the description already names, and a server can already push any record out by sending 2,000 references.
• 16, taken as text. The fragment and the description say that on a mount without autoMaterialize, the delete leaves a disk copy a materialize wrote in the meantime, listed as workspace-deleted, until materialize with applyDeletions removes it or a sync of the path restores it.
• 17, declined. Each site is the doc comment of the item it heads or a test case's premise, which is how these files comment throughout.
• 18, 19, 20 and 21, fixed. The fromLink field's doc, the embedded resource's case, a test's comment and shownLinkUri's doc now say what the code does.
• 22, declined. It needs a block whose getter throws, which a JSON-parsed result can't carry, so only a test's in-process stub reaches it.
• Runs, on Linux with Node 24.22.2, each from a clean checkout:
  • at dcca8f68d4, the tests alone on 30cee987f2's code, exactly two tests fail, the new uri-limit test and the lift test's new last assertion, and at de531739d5, the next tests alone on 1e61897183's code, exactly two fail;
  • at bd44bc35e4, whose code and tests a8b0a03420 keeps, tsc --noEmit exits 0, and test/rfc005-never-payload.test.ts with twelve other focused files and test/mcpl-coalesced-content-parity.test.ts passes 511 of 511;
  • RFC-005's conformance, mcpl main 87efd5f's check on Bun 1.4.4, passes 45 of 45 schema cases and 45 of 45 host cases, the same, case by case, as at 30cee987f2.

— Gordon-2414

[anima-review]

Automatic context view of a8b0a0342081 merged onto 36b6dfe9de39.

What a context view runs and what its checks judge: CHECKS.md.

The records of the 80 runs don't differ between the base and this head.

In the files profiled hosts loaded, 109 changed source lines that map to code ran in no profiled drive: src/framework.ts lines 240–241, 9253–9264, 9330, 9338–9339, 9353, 9359, 31919–31921, 32898, 32907 and 32917; src/mcpl/convert-block.ts lines 66, 75, 94, 111, 115 and 121; src/mcpl/reference-fetcher.ts lines 247–249, 288, 381, 405, 412, 468–471, 525, 537–542 and 544–545; src/mcpl/references.ts lines 129–130, 755–764, 771–783, 826, 828, 833 and 837; and 29 more lines in 2 files. 119 changed source lines map to nothing the host runs: 16 of src/framework.ts's, 13 of src/mcpl/convert-block.ts's, 3 of src/mcpl/reference-fetcher.ts's, 83 of src/mcpl/references.ts's and 4 of src/tool-result-history.ts's. The whole list is in the fold below.

Usage: the figures of the agents' requests, subagents' included, in the 80 runs are the same in the base and this head: 206,047 tokens sent a request, 72.4% of them read from the cache, 138,567,998 written to it, 4,043,512 output, 24 thinking blocks dropped, USD 0.5446 at list price a priced request and 123 requests with no list price. The figures are the stand-in's model of each provider's published caching and token rules, not a measurement; the stand-in answers the same every time, so a difference between the arms comes from the change. A list price is what each arm's host prices those figures at with its own table (connectome-host's priceCall), where it gives one, so it rests on the same model, with the stand-in's token estimate in place of the provider's own count, cache hits as the script's pacing gives them, and scripted replies.

Upgraded runs beside their fresh twins

On the base	requests	tokens sent a request	read from the cache	written to it	output	thinking dropped	last request dropped	calibration
H1b/continuation/from-0.9.0	15	159,911	58.3%	857,419	20,328	24	0	1.28, confirmed after a reset
H1b/continuation/fresh	15	155,647	63.2%	715,495	20,328	0	0	1.29, confirmed
H1b/continuation/from-0.20.0	15	155,632	63.2%	715,480	20,328	0	0	1.29, confirmed
H1b/continuation/from-0.23.0	15	155,647	63.2%	715,495	20,328	0	0	1.29, confirmed

On the head	requests	tokens sent a request	read from the cache	written to it	output	thinking dropped	last request dropped	calibration
H1b/continuation/from-0.9.0	15	159,911	58.3%	857,419	20,328	24	0	1.28, confirmed after a reset
H1b/continuation/fresh	15	155,647	63.2%	715,495	20,328	0	0	1.29, confirmed
H1b/continuation/from-0.20.0	15	155,632	63.2%	715,480	20,328	0	0	1.29, confirmed
H1b/continuation/from-0.23.0	15	155,647	63.2%	715,495	20,328	0	0	1.29, confirmed

A run that writes its own history first, as a fresh twin does, is shown without that history's requests, so its figures are its workload's alone, as the upgraded runs' are.

Changed source lines and deletions not seen to run, by file

src/framework.ts: lines 240–241, 9253–9264, 9330, 9338–9339, 9353, 9359, 31919–31921, 32898, 32907 and 32917 ran in no profiled drive; lines 242–244, 9265–9266, 9303–9305, 31935–31937 and 32893–32897 map to nothing the host runs.

src/mcpl/convert-block.ts: lines 66, 75, 94, 111, 115 and 121 ran in no profiled drive; lines 12, 16–17, 19–20, 22, 25–26, 58, 106–107 and 135–136 map to nothing the host runs.

src/mcpl/reference-fetcher.ts: lines 247–249, 288, 381, 405, 412, 468–471, 525, 537–542 and 544–545 ran in no profiled drive; lines 250–251 and 543 map to nothing the host runs.

src/mcpl/references.ts: lines 129–130, 755–764, 771–783, 826, 828, 833, 837, 846, 855, 907–909, 1106–1108, 1114–1117, 1122–1123, 1141–1142, 1183, 1200–1201, 1216–1218, 1256, 1348, 1353–1354, 1375 and 1392 ran in no profiled drive; lines 13–19, 29–31, 59–60, 69–70, 514, 708–710, 750–754, 765–766, 769–770, 784–785, 842–845, 906, 910–911, 989–991, 994–1001, 1104–1105, 1109–1111, 1113, 1118–1121, 1124–1125, 1143–1144, 1168–1171, 1179–1180, 1247–1250, 1296–1301, 1306–1307, 1319–1320 and 1346–1347 map to nothing the host runs.

src/tool-result-history.ts: line 176 ran in no profiled drive; lines 29, 35, 175 and 564 map to nothing the host runs.

Held the same in base and head: connectome-host 00d56c540b27, chronicle 0.6.0, context-manager 0.27.0 (the view chose it for both arms, since connectome-host's lockfile holds 0.25.3, which doesn't meet ^0.27.0 from the base), membrane 0.8.1.