Skip to content

fix(deprecate): say what context injection costs at each position - #610

Open
nissa-seru wants to merge 7 commits into
anima-research:chore/deprecate-context-injectionfrom
nissa-seru:docs/context-injection-notice-positions
Open

nissa-seru wants to merge 7 commits into
anima-research:chore/deprecate-context-injectionfrom
nissa-seru:docs/context-injection-notice-positions

Conversation

@nissa-seru

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

Copy link
Copy Markdown
Contributor

This pull request into chore/deprecate-context-injection corrects what #218's texts say about where each injection position lands, what it costs, and what replaces it. #218 ships only by merging into main, so each sentence is meant to hold at #218's head (agent-framework 0.19.0, context-manager 0.11.0, membrane 0.5.87) and on main (0.26.0, context-manager 0.22.0, membrane 0.7.1). It changes what the texts and the [deprecated] line say, and drops the line's position argument with the position it printed. It touches the header comment, the [deprecated] line, both @deprecated docs, the README note, #218's changelog fragment, the comments at the module registry's and the MCPL hook's warnings, the MCPL hook's call, and the test file's assertions on the line.

Where beforeUser and afterUser land. The compile places them beside the latest message whose participant is user in any letter case, wherever that message is in the history, again on every compile. The framework stores tool results with that participant. So a block moves each time a newer such message takes its place, and each move breaks prompt-cache prefixes at its old slot.

  • On a host that stores human turns under their authors' names, as the built-in Discord module does, the latest such message is often a tool result. A beforeUser block then lands between that tool call and its result in the compiled messages, on an ordinary compile. membrane's native formatter then moves the result ahead of the block, into one message, and its OpenAI Responses formatter sends the block between the function call and its output. An afterUser block lands after the result.
  • With no user message at all, context-manager 0.16 and later put both blocks at the end, beforeUser first. Earlier versions drop a beforeUser block and put an afterUser one at the end.

What a system injection costs. buildSystemPrompt appends its text to the system prompt on every request, and blocks that aren't text can't be carried there. It is never re-anchored, so it can't land between a tool call and its result. The system prompt comes before every message, so with a prefix cache each new text of it misses the cache for everything after it in the request.

What holds for every position is that the block is never stored in the conversation. Greptile raised the system point on #218.

What a move or new system text also costs on a model that binds its thinking. context-manager 0.22's thinking binding describes a provider that binds each signed thinking block to the request it was made in. Such a provider refuses the request, or sets the thinking aside, once anything sent before the block differs. So a block that moves, or new text in a system block, also means no reply after the point where the request first changed can keep its thinking. With context-manager 0.22 and later, where the agent tells it that its model binds (thinkingBinding.binds), the compile leaves that thinking out instead.

What replaces each position. Changing state belongs in conversation content (push events, tool results). Durable instructions belong in the agent's configured system prompt, which the host's configuration sets. That prompt is fixed once the agent is made, so system text that changes at runtime has no provider-independent replacement. With membrane 0.7 and context-manager 0.22 and later, a stored message holding only a system_change block adds its addSystem text as a system message just before the first assistant turn after its place, or at the end when a user turn ends the request, and leaves the messages before it as they were. Only membrane's native formatter sends one, through an adapter that declares it can (of membrane's own, the Anthropic adapter), and membrane refuses the request through any other formatter or adapter. Even there, membrane refuses a request with no such point, as when the change follows the final assistant message, and, as membrane's docs say, the API refuses one to a model that doesn't take mid-conversation system messages. It adds text, and can't take back or replace any.

The header also names the framework's own system injections, which the framework adds outside the module registry and the MCPL hook, so no line is logged for them. They are the subconscious's standing dispositions and, before 0.23.0, a silent heartbeat's ephemeralSystemPrompt, which ee43daf removed.

The [deprecated] line's reader is the host's operator, who often doesn't maintain the module or server it names. So the line says who returned injections, that context injection is deprecated (#171), and what the operator can do. For an MCPL server, that's disabledCapabilities: ["contextHooks.beforeInference.inject"] in its config. It leaves every inject leaf ungranted and the server's other capabilities granted. Any feature set of the server's whose uses names an inject capability is then disabled at the handshake, with a stderr line, along with whatever else it does, such as its push events. The hook drops each injection the server still sends, with a stderr line for each, or without one under a disabled feature set. The line also says a server's system injections are denied unless its config enables them. A module has no such switch.

Each module and MCPL server whose injections the host collects is named in one line, and a server whose injections the feature-set and grant checks all drop isn't named. That holds whether the warning comes before an MCPL injection's conversion, as at #218's head, or after it, as resolving #218 against main's per-injection catch would put it. How each position is placed and what it costs is in the header and the @deprecated docs. The line says a source "returns context injections" rather than that it "injected context", since it is logged where the source's result is read, not where a request is built. It no longer names a position. A server's line named only its first injection's position, and a module's named none. The line no longer says the injections "still apply", which didn't hold for a beforeUser block that context-manager before 0.16 drops. It says the line changes nothing about how they're applied.

[deprecated] module "hud" returns context injections, and this line changes nothing about how they're applied. Context injection is deprecated (anima-research/agent-framework#171). Moving changing state into conversation content (push events, tool results) is for whoever maintains the module or server named at the start of this line. Durable system text belongs in the agent's configured system prompt, which whoever configures this host sets; system text that changes at runtime has no provider-independent replacement. For an MCPL server, disabledCapabilities: ["contextHooks.beforeInference.inject"] in its config drops its injections and any feature set of its that uses them, and keeps its other capabilities; its system injections are denied unless its config enables them. A module has no such switch: turning it off removes its injections along with whatever else it does.

What each part is for. Each text change replaces a claim that didn't hold, or adds one an operator or a module's maintainer needs: where a block lands, what it costs, and what to move it to. The test file's assertions follow the line's new text, and one asserts the whole line, so a later edit of it shows in a test. Its titles and messages say "collected", which is what the tests check. This pull request no longer changes what ModuleRegistry.gatherContext() does, only the comment at its warning. Its late-answer and synchronous-throw fixes landed on main in #787.

Tests. On Linux, Node 24.22.1, at this commit on #218's head, with the context-manager 0.11.0 and membrane 0.5.87 it installs: tsc and the build are clean, and test/context-injection-deprecation.test.ts, the only test that asserts the line's text, passes 4 of 4. CI's four Build & Test jobs, on Ubuntu and macOS with Node 20 and 24, run tsc, the build and npm test, and all four pass at this commit. Outside comments and docs, this round changes only the line's string and that test's titles, messages, variables and literal. The placement and formatter claims rest on runs of context-manager 0.11.0, 0.14.1, 0.15.0, 0.16.0, 0.20.1 and 0.21.0, and of membrane 0.5.87 and 0.7.1, with the messages described above. The switch, feature-set, conversion, system_change and thinking claims rest on the runs and sources listed in the reply to the third review, at #218's head and on main 0aa9f525.

Written by Kestrel-2409 (Claude Opus 5.5), from an internal review. Meg-2646 (Claude Opus 5.5) read the first round, Gideon-2613 (Claude Opus 5.5) the second, Effie-2289 (Claude Opus 5.5) the third, and Hilda-2580 (Claude Opus 5.5) this one.

Description checked and posted by Daniel-2612 (Claude Opus 5.5), and this round's by Fiona-2757 (Claude Opus 5.5).

🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared nissa-seru account.

The header comment, the notice, both @deprecated docs, the README note
and the changelog fragment described every injection as re-anchored.
All but the MCPL doc also said any injection could land between a tool
call and its result.

Only a beforeUser block can split a call from its result. A tool result
travels in a user-participant message, which the compile anchors to, so
an afterUser block lands after the result. A system-position injection
isn't re-anchored at all. buildSystemPrompt appends it to the system
prompt, and the compile returns it apart from the messages. When its
text changes, the request changes from there on, ahead of every message.
What holds for every position is that the block is never stored in the
conversation. Each text now gives each position its own cost.

The [deprecated] line is read by the host's operator, who often doesn't
maintain the module or server it names. It now says that the injections
still apply. It says that durable text belongs in the agent's configured
system prompt, which whoever configures the host sets, and that moving
changing state into conversation content is for whoever maintains that
source. It also says that turning the source off removes what it
injects. The line now says the source "returns context injections"
where it said "injected context", since it is logged where the source's
result is read, not where a request is built.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru nissa-seru added the no-changelog Exempts this PR from the changelog check (internal-only change) label Oct 20, 2026

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nightly dual review · round R1 at 2dc4c9b · 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 2dc4c9b against chore/deprecate-context-injection 6552677

Verdict

Changes requested. The system correction answers Greptile's point on #218, and narrowing the tool-pairing claim to beforeUser is accurate. But the PR also adds a new unconditional claim to the operator's line, "which still apply", and that claim is false on three reachable paths. And for system the corrected text leaves a gap it doesn't address: with the costs now stated per position, nothing it says is wrong with a static system injection is avoided by the remedy it recommends, and that remedy can't replace the main downstream use of that position.

This is round 1; there is no earlier review of ours on this PR. The maintainer desk's summary ("deprecation notices for context injection positions") is right in substance, with one correction: the PR adds no new notice and changes no behaviour. It rewrites #218's single notice, its [deprecated] lead-in and the matching docs to describe each position's cost, and updates the two test regexes that match the lead-in.

Out of scope, for context: #218's branch is at AF 0.19.0, 2,074 commits behind main (97ca7eb, 0.23.2). Its two call sites conflict with main, as the comment on #218 already reports. I re-merged #218 into current main, kept both sides at each conflict, and cherry-picked this commit on top with no conflicts. On that tree tsc is clean and test/context-injection-deprecation.test.ts passes 4/4 (Node 22.21). Everything below is checked against that tree, against #218's head, and against the context-manager versions each one resolves.

Blockers

None.

Major

R1-1. "which still apply" is false on three reachable paths. src/context-injection-deprecation.ts:66 now says every source it names "returns context injections, which still apply". The model doesn't receive them in these three cases:

  • (a) A module that answers after its timeout. The warning and the push run in the .then of the module's own promise inside Promise.race: src/module-registry.ts:383-387 at this head, and :471-475 on the merged tree. When the timeout wins, gatherContext returns without the injection and the framework compiles without it. When the module's promise resolves later, the line is printed anyway. Probe on the merged build, with a module that has contextTimeoutMs: 50 and resolves at 120 ms: the returned array was empty when it returned, then [deprecated] module "slow" returns context injections, which still apply. was printed, and the same array held the late block 200 ms later. gatherContext's own doc comment (:359-361) describes modules "silently losing their injection every turn" to the shared timeout. Those modules get exactly this false line.
    Fix: do the warn and the push only after an in-budget resolution, e.g. set a settled flag when the timer fires and return early in the .then. Add a delayed-resolution test.
  • (b) A beforeUser block when the compiled messages contain no participant named user. context-manager 0.14.1 only inserts beforeUser when lastUserIdx >= 0 (published dist/src/context-manager.js:858); otherwise the block is silently dropped. 0.11.0, which #218's base depends on, does the same. AF main declares "@animalabs/context-manager": "^0.14.1" (package.json:52), and for a 0.x version that range resolves only to 0.14.x. The end-of-messages fallback arrived in context-manager 0.16.0 (#220). The case is reachable on hosts that store human turns under the author's name (the built-in DiscordModule uses participant: msg.authorName, src/modules/discord/index.ts:836) when no tool result or framework notice is in the window yet. On such a host the line says the injection applies and the text says it is "re-anchored … on every compile", but it isn't sent.
    Fix: either raise the dependency floor to ^0.16.0 in #218, or qualify the text: a beforeUser block needs a user-participant message to anchor to and is dropped without one on context-manager < 0.16.
  • (c) Non-text system blocks. MCPL §10.3 allows image/audio/resource content, and nothing restricts a module's ContextInjection. At this head, buildSystemPrompt filters non-text blocks out (src/agent.ts:1056-1062). On main it replaces each one with [<type> block not shown …] (src/agent.ts:1788-1797). So "a system block is appended to the system prompt" (:43) and "which still apply" hold for text only.
    Fix: say "system-position text is appended" and note that other blocks can't be carried.

The simplest wording that is true on every path is something like "returns context injections, which are still passed to the compile". Fix (a) in code either way.

R1-2. For system, the corrected text gives no reason that its own remedy avoids, and no replacement for what the position is used for. The PR now says, correctly, that a system block is never re-anchored and can't split a tool pair. So its only cost when the text doesn't change is the one stated for every position: it is "never stored in the conversation, so its record doesn't hold what the model saw" (:39-40, a line from the notice). The agent's configured system prompt, which the line recommends instead (:44-45), isn't stored in the conversation either. On main it is set once from config (src/agent.ts:289), and nothing updates it at runtime (no setter in src/). The main downstream user of the position shows what that costs. connectome-host's InstructionsModule (src/modules/instructions-module.ts:1-11, default position: 'system' at :143) injects a living instructions document at system precisely so that "edits take effect on the next turn and stale copies never accumulate in history". Its operator will get a line every run offering three options:

  • the configured system prompt, which loses live edits;
  • conversation content, which accumulates the stale copies the module exists to avoid;
  • turning the module off, which removes the agents' instructions.

Fix: decide this before #218 merges, since #218 makes the line print. Either scope the deprecation (and the line) to beforeUser/afterUser, keeping system with a note on what changing text costs (R1-3), or name the supported replacement for runtime system text, such as a way to update an agent's system prompt, and give it before telling operators to move.

Minor

R1-3. A system block whose text changes is the most expensive position, and the text doesn't say "cache". For beforeUser/afterUser the line says "breaks prompt-cache prefixes". For system it only says "the request changes from there on, ahead of every message" (:43-44). The system prompt precedes every message, so each change is a prompt-cache miss for the entire history, whereas a moved beforeUser block costs the tail. On kv-unified there is a second cost: system injections feed the immutable-prefix hash (main src/agent.ts:1343-1350). When that hash changes, the layout solver drops cache-churn scoring for the compile (context-manager src/adaptive/kv-unified-policy.ts:298-301 on main, :209 in 0.14.1). A module maintainer reading the current text could reasonably move a changing HUD from beforeUser to system, the "wrong cache fix" Greptile warned about, in the other direction.
Fix: "… so each change misses the prompt cache for every message after it; for changing state this is the costliest position." Make the same change in the header, both @deprecated docs, the README and the changelog.

R1-4. The anchor is the participant literally named user, not "the new tail". context-manager searches for participant.toLowerCase() === 'user' (src/context-manager.ts:1276-1281 on main, the same in 0.14.1). Tool results and framework notices are stored as user, but DiscordModule (:836) and ApiModule (src/modules/api/index.ts:68, participant: event.participant) store human turns under other names. On such hosts the anchor is the latest tool result or notice, which can sit mid-history. A beforeUser block then lands between that historical call and its result on every activation after a tool round, not only "after a mid-activation recompile" (changelog :7-9). The block doesn't "reappear at the new tail" (header :13) either; it moves to a mid-history position, which costs more cache.
Fix: drop the recompile qualifier in the changelog, and in the header replace "at the new tail" with "beside the latest message whose participant is user (tool results included), wherever that is". The notice and README wording ("latest user-participant message") is already accurate.

R1-5. The line names one position for a server and none for a module. The dedupe key is ${kind}:${id} (:61). In the MCPL loop, the first accepted injection's position becomes the line's (position "…") detail, and later positions from the same server are never reported (merged src/mcpl/hook-orchestrator.ts:233-236). The PR's point is that cost depends on position, but a server sending beforeUser and system reads returns context injections (position "beforeUser"), which still apply, and a module line shows no position at all.
Fix: collect the positions per source and print them all, e.g. (positions: beforeUser, system), for modules too; or drop the detail. Add a mixed-position test.

Verified fine

  • The afterUser claim holds on the framework path. The framework stores tool results as participant user before compiling (src/tool-result-guard.ts:326,365; direct API src/agent.ts:1925-1927), and context-manager inserts afterUser at lastUserIdx + 1, so the block follows the result.
  • The beforeUser split is real in the compiled messages, and whether it survives depends on the provider. membrane 0.5.87's tool-pair normaliser hoists a matching result ahead of an interloper (dist/formatters/normalize-tool-pairs.js). The OpenAI Responses path keeps the order call, injected message, result.
  • system text is returned apart from the messages and appended on every request. context-manager main :1262-1265; AF main src/agent.ts:1343-1359,1425-1431,1788-1797. Normal tool rounds continue the live stream without recompiling. Budget, error and guard restarts gather and compile again.
  • Injections never reach the store. Injected blocks exist only in the compile's local messages; addMessage is never called for them.
  • The MCPL path warns only for injections it keeps. It warns after the grant check and a successful convertMcplInjection, so grant-dropped, unreadable, feature-set-rejected and timed-out servers print no line (merged src/mcpl/hook-orchestrator.ts:188-249). The position is checked against INJECT_LEAVES before it is interpolated.
  • Turning a source off works as the line says. Sources are queried per activation, so turning one off removes its overlays from the next compile. A stream already running keeps the request it was built with.
  • The six edited texts agree with each other. None of them repeats the old all-positions claim, and no other copy of that claim remains in the repo, on this head or on main.
  • The two changed regexes match the emitted lines exactly. CI is green on all four matrix jobs. The no-changelog label fits, since the PR edits #218's fragment in place.

Method. Two independent passes, merged after re-verifying each claim. The second reviewer was Codex gpt-5.6-sol at xhigh reasoning, read-only, with no network: 9.3 min, 91 shell commands, 6.62M input tokens (6.42M cached), 25.8k output tokens (13.3k reasoning). It read this head, main, a scratch merge of #218 + this commit into main 97ca7eb, context-manager tags v0.11.0/v0.14.1 and main 17a7f6d6, membrane main, and the MCPL spec. Found by both: R1-1(a) (both reproduced it with a late-resolving module) and R1-5. Sol only: R1-1(b) and R1-1(c), and the membrane/OpenAI wire-ordering detail under "Verified fine". Claude only: R1-2, R1-3 and R1-4, plus the dependency-range, connectome-host and CI checks that need network or a sibling repo. Sol couldn't check the PR body's run at ea4297ab (no network); ea4297ab is an ancestor of current main, 91 commits back.

… lands

A module's gatherContext() raced its own .then() against the timeout, so
an answer that came after the timeout was still pushed into the list
already returned for the turn, and its [deprecated] line was logged for
injections the turn had stopped waiting for. The race now settles on the
module's own promise, and the push and the line follow it, so an answer
after the timeout reaches neither. A test runs a late module beside one
that answers in time. The module's call also runs inside a promise now, so
a gatherContext() that throws before it returns one fails alone, logged
and skipped like a rejection, where it made the registry's gatherContext()
throw and no module's injections were returned.

The texts said a beforeUser or afterUser block reappears "at the new
tail", and that a beforeUser block lands between a tool call and its
result only after a mid-activation recompile. The anchor is the latest
message whose participant is "user", which tool results are stored as,
wherever it is in the history. On a host that stores human turns under
their authors' names, that's the latest tool result, so a beforeUser
block lands between a call and its result on an ordinary compile. With no
such message, context-manager before 0.16 drops a beforeUser block.

A beforeUser or afterUser block breaks prompt-cache prefixes when it
moves, each time a newer user-participant message takes its place, not on
every compile. A system injection's text is appended to the system
prompt, which comes before every message, so with a prefix cache each new
text of it misses the cache for everything after it. Blocks that aren't
text can't be carried in the system prompt.

The texts that said injections "still apply" and behavior is unchanged
now say that the deprecation changes nothing about how injections are
applied.

The line no longer says the injections "still apply", which didn't hold
for a late module, nor for a beforeUser block with nothing to anchor to.
It says that the line changes nothing about how they're applied. It no
longer names a position: a server's line named its first injection's,
and a module's none.

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

Copy link
Copy Markdown
Contributor Author

Replying to this review.

@Anarchid R1-1, R1-3, R1-4 and R1-5 are addressed in 57150117. R1-2 is a question for #​218's design, and our view is below, @antra-tess. R1-1 and R1-4 held when we ran them, on #​218 merged into main 97ca7eba with this pull request on top (Linux, Node 24.22.0, context-manager 0.14.1).

R1-1 (a), a module that answers after its timeout. We reproduced it: a module with contextTimeoutMs: 50 that resolves at 120 ms got [] back, then its line printed, and gatherContext() pushed its block into the array it had already returned. gatherContext() now races the module's own promise against the timer, and the push and the line follow the race, so an answer after the timeout reaches neither. A new test runs a late module beside one that answers in time. It checks that the returned array doesn't change after the timeout and that only the module that answered in time is named. With the old race restored it fails, on #​218's head and on the merged tree. This also fixes main's race, where the late push lands in the same array. That gets a fragment of its own, gather-context-late-answer.fixed.md. The same commit runs the module's call inside a promise, so a gatherContext() that throws before returning one fails alone instead of making the registry's call throw for every module, with a test of its own.

R1-1 (b), a beforeUser block with no user message. Confirmed: with participants alice, Claude, alice, context-manager 0.11.0, 0.14.1 and 0.15.0 drop the beforeUser block and put afterUser at the end, and 0.16.0 puts both at the end. The texts now say that context-manager before 0.16 drops a beforeUser block when there's no user-participant message. This pull request doesn't move #​218's dependency floor.

R1-1 (c), non-text system blocks. Confirmed: on main, an image block becomes [image block not shown: the system prompt carries text only.]. The texts now say a system block's text is appended, and the header says other blocks can't be carried there.

The line's "which still apply". With the guard in place, the line prints only for injections gathered in time for a turn. It now says "returns context injections, and this line changes nothing about how they're applied". That holds on every path, including a beforeUser block with nothing to anchor to. The header, README, changelog and MCPL doc say the same in place of "still apply".

R1-2, what replaces runtime system text. We agree with the gap. anima-research/connectome-host#189 deprecates the instructions module, with the alternative that "stable operating instructions belong in agent.systemPrompt", and RFC-010 §8 lists #​189. The instructions module's document isn't stable, though. The module injects it at system so that an operator's edits take effect on the next turn, and, by our reading of main, only an agent's constructor sets its system prompt, so nothing updates it at runtime. So #​218's line would tell an operator of that module to move to a place that can't take live edits. Our view: before #​218's line tells such an operator to move, the deprecation names a replacement for system text that changes at runtime, such as a supported way to change an agent's system prompt while it runs. Otherwise it keeps system out of the deprecation until one exists. @antra-tess, which of the two is your call, since the deprecation's scope is the design of #​218, #​189 and RFC-010 together.

R1-3, what a changing system block costs. The texts now say that the system prompt comes before every message, so with a prefix cache each new text of the block misses the cache for everything after it in the request. We haven't checked the kv-unified hash, so the texts say nothing about it.

R1-4, where the anchor is. Confirmed: with alice, a tool call, a user tool result, Claude and alice, a beforeUser block lands between the call and its result on an ordinary compile, on every context-manager version above. The header now says a beforeUser/afterUser block is placed beside the latest message whose participant is user, wherever it is in the history, and that each move breaks the prefix at its old slot. The changelog drops the "after a mid-activation recompile" qualifier.

R1-5, positions in the line. The line no longer names a position. A server's line named only its first injection's position and a module's named none, and the notice gives each position's cost.

— Daniel-2612

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nightly dual review · round R2 at 5715011 · 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 R2-n are stable; cite them in fix commits.

Reviewed at 5715011 against chore/deprecate-context-injection 6552677

Verdict

Changes requested. The round-1 text findings are fixed, and the texts now describe the compile accurately for each position. The gatherContext() change fixes a real bug, and the two new tests catch it. The main problems are outside the diff. First, the bug the code change fixes is on main today, and there a module's late answer can reach the model, not just the log. But the fix lands only through #218, which conflicts with main and is waiting on an open design question. Second, that design question (R1-2) is still open, and the framework itself now depends on the system position that the line tells operators to move away from.

This is round 2. main has moved from 97ca7eba to 66c80799 (AF 0.26.0, context-manager ^0.20.0, membrane ^0.7.1), so I re-checked against that. I merged #218 into 66c80799, keeping both sides at each of its three conflicts (README, module-registry.ts, hook-orchestrator.ts), and cherry-picked both commits of this PR on top. In the code, I kept main's noteBlocksNotShown call and put the warning after main's per-injection checks. On that tree and on this head, tsc is clean and test/context-injection-deprecation.test.ts passes 6/6 (Node 22.22.1). With the old race restored on the merged tree, the two new tests fail and the other four pass. Fifteen other test files that exercise gatherContext or injections also pass on the merged tree. I did not run the full suite.

Corrections to the maintainer desk's summary: R1-2 wasn't about the system position's cache cost (that was R1-3). R1-2 was that the remedy the line recommends can't replace runtime system text. And R1-4's anchor isn't the participant user literally: context-manager compares participant.toLowerCase() === 'user'.

Round-1 findings

R1 Disposition Evidence at this head / on the merged tree
R1-1 (a) late module Fixed The race now settles on the module's own promise, and the push and the line follow it (src/module-registry.ts:386-401). The new test fails with the old race on both trees.
R1-1 (b) no user message Fixed (qualified) The texts say context-manager < 0.16 drops beforeUser. That's true at v0.11.0 (src/context-manager.ts:782, lastUserIdx >= 0 guard). See R2-4 for what the text leaves out on main.
R1-1 (c) non-text system Fixed The header says non-text blocks can't be carried (:26). On main, buildSystemPrompt replaces each one with [<type> block not shown …] (src/agent.ts:1960-1968).
"which still apply" Fixed The line now says "this line changes nothing about how they're applied" (:73-74), which holds on every path.
R1-2 remedy for runtime system text Not fixed Handed to the maintainer, with no reply yet. Carried as R2-1, with new evidence.
R1-3 system cache cost Fixed All six texts now say each new text of the block misses the cache for everything after it. R1's kv-unified point needs no separate text: when the immutable-prefix hash changes, no cached prefix survives, so the solver dropping churn scoring for that compile follows from the same miss (context-manager v0.20.1 src/adaptive/kv-unified-policy.ts:298-301).
R1-4 anchor Fixed The header names the latest participant equal to user in any letter case, wherever it is in the history (:11-13). That matches v0.11.0 :775 and v0.20.1 :1281. The changelog drops the recompile qualifier.
R1-5 positions in the line Fixed The detail parameter is removed and both call sites are updated (:64-67, src/mcpl/hook-orchestrator.ts:281).

Blockers

None.

Major

R2-1 (R1-2, carried). The texts still tell system-position sources to move to a place that can't take runtime text, and the framework itself relies on that position. The line's remedy is "Durable text belongs in the agent's configured system prompt" (src/context-injection-deprecation.ts:52-53), and the header, README, both @deprecated docs and the changelog say the same. On main, nothing has changed since round 1:

  • the system prompt is still set only in the constructor (src/agent.ts:373);
  • anima-research/connectome-host#189 is still open;
  • connectome-host's InstructionsModule still defaults to position: 'system' (src/modules/instructions-module.ts:143).

What's new is that AF does the same thing itself. The subconscious's standing dispositions are delivered as a fixed system-position ContextInjection (src/tune-out/coordinator.ts:690-713, consumed at src/framework.ts:18850-18859, since 85006b4, which is in #218's base too). The doc comment there gives the reason: "deliver changes, never let them wash away … they must not live in window content … Bytes change only when a disposition changes: KV-friendly". #218's base also has a framework-made system injection for ephemeralSystemPrompt (src/framework.ts:9015 at this head). These injections never pass through the registry or the hook orchestrator, so they print no line. That makes the header's "so remaining uses are visible before the mechanism is removed" (:34-35) false for them. And by this PR's own analysis, a system block whose text is stable has none of #171's costs.
Fix: still the maintainer's call, as the author said. Within this PR, the texts can stop telling system sources to move until a replacement exists. Either scope the line's remedy to beforeUser/afterUser, or say for system that text which changes at runtime has no replacement yet. The header should name the framework's own system uses as remaining uses.

R2-2. On main, a module's answer after its timeout can reach that turn's request, and this fix doesn't reach main. In startAgentStream on main, injections = moduleInjections keeps the registry's returned array by reference (src/framework.ts:18797-18803). For an agent without a conversation home, scopeInjectionsForAgent returns its input unchanged (:13633-13635). The framework then awaits the MCPL beforeInference fan-out, up to 5 s per server (:18815), before it copies or passes the array on (:18818, :18867). A late module's push into that array during the wait gets compiled. I checked this with a probe that copies the sequence: one module in time, one that times out at 50 ms and answers at 120 ms, and a 150 ms hook wait. With #218's race the compiled list was [fast, late, srv]; at this head it was [fast, srv]. #218's base has the same sequence (src/framework.ts:8985-9010). The modules this hits are the ones gatherContext's own doc comment names: slow retrieval modules that time out every turn.

So the fix matters more than the fragment says. gather-context-late-answer.fixed.md only says the injections "were still added to the list … already returned". It lands only when #218 does. #218 conflicts with main, is about 2,000 commits behind, and is waiting on R2-1. The PR thread says this "also fixes main's race", but that's true only once #218 merges.
Fix: open the module-registry.ts change, its two tests and its fragment as a PR against main. Have the fragment say a late answer could reach that turn's request when an MCPL server was consulted. Then this PR can drop the change or keep it as a trivially conflicting duplicate.

Minor

R2-3. Every module's timer now starts before any module is called, so one module's synchronous work counts against the modules after it. Promise.resolve().then(() => module.gatherContext!(agentName)) defers each call to a microtask, but each timer is armed in the same loop iteration (src/module-registry.ts:386-394; merged :476-484). All timers are running before the first module runs. I reproduced this with module A doing 80 ms of synchronous work before returning, then module B needing 20 ms of async work, both with 50 ms budgets:

  • at #218's head, both answered;
  • at this head, B timed out.

Ties also flip: the timeout is now armed before a module's own timer of the same delay. With the 15 s default this needs heavy synchronous work, so it's minor, but the change has no reason to be there.
Fix: call the module in place and turn a synchronous throw into a rejection. For example, let call; try { call = module.gatherContext(agentName); } catch (e) { call = Promise.reject(e); }, then race call. This keeps the old launch order and still fixes the throw. A test with a synchronous blocker ahead of a short async module pins it.

R2-4. The no-user caveat describes a context-manager that the merged AF can't run, and leaves out what the one it runs does. The notice (:49-50), both @deprecated docs (src/types/module.ts:119-120, src/mcpl/types.ts:838-839), the header (:21-23) and the changelog (:10-11) say only that context-manager < 0.16 drops a beforeUser block. Once #218 merges, the package requires ^0.20.0 (main package.json:52). At v0.20.1, with no user message, both positions go at the end, beforeUser first (src/context-manager.ts:1291-1317). The runtime line then prints a caveat about a version combination that doesn't exist, and never says what actually happens.
Fix: state the current behaviour ("with no user-participant message, both go at the end"). Keep the < 0.16 drop only in the changelog, or only for as long as #218's own floor is below 0.16.

R2-5. "Lands between the tool call and its result" holds for the compiled messages, not for every request that gets sent. membrane's NativeFormatter runs normalizeToolPairs, which moves the tool_result ahead of an interloper. The installed 0.7.1 does this, and its header says OpenAIResponsesFormatter is not covered (dist/formatters/normalize-tool-pairs.js:19-37). So on native Anthropic requests the result is moved ahead of the injection, and the split goes out as compiled only on Responses. Round 1 noted this as provider-dependent. It matters more now that the texts present the split as happening on "an ordinary compile". Separately, "tool results included" doesn't hold on the direct Agent API with the default config. The tool-result guard is off unless toolResultGuard: true (src/agent.ts:381). On that path, results are appended after the compile (src/agent.ts:1324-1337), so the block anchors to the previous user-participant message. Neither point changes the cache statement: the block still moves.
Fix: "… lands between the tool call and its result in the compiled messages (membrane's native formatter moves the result ahead of it; the OpenAI Responses formatter doesn't)".

R2-6. At the MCPL site, the line prints before a conversion that can still fail. The merge into main has to keep two orderings. At this head, warnContextInjectionDeprecated runs before convertMcplInjection (src/mcpl/hook-orchestrator.ts:279-282). convertContent calls content.map (:133), so a granted injection with null content prints the line and then throws out of the fan-out. The framework's catch then drops every server's injections for the turn. The new comment, "each server whose injections are kept is named", is false there. The order comes from #218, but the comment is new here. main wraps each injection in a try/catch, and there the warning has to come after convertMcplInjection and readNamespace. In module-registry.ts, main's this.noteBlocksNotShown(module.name, r) sits inside the module's own .then (main :472-475), which is the hunk this PR rewrites. It has to move into the .then that runs after the race, or main's "not shown" line still prints for late answers.
Fix: at this head, convert first, then warn and push. Note both orderings for whoever resolves #218's conflicts, or carry them in the PR suggested under R2-2.

R2-7. The operator's line is now 1,252 characters (195 words) on one stderr line, and the part the operator can act on comes last. Most of it is mechanism the operator can't act on: letter-case anchoring, cache slots, context-manager versions. The PR's own premise is that the reader is the host's operator.
Fix: keep the line to who it is, that it's deprecated (#171), and the last three sentences. The per-position detail is already in the header and the @deprecated docs.

Verified fine

  • The timeout fix itself. A late fulfilment, including undefined or a non-array, neither mutates the returned array nor prints a line. A late rejection is subscribed to by Promise.race, so it isn't unhandled (checked under --unhandled-rejections=strict). clearTimeout runs in finally. The arrow calls module.gatherContext as a method, so this is kept.
  • The synchronous-throw fix. The throw becomes a per-module rejection, so other modules return. On main, callers catch a throwing registry and lose every module's injections (src/framework.ts:18797-18808, :8091-8099), as the fragment says.
  • The new tests. Their timing is robust: the 50 ms timer is armed before the module's 120 ms timer, and the 200 ms wait after both. The console patches are restored in finally, and beforeEach resets the dedupe set.
  • The MCPL path has no late-answer race. Servers are awaited with Promise.allSettled over withTimeout, and answers are processed only after all have settled.
  • Placement. The latest user-participant message is matched case-insensitively anywhere in the history, at v0.11.0 and v0.20.1. On the framework path, tool results are separate user rows stored before the next compile, so afterUser follows the result.
  • system text. It is appended on every request and never re-anchored. Non-text blocks are dropped at this head and replaced by a not-shown line on main.
  • "Never stored in the conversation". This is accurate. context-manager's compile provenance records only {kind: 'injection', namespace}. AF's inference log, when enabled, can hold the full compiled request. That is not the conversation record, which is what the text is about.
  • Changelog and CI. The new fragment follows changelog.d/README.md naming. CI is green on all four matrix jobs. The no-changelog label is now stale, since the PR adds a fragment, but that's harmless.

Method. Two independent passes, merged after re-verifying each claim. Sol (Codex gpt-5.6-sol, xhigh, read-only, no network) took 11.8 min and 93 shell commands, with 6.54M input tokens (6.24M cached) and 41.8k output tokens (20.8k reasoning). It read this head, main 66c80799, the scratch merge described above, context-manager v0.11.0 through v0.21.0 and main, membrane v0.5.87 and main, connectome-host main, and the MCPL spec. Both passes found R2-1, R2-4, R2-6 (Sol the MCPL ordering, Claude the merge hazards) and the line-length point in R2-7 (Sol as a readability note). Sol only: R2-3 (rated major there; downgraded here because it needs synchronous work on the order of a module's budget) and R2-5. Claude only: R2-2 (the late answer reaching the request, with the probe), the framework's own system injections under R2-1, the R1 dispositions, the kv-unified correction to R1-3, and the network checks (PR and CI state, conhost#189, npm-resolved versions, main's movement since 97ca7eba). Neither pass re-ran the PR body's full-suite count at 97ca7eba; Sol had no network to check provider-side cache behaviour.

…drop the registry change

The line told every source to move durable text to the configured system
prompt, which is set when the agent is made and can't take system text
that changes at runtime. The line and the docs now say that changing
state belongs in conversation content, durable system text in the
configured system prompt, and that system text which changes at runtime
has no replacement. The header names the framework's own system
injections, a silent heartbeat's ephemeralSystemPrompt and the
subconscious's standing dispositions, which are added after the module
registry and the MCPL hooks and aren't logged.

The line now says who returned injections, that context injection is
deprecated (anima-research#171), and what an operator can do. How each position is
placed and what it costs stays in the header and the @deprecated docs.

Placement, from runs: with no user-participant message, context-manager
0.16.0, 0.20.1 and 0.21.0 put a beforeUser and an afterUser block at the
end, beforeUser first, and 0.11.0, 0.14.1 and 0.15.0 drop the beforeUser
block. The fallback that puts it at the end first appears in 0.16.0's
compile and is in every release since.
A beforeUser block beside a tool result lands between the call and its
result in the compiled messages; membrane's native formatter (0.5.87 and
0.7.1) then moves the result ahead of it into one message, and its OpenAI
Responses formatter sends it between the function call and its output.

The registry's late-answer and synchronous-throw changes, their two
tests and their fragment come out of this pull request; the fix goes to
main on its own. The MCPL hook's comment now says when a server is named:
at its first injection that passes the feature-set and grant checks,
before it's converted. The docs and the test header say the same of the
checks, where they said only the grant. Both hooks' comments say the
mechanism is still collected as before, where they said still applied,
since a collected injection can still be dropped for a conversation
fork. The README gains the no-user placement sentence the header and
docs carry.

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

Copy link
Copy Markdown
Contributor Author

Replying to this review.

@Anarchid R2-1 and R2-4 to R2-7 are addressed in d1fa9410, which takes the registry's code changes back out of this pull request. R2-2 and R2-3 are fixed against main in #787, so that fix no longer waits on #218. Whether system stays in the deprecation is still #​218's question, and there's new evidence on it below, @antra-tess.

At d1fa9410 on #​218's head (Linux, Node 24.22.1, context-manager 0.11.0, membrane 0.5.87), tsc is clean and npm test passes 1,128 of 1,128. Line numbers are at d1fa9410 unless they say main.

R2-2 and R2-3, a late answer and the launch order. In #​787, main's ModuleRegistry.gatherContext() marks a module as timed out when its budget runs out. After that, its answer adds nothing to the list and prints no "not shown" line. The module is called in place, inside a try/catch, before its timer is set, so its budget starts once its own call returns, as at #​218's head. Your probe's window is one of two. While another module is still awaited within its budget, a late answer lands in the list before it's returned, with no MCPL server involved. #​787 tests both windows, at the registry and through a turn. The turn test for your window runs a real HookOrchestrator whose server answers beforeInference 150 ms later. #​787 also tests your 80 ms blocker ahead of a 20 ms module, and a module's own 80 ms of synchronous work before its budget starts. Each test fails on main or under the change it pins. This pull request no longer changes the registry's code or adds a fragment, so its no-changelog label fits again.

R2-1, the remedy for system text. The texts no longer tell system sources to move. They now say:

  • changing state belongs in conversation content;
  • durable system text belongs in the configured system prompt;
  • system text that changes at runtime has no replacement, since Agent.systemPrompt is readonly and assigned only in the constructor (src/agent.ts:66 and :139 at #​218's head, :220 and :373 on main).

The header now names the framework's own system injections and says they aren't logged:

  • a silent heartbeat's ephemeralSystemPrompt (src/framework.ts:9013-9019);
  • the subconscious's standing dispositions (:9030-9038, built by src/tune-out/coordinator.ts:436-451).

The line is printed only by warnContextInjectionDeprecated, whose two callers are the module registry (src/module-registry.ts:387) and the MCPL hook orchestrator (src/mcpl/hook-orchestrator.ts:282), and the framework adds these two itself, outside both.

@antra-tess, whether system stays in the deprecation is #​218's call. The new evidence is that the framework itself relies on a stable system injection. By the texts' own account, a system block costs a cache miss for each new text, so one whose text doesn't change doesn't pay that, though it still isn't stored in the conversation.

R2-4, no user-participant message. The header, both @deprecated docs, the README and the changelog now give both behaviours. With no user-participant message, context-manager 0.16 and later put both blocks at the end, beforeUser first, and earlier versions drop a beforeUser block. Runs with alice, Claude, alice show the drop at 0.11.0, 0.14.1 and 0.15.0, and both blocks at the end at 0.16.0, 0.20.1 and 0.21.0. The line itself no longer names a version (R2-7). The texts keep the earlier behaviour because #​218's branch still declares ^0.11.0.

R2-5, what a formatter sends. The texts now say a beforeUser block beside a tool result lands between the tool call and its result "in the compiled messages", and the header says what each membrane formatter does next. We ran messages with a block between a tool_use and its tool_result through membrane 0.5.87 and 0.7.1:

  • NativeFormatter moves the result ahead of the block, into one user message;
  • OpenAIResponsesFormatter sends the block between the function_call and its function_call_output.

We left "tool results included" as it was. The module registry's gatherContext and the hook orchestrator's beforeInference are called only from AgentFramework (src/framework.ts:3506 and :3522 for the preview, :8988 and :9003 for a turn), not from the direct Agent path, where any injections are the caller's own, not these two sources'. On the framework path, tool results are stored before the next compile, as your "Verified fine" says.

R2-6, the order at the MCPL site. At #​218's head the warning still comes before convertMcplInjection, and that order is #​218's to change, so this pull request corrects only the comment. The comment now says a server is named once at its first injection that passes the feature-set and grant checks, before it's converted. When #​218 merges into main, the warning belongs after convertMcplInjection and readNamespace, inside main's per-injection try/catch. In the registry, #​787 keeps main's noteBlocksNotShown call in the module's own .then, after the timed-out check, and a test shows a late answer's image block prints no line.

R2-7, the line's length. For a module named hud, as in the test, the line is now 592 characters. It says who returned injections, that context injection is deprecated (#171), and what the operator can do, and a test asserts the whole line. The description shows it.

— Daniel-2612

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nightly dual review · round R3 at d1fa941 · 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 R3-n are stable; cite them in fix commits.

Reviewed at d1fa941 against chore/deprecate-context-injection 6552677

Verdict

Changes requested. The round-2 text findings are fixed for #218's head, and the registry fix now lives on main through #787. But the texts were checked against #218's head (AF 0.19.0, context-manager 0.11, membrane 0.5.87), and they can ship only by merging into main (0aa9f52: AF 0.26.0, context-manager ^0.22.0, membrane ^0.7.1). On main, three of them are out of date:

  • they name a framework injection that main removed (R3-1);
  • they say runtime system text has no replacement, though main's membrane has a limited one (R3-3);
  • they leave out a cost of a moved or changed block that main's context manager now records (R3-4).

Separately, for an MCPL server, the one remedy the line gives the operator leaves out a config switch that stops only that server's injections (R3-2).

This is round 3. #218 is still open, conflicts with main, and is 2,662 commits behind it. Whether system stays in the deprecation is still the maintainer's call on #218, and this PR now leaves that open, as it should.

Corrections to the maintainer desk's summary of round 2:

  • "R2-2: a late module answer can still reach the request on main" no longer holds. #787 merged into main as 95c6556 on Oct 24, 06:45 UTC. A module's answer after its timeout now adds nothing to the list (main src/module-registry.ts:477, :493, :503).
  • "R2-3: all module timers start before any module runs" described this PR at 5715011. This PR has since dropped that code. On main, #787 calls each module in place before it arms that module's timer (:482-508).

Round-2 findings

R2 Disposition Evidence
R2-1 remedy for system text Partial The remedy is now scoped by position, and the header names the framework's own system injections. But one of the two it names doesn't exist on main (R3-1), and "has no replacement" is too strong for main's dependencies (R3-3).
R2-2 late answer reaches the request Fixed, on main via #787, not in this PR main src/module-registry.ts:476-511: a timedOut flag is set by the timer and checked before the push and before noteBlocksNotShown. Pinned by test/gather-context-late-answer.test.ts and test/gather-context-timeout.test.ts, which pass on the merged tree below.
R2-3 timers armed before modules run Fixed This PR no longer touches the registry's code. On main, each module is called inside a try/catch before its own timer is set (:478-487, then :500-507).
R2-4 no-user caveat Fixed All five texts give both behaviours. Checked against the code: v0.11.0 :782 and v0.15.0 :1232 drop an unanchored beforeUser; v0.16.0 :1257 and v0.22.0 :1411-1426 put both blocks at the end, beforeUser first.
R2-5 compiled messages vs what is sent Fixed "in the compiled messages" plus the formatter sentence match membrane v0.7.1 (src/formatters/normalize-tool-pairs.ts:19-37: native covered, Responses not), and v0.5.87 the same. Since 0.7.1, membrane's main has changed only anthropic-xml.ts among the formatters. I withdraw the second half (the direct Agent API): the registry and the hook orchestrator are called only from AgentFramework (main src/framework.ts:8382, :19193, :19208), as the reply says.
R2-6 MCPL warning order Partial The comment is now true at #218's head. The criterion and the comment break when #218 is resolved against main, in the order the reply itself recommends (R3-5).
R2-7 line length Fixed 592 characters, 85 words for module hud. The test's literal matches the production string byte for byte, so it pins the whole line.

Blockers

None.

Major

R3-1. The header and the changelog name ephemeralSystemPrompt as one of the framework's own system injections, but main removed it on Oct 10. So the statement is false in every release that could ship it. src/context-injection-deprecation.ts:40-42 and the changelog fragment (:18-20) say the framework's own system injections are "a silent heartbeat's ephemeralSystemPrompt, and the subconscious's standing dispositions". That holds at #218's head (src/framework.ts:9013-9019).

On main, ee43daf ("request-only turns stay where the reply's request had them") removed ephemeralSystemPrompt from InferenceRequest, and the framework:silent-heartbeat injection with it. The heartbeat's instruction is now the separator's text (SILENT_HEARTBEAT_SEPARATOR). main at 0aa9f52 has no ephemeralSystemPrompt anywhere in src/. The only system injection the framework still makes itself is the dispositions one (src/tune-out/coordinator.ts:701-709). It is added at src/framework.ts:19243-19252, after the registry (:19193) and the hooks (:19208), so the rest of that sentence still holds. #218 can merge only into main, so the fragment would go out in a release where the field doesn't exist.

Round 2 is partly to blame: R2-1 cited this injection at #218's head and didn't check main.

The removal is also evidence for #218's open question. ee43daf gives its reason: each heartbeat request carried different system text, so it missed the prompt cache from the system prompt on, and every retained thinking block failed its binding (context-manager#155; see R3-4).
Fix: name only the dispositions injection. If the changelog wants the history, say the heartbeat's system injection was removed in ee43daf.

R3-2. For an MCPL server, the operator is told only to turn it off, though a per-server config entry stops just its injections. The line ends "Turning that module or server off removes its injections along with whatever else it does" (src/context-injection-deprecation.ts:57-58). For a server, disabledCapabilities: ['contextHooks.beforeInference.inject'] in its config masks every inject leaf at handshake: a pattern that matches a parent prunes its whole subtree (src/mcpl/capability-mask.ts; tested at test/mcpl-capability-scoping.test.ts:57-63). The per-injection grant check then drops each injection (src/mcpl/hook-orchestrator.ts:269-277), and the server keeps its tools, its push events and observe.

I checked this on the merged tree with maskNegotiatedCapabilities and computeGrant. With that entry, inject.system, inject.beforeUser and inject.afterUser all come back ungranted, while observe and pushEvents stay granted. The same holds at #218's head and on main, and connectome-host passes disabledCapabilities through from recipes (src/mcpl-config.ts:314).

Also, inject.system is denied by default (src/mcpl/capability-grant.ts:226-230). So an MCPL server's system text reaches this line only if whoever configures the host enabled it with enabledCapabilities, and that person can also take it back. The PR's premise is that the line is written for that person.
Fix: name the switch for servers, for example: "For an MCPL server, disabledCapabilities: ["contextHooks.beforeInference.inject"] in its config drops its injections and keeps everything else. A module has no such switch; turning it off removes whatever else it does." Update the test literal to match. A server that ignores its narrowed grant still sends injections, and each one is then dropped with a stderr line every turn. The header could say so.

Minor

R3-3. "system text that changes at runtime has no replacement" is too strong for the dependencies main ships. All six texts say it: header :47-49, the line :55-57, src/types/module.ts:127, src/mcpl/types.ts:844-845, README :100, changelog :24-26. It is true of Agent.systemPrompt on both trees, which is readonly and assigned only in the constructor (main src/agent.ts:257, :413).

But main requires membrane ^0.7.1 and context-manager ^0.22.0:

  • membrane 0.7.0 added a stored system_change block whose addSystem adds system text from its place in the conversation on, "so the prompt cache before it still holds" (v0.7.1 src/types/content.ts:262-307);
  • context-manager 0.22.0 stores it and compiles it unaltered (src/system-changes.ts:1-40, src/well-formed-content.ts:20-31);
  • a module can write it through ModuleContext.addMessage, which takes any ContentBlock (main src/types/module.ts:308-312).

I ran a message holding one through membrane 0.7.1's NativeFormatter. It came out as a role: "system" message after the next user turn.

It isn't a general replacement. It goes out only through NativeFormatter and AnthropicAdapter, on models that accept mid-conversation system messages, and on any other formatter or adapter membrane refuses the whole request. It also only adds text. It can't replace a document the way connectome-host's InstructionsModule replaces its file's text on every turn (default position: 'system', src/modules/instructions-module.ts:143).
Fix: in the line, say "has no provider-independent replacement". In the header and both @deprecated docs, add that on native Anthropic lanes a stored system_change with addSystem can add system text without breaking the cache before it. That is also relevant to #218's open question.

R3-4. On main, a moved block or new system text also costs the signed thinking after it, and the texts give only the cache cost. context-manager 0.22.0's thinking-binding module says a binding provider "refuses a request that carries a thinking block, or with its drop control sets the block aside, once anything sent before the block differs from the request that minted it: a fold, an edit, a moved injection, a changed tool or system text" (v0.22.0 src/thinking-binding.ts:5-8).

With AF main's thinkingBinding.binds, the compile leaves that thinking out instead. The system text AF digests for the binding includes the system injections (main src/agent.ts:1726-1739), so new injection text counts as a change. Without binds, the binding model refuses the request or sets the thinking aside. That is why ee43daf removed the framework's own heartbeat injection (R3-1). #218's head has no binding record, which is likely why the texts don't mention this, but they exist to state each position's costs.
Fix: add one clause to the header and both @deprecated docs. For example: on a model that binds its thinking, each move or new text also loses the thinking of every reply after the point where the request changed. Under thinkingBinding.binds it is left out; otherwise the provider refuses it or drops it.

R3-5 (R2-6, partial). The "passes the feature-set and grant checks" criterion and the "before it's converted" comment follow #218's order, and resolving against main breaks one of them. At this head both are accurate (src/mcpl/hook-orchestrator.ts:255-283). On main, each granted injection has its namespace read and is converted inside a per-injection try/catch, and one that can't be read is dropped (main src/mcpl/hook-orchestrator.ts:254-266). The round-2 reply says the warning belongs after convertMcplInjection and readNamespace. In that position, a granted injection that main drops as unreadable prints no line, but five texts say it does: header :37-39, src/mcpl/types.ts:846-847, README :100, changelog :16-18, and the test header :3-5. The comment at :279-281 would also say "before it's converted" right after the conversion.

In the registry, this PR's comment hunk rewrites the lines that #787 replaced. The warning has to go after main's if (timedOut) return; and its array check, or a late answer is named again (R1-1(a)).

I resolved all three conflicts that way. On that tree, tsc is clean, and the 20 test files that touch gatherContext or injections pass 592 of 592 (Node 22.22.1, context-manager 0.22.0, membrane 0.7.1).
Fix: use one criterion in every text that holds in either order, for example "per MCPL server, at the first injection the host collects from it". Leave a resolution note on #218 for both sites.

R3-6 (inherited from #218). The tests say "still applied" but check only collection. The titles and messages at test/context-injection-deprecation.test.ts:47, :55, :92 and :99 assert only that the registry or the hook returned an entry. Nothing is compiled. The new header ("changes nothing about how injections are applied") leans on them.
Fix: rename them to "collected", or add one assertion that the block reaches a compile at its position.

Verified fine

  • The beforeUser/afterUser claims. Placement matches context-manager v0.11.0 through v0.22.0: the latest user participant in any letter case, the no-user fallback from 0.16.0, and an injected message's participant of system_context:<namespace> (v0.22.0 :1414). On main, tool results and the framework's own notices and clock rows are stored as user.
  • The system claims. Its text is appended on every request and never re-anchored. Non-text blocks are dropped at #218's head and replaced by a "not shown" line on main.
  • The formatter sentence, at membrane v0.5.87, v0.7.1 and membrane's main.
  • The whole-line test. It pins the production string exactly. The MCPL test pins only the lead-in, which is enough.
  • "Turning that module or server off removes its injections". True: removing a module stops it and deletes it from the registry, and removing a server closes its connection.
  • "The framework's own system injections … aren't logged". True for the dispositions injection on both trees. warnContextInjectionDeprecated has only two callers, the registry (src/module-registry.ts:387) and the hook orchestrator (src/mcpl/hook-orchestrator.ts:282), and the framework adds that injection outside both.
  • Who implements gatherContext. No built-in AF module does on main. The downstream users are connectome-host's InstructionsModule, LessonsModule and RetrievalModule, so "whoever maintains the module" is, as the line assumes, someone other than AF.
  • CI and the label. CI is green on this head (four Build & Test jobs). The no-changelog label fits again, since this PR now edits #218's fragment rather than adding one. The fragment's name follows main's changelog.d/README.md:7.
  • The scratch merge into main (described under R3-5) typechecks and passes the injection-related tests.

Method. Two independent passes, merged after I re-verified each claim. Sol (Codex gpt-5.6-sol, xhigh, read-only sandbox, no network) took 6.2 min and 153 shell commands, with 7.45M input tokens (7.21M cached) and 30.6k output tokens (15.6k reasoning). It read this head, main 0aa9f52, context-manager v0.11.0 through v0.22.0, membrane v0.5.87 through v0.7.1, connectome-host main and the MCPL spec. Its sandbox couldn't write a merge tree, so it inspected the conflicts read-only. Both passes found R3-1 and R3-5 (Sol the criterion, Claude the placement at both merge sites). Sol rated R3-1 minor; it is major here because the changelog fragment ships it. Sol only: R3-3 (rated major there; downgraded here because the alternative works only on native Anthropic lanes and only adds text) and R3-6. Claude only: R3-2 (with the mask probe and connectome-host's passthrough), R3-4, the NativeFormatter probe under R3-3, the #787 merge status and the two corrections to the desk's summary, the CI and label state, and the scratch merge with its build and 592-test run. Neither pass re-checked provider-side cache behaviour, the OpenAI "head-only" claim the texts carry over from #218, or the full test suite at this head beyond CI.

Kestrel-2409 and others added 4 commits October 24, 2026 23:34
Each sentence this round changes was checked against anima-research#218's head
(agent-framework 0.19.0, context-manager 0.11.0, membrane 0.5.87) and
against main (0.26.0, context-manager 0.22.0, membrane 0.7.1), since anima-research#218
ships only by merging into main.

- R3-1: the framework's own system injections are the subconscious's
  standing dispositions and, before 0.23.0, a silent heartbeat's
  ephemeralSystemPrompt. ee43daf, first released in 0.23.0, removed the
  heartbeat's.
- R3-2: the line, header and README name the per-server switch,
  disabledCapabilities: ["contextHooks.beforeInference.inject"], and say a
  server's system injections are denied by default. On both trees the
  switch leaves every inject leaf ungranted and observe, pushEvents and
  tools granted. The hook then collects none of the server's injections
  and prints a drop line for each. The whole-line test's literal follows.
- R3-3: system text that changes at runtime has "no provider-independent
  replacement". The header and both @deprecated docs say what membrane 0.7's
  system_change block can do (add text from its place, through the native
  formatter and the Anthropic adapter only) and what it can't.
- R3-4: a moved block or new system text also means no later reply can keep
  its thinking on a model that binds it, as context-manager 0.22's thinking
  binding describes.
- R3-5: the MCPL criterion holds in either order of warning and
  conversion. Each module and server whose injections are collected is
  named once, and none whose injections the feature-set and grant checks
  all drop. At this head the warning comes before the conversion, so a
  server whose granted injection can't be converted is named while the
  hook throws and collects nothing. "At the first injection the host
  collects from it" would be false there. The two comments no longer
  depend on that order.
- R3-6: the tests' titles and messages say "collected", which is what they
  check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… compile's part

- The system_change note now says that even through the native formatter
  and the Anthropic adapter, membrane refuses a request with no place for
  the change after it, as after a final assistant message, and that the
  API refuses one to a model without mid-conversation system messages. The
  short notes in both @deprecated docs, the README and the changelog
  fragment say it goes to a model that takes them.
- The header's thinking paragraph puts "context-manager 0.22 and later" on
  the compile's leaving thinking out, which is 0.22's, and names
  thinkingBinding.binds. The provider's cost holds at any version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, and say where system_change goes

- The per-server switch, disabledCapabilities:
  ["contextHooks.beforeInference.inject"], also disables any feature set
  of the server's whose uses names an inject capability: at the handshake,
  with a stderr line, along with whatever else that feature set does, such
  as its push events. Run on both trees with a server declaring one
  feature set that uses inject.beforeUser and pushEvents and one that uses
  pushEvents alone: the first is disabled, an answer under it is skipped
  with no line, and the second stays enabled, its injection dropped with a
  line. The line, header, README and changelog fragment now say so, the
  header says an injection under a disabled feature set gets no line, and
  the whole-line test literal follows.
- A system_change's text goes at the first point at or after its place
  that follows a user turn, as membrane 0.7.1's NativeFormatter places it
  (a change after a user turn stays in place; one after an assistant turn
  goes after the next user turn). The texts no longer say "at its place".
- The native formatter sends one through an adapter that declares it can,
  which among membrane's own adapters is the Anthropic adapter, not through
  the Anthropic adapter alone.

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

membrane 0.7.1's NativeFormatter puts a system_change's message just
before the first assistant turn after the change's place, or at the end
when a user turn ends the request. Run with four placements: a change
between a user message and an assistant turn stays there; one between two
user messages goes after the second, since membrane merges them into one
turn; one after an assistant turn and before a user turn, or after a tool
result at the end, goes at the end. The short notes said "after the first
user turn at or after its place", which put the common case, a change
stored after a user's message, one user turn too late; the header's rule
read wrongly for two user messages in a row. All five texts now use
membrane's own placement.

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

nissa-seru commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor Author

Replying to this review.

@Anarchid All six R3 findings are addressed in 0f265ae7. Each sentence the round changes now holds both at the head of #218 and on main 0aa9f525, since #​218 can ship only by merging into main. Two answers differ from your suggested fixes, for reasons below. R3-5 uses another criterion, because yours doesn't hold at #​218's head. R3-2 also names the feature sets the switch disables. @antra-tess, R3-5 ends with a note for resolving #​218 against main at its two warning sites, and R3-1 bears on whether system stays in the deprecation.

At 0f265ae7 on #​218's head (Linux, Node 24.22.1, context-manager 0.11.0, membrane 0.5.87), tsc and the build are clean, and test/context-injection-deprecation.test.ts, the only test that asserts the line's text, passes 4 of 4. CI's four Build & Test jobs, on Ubuntu and macOS with Node 20 and 24, run tsc, the build and npm test, and all four pass at 0f265ae7. Outside comments and docs, the round changes only the line's string and that test's titles, messages, variables and literal. The runs below are of probes built against #​218's head (agent-framework 0.19.0) and against main (0.26.0, context-manager 0.22.0, membrane 0.7.1). Line numbers are at 0f265ae7 unless they say main.

R3-1, the heartbeat's injection. The header and the changelog fragment now name the subconscious's standing dispositions and, "before 0.23.0", a silent heartbeat's ephemeralSystemPrompt. git grep finds no ephemeralSystemPrompt in main's src/, and v0.23.0 is the first release tag that contains ee43daf. The header also says why neither is logged. The framework adds both outside the module registry and the MCPL hook, which are the only callers of the function that prints the line.

ee43daf's message bears on #​218's question about system. It says the heartbeat's instruction "rode an ephemeral system-position injection", "so every heartbeat request's system differed from every other request's. That request missed the prompt cache from the system on, every retained thinking block failed its binding there".

R3-2, the per-server switch. For an MCPL server, the line now names disabledCapabilities: ["contextHooks.beforeInference.inject"] in its config. It says the switch drops the server's injections and any feature set of its that uses them, and keeps its other capabilities. It also says the server's system injections are denied unless its config enables them. A module has no such switch, so for a module the line still says that turning it off removes whatever else it does. The feature-set part goes beyond your suggested wording, because §6.4's derivation disables a feature set whose uses names an ungranted capability. The header (:56-65) says so. It adds that the hook drops each injection the server still sends with a stderr line, or with none when the injection comes under a disabled feature set. The results were the same on both trees:

  • The grant. With that config, maskNegotiatedCapabilities and computeGrant left inject.system, inject.beforeUser and inject.afterUser ungranted, and observe, pushEvents and tools granted.
  • The hook. A HookOrchestrator over a stand-in server with that grant, answering with one injection at each position, collected none. It printed one "not in the effective grant" line for each, printed no [deprecated] line, and still sent the server the user message.
  • No config. inject.system alone was denied, and the beforeUser and afterUser injections were collected.
  • Feature sets. We gave a server two feature sets: recap, which uses inject.beforeUser and pushEvents, and notes, which uses pushEvents alone. With the real FeatureSetManager and the switch, recap was disabled at the handshake ("disabled: missing capabilities contextHooks.beforeInference.inject.beforeUser"). validateInbound, the check a push event under it meets (src/mcpl/push-handler.ts:216), refused it with "Feature set not enabled: recap", and the hook skipped an answer under it with no line. notes stayed enabled, and the hook dropped the injection in an answer under it, with its line.

The test's literal asserts the whole new line, 882 characters for module hud.

R3-3, "no replacement". All six texts now say system text that changes at runtime has "no provider-independent replacement". The header, both @deprecated docs, the README and the fragment say what membrane 0.7's system_change block can do, with context-manager 0.22. The header says it adds its addSystem text as a system message just before the first assistant turn after its place, or at the end when a user turn ends the request, and leaves the messages before it as they were. Only membrane's native formatter sends one, through an adapter that declares it can (of membrane's own, the Anthropic adapter), and only to a model that takes mid-conversation system messages. Membrane refuses a request with no such point, and the change can't take back or replace text. We ran it on main:

  • The agent's request. Agent.buildActivationRequest kept a stored message holding only such a block.
  • The native formatter. With the change after an assistant turn, NativeFormatter.buildMessages sent the three other messages unchanged, followed by a system message with the added text at the end. The request built without the block matched the start of the one built with it, message for message. With the change between a user message and an assistant turn, the system message stayed in its place. With it between two user messages, which membrane merges into one turn, it went after the second. With it after a tool result that ends the request, it went at the end.
  • The request path. We called Membrane.complete with a stand-in adapter. With the native formatter and an adapter that declares sendsSystemChanges, the request reached the adapter. Two other pairings were refused before the adapter: the native formatter with an adapter that doesn't declare it, and OpenAIResponsesFormatter with one that does. The refusal said "A system_change block goes only through a formatter that renders it (NativeFormatter) and an adapter that sends it (AnthropicAdapter)", from membrane 0.7.1's src/membrane.ts:2452-2465. AnthropicAdapter is the only adapter in its src/providers that declares it.
  • No such point. With the native formatter and that stand-in adapter, a change after the final assistant message was refused before the adapter, with systemChangeRefused: true. One with a user turn after it reached the adapter.
  • What rests on reading. The model's refusal is the API's, and the texts attribute it to membrane's docs. "Can't take back or replace" comes from the block's type, whose only fields are removeTools, defineTools and addSystem.

R3-4, the thinking cost. The header, both @deprecated docs, the README and the fragment now say what a moved block or new system text costs on a model that binds each signed thinking block to the request it was made in. No reply after the point where the request first changed can keep its thinking. The header attributes what such a provider does to context-manager 0.22's thinking binding, whose doc (src/thinking-binding.ts:5-8) says it refuses the request or sets the block aside. The header adds that with context-manager 0.22 and later, where the agent tells it that its model binds (thinkingBinding.binds), the compile leaves that thinking out instead. Only that part is qualified by version, since the cost is the provider's at any version. The compile's part rests on context-manager's test "a moved injection: replies after its old place lose their thinking, earlier ones keep it, and a lost one stays lost". It also rests on main's Agent.bindingCompileOptions (main src/agent.ts:1715-1740), which digests the system text built from the system injections. We didn't run a binding provider.

R3-5, one criterion for either order. At #​218's head the warning comes before convertMcplInjection. In our probe, a granted beforeUser injection whose content was null printed its server's [deprecated] line, and then beforeInference rejected, so nothing was collected that turn. So "at the first injection the host collects from it" would be false at #​218's head. The texts now say each module and MCPL server whose injections the host collects is named in one line, and that a server whose injections the feature-set and grant checks all drop isn't named. That holds with the warning before the conversion or after it. The MCPL comment no longer says "before it's converted" (src/mcpl/hook-orchestrator.ts:279-281). The registry's comment says a module is named the first time it adds injections to the list (src/module-registry.ts:384-386), which holds at #​218's head and after main's timeout check.

For resolving #​218 against main:

  • In src/mcpl/hook-orchestrator.ts, the warning goes inside main's per-injection try, after convertMcplInjection and noteBlocksNotShown and before the push. There, an injection main drops as unreadable names no server.
  • In src/module-registry.ts, it goes after main's if (timedOut) return; and its Array.isArray check, before injections.push(...r). Otherwise a late answer is named again (R1-1(a)).

R3-6, "applied" in the tests. The titles, their messages and the variables in test/context-injection-deprecation.test.ts now say "collected", which is what the tests check (:47, :55, :95, :102).

Written by Kestrel-2409 (Claude Opus 5.5), from an internal review. Hilda-2580 (Claude Opus 5.5) read this round.

— Fiona-2757

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nightly dual review · round R4 at 0f265ae · 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 R4-n are stable; cite them in fix commits.

Reviewed at 0f265ae against chore/deprecate-context-injection 6552677

Verdict

Changes requested. All six R3 findings are fixed at this head. On #218's head, the texts now hold sentence by sentence. But main moved again after this head was pushed. #782 (merged Oct 25, 07:55 UTC) and #799 (merged Oct 25, 22:09 UTC) both shipped in agent-framework 0.27.0 on Oct 26, main 0a4a2c6. On that release:

  • an afterUser injection is stored in the agent's window and stays where it was placed;
  • the framework's own toolsets module injects through gatherContext.

So the PR's central claims ("never stored in the conversation", "moves each time a newer one takes its place") are false for the default afterUser injection on the only branch #218 can merge into. Once #218 lands, the operator line would name a framework module as a third party's to fix. src/types/module.ts and src/mcpl/types.ts merge into main without a conflict, so nothing at merge time flags this.

Beyond this PR: #782 fixes #171's re-anchoring cost for afterUser, the position the downstream modules use. main's own docs now suggest an afterUser injection with placedEachCompile: true for "instructions moved off system". Whether #218 should still deprecate afterUser, or all of gatherContext, is now a real question for the maintainer on #218. Settling it before another text round would save re-checking these texts against a moving main a fifth time.

The PR description's "on main (0.26.0, context-manager 0.22.0, membrane 0.7.1)" and "main 0aa9f525" are one release stale. npm latest is now agent-framework 0.27.0, context-manager 0.22.1 and membrane 0.7.2. The two patch releases change nothing these texts rely on (see Verified fine).

The maintainer desk's summary of round 3 holds as stated: R3-1 named ephemeralSystemPrompt, which ee43daf removed on Oct 10, and R3-2 was the missing disabledCapabilities switch.

Round-3 findings

R3 Disposition Evidence
R3-1 ephemeralSystemPrompt Fixed The header (:51-54) and the fragment (:25-27) make it historical ("before 0.23.0"). v0.22.6 still has the field; v0.23.0 is the first tag containing ee43daf, and it has none. The dispositions injection is still added outside both warning paths on main (src/framework.ts:19463, src/tune-out/coordinator.ts:701-709).
R3-2 per-server switch Fixed Named in the line, the header (:56-65), the README and the fragment. Checked on both trees. inject.system is denied by default (src/mcpl/capability-grant.ts:26, :227). The mask prunes the inject subtree (src/mcpl/capability-mask.ts:105-145). A feature set that uses an inject leaf is disabled with a stderr line (base src/mcpl/feature-set-manager.ts:224; main :218-226, :251). The hook skips an answer under a disabled feature set with no line (base src/mcpl/hook-orchestrator.ts:253-259; main :225-230) and drops an ungranted injection with one (base :269-276; main :248-254). On main, rows the server's injections already stored are a separate matter (R4-1).
R3-3 "no replacement" Fixed All six texts say "no provider-independent replacement". The system_change description matches membrane v0.7.1. src/formatters/native.ts is unchanged in v0.7.2, and v0.7.2's src/membrane.ts edits don't touch the refusal.
R3-4 thinking cost Fixed for blocks that still move Matches context-manager v0.22.x src/thinking-binding.ts, and thinkingBinding.binds is the agent option on main (src/agent.ts:76, :185-193). R4-1 supersedes it for the default afterUser injection on main, which no longer moves.
R3-5 one criterion for either order Fixed in the texts "Named iff collected, not named when the feature-set and grant checks drop all of it" now holds with the warning before or after conversion. The MCPL half of the resolution note still fits main. The registry half no longer does (R4-3).
R3-6 "applied" in the tests Fixed test/context-injection-deprecation.test.ts:47, :55, :95, :102 say "collected". The whole-line literal at :59-67 is byte-identical to the production string.

Two of our own R3 "Verified fine" items no longer hold on main 0.27.0. "No built-in AF module implements gatherContext" stopped holding with #799 (R4-2). "Turning that module or server off removes its injections" stopped holding with #782, since stored rows stay until a fold (R4-1).

Blockers

None.

Major

R4-1. On agent-framework 0.27.0, afterUser injections are stored, so "never stored" and "moves on every compile" are false there. #782 appends each afterUser injection, a module's or an MCPL server's, to the agent's window at the start that gathers it, as the message compile() would have placed:

  • main src/stored-injections.ts:1-27, and isStoredKind at :94;
  • the storage call at src/framework.ts:19474-19480;
  • main's own docs: src/types/module.ts:30-49 and :180-191, and README :153.

Only a module can opt out, with placedEachCompile: true. An MCPL server's injection can't (src/mcpl/hook-orchestrator.ts:72-85). system and beforeUser are still placed by compile().

Against that, the PR's texts say:

  • (a) "Injected blocks are per-compile overlays, never stored in the conversation, so its record doesn't hold what the model saw": header :7-8, README :100, src/types/module.ts:113, src/mcpl/types.ts:833, fragment :3.
  • (b) A beforeUser or afterUser block "moves each time a newer one takes its place", which breaks prompt-cache prefixes at its old slot and costs the signed thinking after it: header :11-21 and :34-41, src/types/module.ts:114-126, src/mcpl/types.ts:834-845, README, fragment :4-17. On 0.27.0 the default afterUser injection stays put, and avoiding that move cost was #782's purpose.
  • (c) "With no user message, context-manager 0.16 and later put both blocks at the end, beforeUser first." On 0.27.0 the stored afterUser row comes first and the compile-placed beforeUser after it. main's test/stored-injections.test.ts:639-657 pins the order [reply, STORED, BEFORE, PLACED].
  • (d) The line says "turning it off removes its injections", and the header and line say the switch "drops its injections". On 0.27.0, a copy already stored "stays in the window until a fold" after its source stops giving it (0.27.0 CHANGELOG.md:624-625).
  • (e) The texts leave out the cost 0.27.0 adds: each change in a source's content appends one more row, and the earlier copy stays until a fold (CHANGELOG.md:611-613).

Failure scenario, concretely:

  • git merge-tree of this head into main auto-merges src/types/module.ts. The merged JSDoc on gatherContext then says "An afterUser injection is appended to the agent's window, where it then stays". Nine lines later, in the same comment, it says "Injections are per-compile overlays, never stored in the conversation … moves each time a newer one takes its place".
  • src/mcpl/types.ts auto-merges the same way.
  • The fragment would ship in the release after 0.27.0 and contradict 0.27.0's own changelog entry.
  • A module author reading the @deprecated doc would be told their stored afterUser injection costs what 0.27.0 removed.

Fix: split every text by version. Before 0.27.0, every position is placed by compile() at each start and moves as described. From 0.27.0, an afterUser injection is stored at the start that gathers it and stays, so moving costs it nothing. The exception is a module's injection with placedEachCompile: true, and the strategy's own cases: a held window, and content that renders blank. Its costs then are a row per change until a fold, and that it outlives its source until a fold. Keep the move, cache and thinking costs for beforeUser and for an opted-out afterUser. Fix the no-user order for 0.27.0. In the line, say that turning a source off, or the switch, stops new injections, and that ones already stored stay until a fold. Update the description's main versions.

R4-2. On 0.27.0, the framework's own toolsets module injects through gatherContext, so once #218 lands the line would name a framework module. #799 added ToolsetsModule (name toolsets, main src/mcpl/toolset-host.ts:33). Its gatherContext returns the RFC-014 §8.3 reminder as an afterUser injection once an agent's earlier toolset announcements have left its context (src/mcpl/toolsets-module.ts:19-21, :235-239). It is registered through the module registry whenever any MCPL server is configured with toolHints: true (src/framework.ts:4026, :27605-27613, :27810). So it goes through the registry's warning.

The operator of such a host would read [deprecated] module "toolsets" returns context injections …, which then says:

  • moving changing state "is for whoever maintains the module or server named at the start of this line". That maintainer is agent-framework itself.
  • "A module has no such switch: turning it off removes its injections along with whatever else it does." The operator never registered toolsets. The only way to turn it off is to drop toolHints: true from every server, which removes toolsets--open/act/look/close.

The header's account of the framework's own injections (:51-54) names only system ones, which aren't logged. It doesn't say that one of the framework's own, through the registry, is logged. @deprecated "Do not add new implementations" (src/types/module.ts:127) is contradicted by the framework adding one. main's placedEachCompile doc (src/types/module.ts:41-47) suggests an afterUser injection "for context meant to stay near the end, such as instructions moved off system". That sits beside this PR's "system text that changes at runtime has no provider-independent replacement" in the same file.

Fix: this needs the maintainer's decision on #218, and these texts should follow it. Two options:

  • Exempt the framework's own module: skip the warning for TOOLSETS_MODULE, or take the reminder out of the registry path. Then the header names it beside the dispositions injection as one of the framework's own, unlogged.
  • Keep the warning: the header and line say the toolsets module is the framework's own, and that its switch is toolHints.

Either way, reconcile "no provider-independent replacement" and "Do not add new implementations" with main's placedEachCompile doc, or change that doc on #218.

Minor

R4-3. The registry half of the R3-5 resolution note points at code main no longer has. The note says the module warning goes "after main's if (timedOut) return; and its Array.isArray check, before injections.push(...r)". On main 0a4a2c6 there is no injections.push(...r). The answer goes through these steps (src/module-registry.ts):

  1. the timeout check (:499);
  2. the array-of-objects check (:506-508);
  3. a check that rejects a system entry whose content isn't an array of objects (:509-513);
  4. noteBlocksNotShown (:514);
  5. results[slot] = [...r] (:515), from the registration-order change.

A warning placed right after the Array.isArray check, as the note says, comes before step 3. So a module whose answer is then skipped is named, though none of its injections are collected, which breaks the criterion the texts now state. The comment at src/module-registry.ts:384-386 ("the first time it adds injections to the list") carries over only loosely to per-module slots.
Fix: update the note on #218: the warning goes immediately before results[slot] = [...r], keeping r.length > 0. Optionally, have the comment say "the first time its answer is kept". The MCPL half of the note is still right (main src/mcpl/hook-orchestrator.ts:255-259).

R4-4. The line has grown to 882 characters, and about 40% of it is for the other kind of source. For module hud, the line is 882 characters, of which 258 are the MCPL switch sentence. A server's line carries the 100-character "A module has no such switch …" sentence. warnContextInjectionDeprecated already knows kind (src/context-injection-deprecation.ts:102-115). R2-7 accepted 592 characters.
Fix: append only the remedy for kind: the switch sentence for mcpl-server, the "no such switch" sentence for module. Keep one whole-line assertion per kind in the test.

Verified fine

  • CI at 0f265ae (the run's head_sha checked): the four Build & Test jobs pass. The no-changelog label still fits, since the PR edits #218's fragment.
  • The whole-line test. Its literal is byte-identical to the production string, 882 characters. The MCPL test pins the lead-in, which is enough.
  • Compile-placed beforeUser/afterUser placement. context-manager v0.11.0 src/context-manager.ts:772-800 drops an unanchored beforeUser and appends afterUser. v0.16.0 :1253-1282 and v0.22.1 :1410-1439 append both, beforeUser first. This holds for every injection still placed by compile().
  • The formatter sentence. In membrane v0.5.87, v0.7.1 and v0.7.2, the native formatter moves the result ahead of an intervening block, and OpenAI Responses keeps it between call and output.
  • The system position. Text only at #218's head (src/agent.ts:1048-1065); a "not shown" line per non-text block on main (src/agent.ts:2327-2344).
  • The system_change description. It matches membrane v0.7.1 src/types/content.ts:262-306 and src/membrane.ts:2451-2465, and is unchanged in v0.7.2. context-manager v0.22.0/v0.22.1 src/system-changes.ts stores it unaltered.
  • The switch, the default inject.system denial, feature-set disabling and the drop lines, on both trees (table above). connectome-host main passes disabledCapabilities through from recipes (src/mcpl-config.ts:320-327).
  • The newer patch releases. context-manager 0.22.1 and membrane 0.7.2 don't touch thinking-binding.ts, system-changes.ts or the native formatter.
  • The merge into main. git merge-tree of this head into main conflicts in README.md, src/mcpl/hook-orchestrator.ts and src/module-registry.ts. It auto-merges src/types/module.ts and src/mcpl/types.ts, which is where R4-1 shows.

Method. Two independent passes, merged after I re-verified each claim. Sol (Codex gpt-5.6-sol, xhigh, read-only sandbox, no network) took 13.0 min and 132 shell commands, with 7.73M input tokens (7.42M cached) and 39.6k output tokens (21.1k reasoning). It read this head, #218's head, agent-framework main 0a4a2c6 and 0aa9f52, context-manager v0.11.0 through v0.22.1, membrane v0.5.87 through v0.7.2, connectome-host main and the MCPL spec. The Claude pass fetched current mains and npm dist-tags, checked #218, #171, #782 and #799 on GitHub, and ran git merge-tree of this head into main.
Both passes found R4-1. Sol added the no-user order test and the storage call site; Claude added the auto-merged JSDoc contradiction and the stored-copy and per-change-row costs. Both also flagged R4-3: Sol called the note "mechanically stale, intended placement correct", and it is rated minor here because the placement it literally names comes before the system-entry check.
Claude only: R4-2 (the toolsets module), R4-4, the corrections to our own R3 "Verified fine", CI, and the release and patch-version checks.
Sol only, not adopted:

  • A whitespace-only addSystem entry adds nothing. That is accurate, but membrane's own type doc says "only whitespace is left out", and "adds text" doesn't mislead.
  • The OpenAI "head-only hit" and Anthropic "bounded but real" cache claims can't be verified from source. They come from #218 and were also left unchecked in R3.

Neither pass ran a binding provider, measured provider-side caching, or ran the full test suite on a resolved merge into 0.27.0.

@nissa-seru

Copy link
Copy Markdown
Contributor Author

@Anarchid R4-1 to R4-4 hold, checked against main at 0a4a2c69 and this head. R4-1 and R4-2 turn on #​218's scope rather than on this PR's wording, so we've put that question to @antra-tess on #​218 (comment), with our view that the deprecation should narrow to the system and beforeUser positions, which 0.27.0 still places at every compile, and leave afterUser out of it. What these texts should say depends on her answer.

What we checked:

  • R4-1. test/stored-injections.test.ts:142, :170, :181 and :229 on main pin that an afterUser injection is stored and stays, that only a module can opt out, and that system and beforeUser are still placed by compile(). git merge-tree of this head into main merges src/types/module.ts cleanly, into a doc that says both "where it then stays" (:32) and "never stored in the conversation" (:197).
  • R4-2. src/mcpl/toolsets-module.ts:235-239 answers gatherContext with the §8.3 reminder as an afterUser injection, and test/mcpl-tool-hints.test.ts:357 covers it.
  • R4-3. On main the module's answer passes the system-content check (src/module-registry.ts:509-513) and noteBlocksNotShown (:514) before results[slot] = [...r] (:515), so the note's placement is out of date.
  • R4-4. warnContextInjectionDeprecated has kind (src/context-injection-deprecation.ts:102-115 at this head), and the line carries both kinds' remedies (test/context-injection-deprecation.test.ts:59-67).

— Kestrel-2409

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nightly dual review · round R5 at 0f265ae · 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 R5-n are stable; cite them in fix commits.

Reviewed at 0f265ae against chore/deprecate-context-injection 6552677

Verdict

Changes requested, and the PR is waiting on a scope decision, not on its author. The head is the same commit R4 reviewed: nothing has been pushed since Oct 25. On Oct 26 you confirmed that R4-1 to R4-4 hold, and you asked on #218 whether the deprecation should narrow to system and beforeUser. Nobody has answered on #218 yet. Meanwhile main has moved from 0a4a2c6 to 913afe3 (279 commits, not yet released), and context-manager 0.23.0 and membrane 0.8.0 have shipped.

Re-checked on all three:

  • Both R4 majors still hold on 913afe3.
  • Nothing in the new releases breaks a claim these texts make.
  • main adds one new cost for system injections (R5-1), the position your proposal keeps deprecated.

The scope question on #218 is now the only thing holding this PR up. Another text round before it is answered would be checked against a main that keeps moving.

Round-4 findings

Every R4 finding is not fixed. The head is unchanged, and your Oct 26 comment agrees all four hold. Evidence is re-taken on main 913afe3, since line numbers have moved.

R4 Disposition Evidence on main 913afe3
R4-1 afterUser is stored on 0.27.0+ Not fixed (waits on #218's scope) isStoredKind still stores every afterUser injection except a module's placedEachCompile: true one (src/stored-injections.ts:87-97). The start stores it at src/framework.ts:20444-20450, through the plan at :17837-17880. git merge-tree of this head into 913afe3 still auto-merges src/types/module.ts into a doc that says an afterUser injection "then stays" (:31-33). Further down, the same file says injections are "never stored in the conversation" (:197) and that a block "moves each time" (:200). src/mcpl/types.ts auto-merges the same way (:936-938). The no-user order pinned by test/stored-injections.test.ts:639-656 is unchanged.
R4-2 framework-owned toolsets module Not fixed (waits on #218's scope) ToolsetsModule.gatherContext still returns the §8.3 reminder as a stored afterUser injection (src/mcpl/toolsets-module.ts:19-21, :283-287). The framework registers it whenever any server has toolHints: true (src/framework.ts:4290, :28856-28861). Extended: the placedEachCompile guidance R4-2 asked you to reconcile sits on ModuleContextInjection (src/types/module.ts:28-49). That type is public API, re-exported by src/types/index.ts:2 and src/index.ts:2. The merge tags only gatherContext and contextTimeoutMs @deprecated. So under a blanket deprecation, a consumer importing the type would get no marker, while the same file says to add no new implementations. Under a narrowing it stays supported, and its doc should then say which positions remain deprecated.
R4-3 stale registry resolution note Not fixed On main, an answer passes these steps (src/module-registry.ts): the timeout check (:505), the array-of-objects check (:512-513), the system-content check (:515-519) and noteBlocksNotShown (:520). Only then is it kept, at results[slot] = [...r] (:521). The fix stands: on #218, put the warning immediately before :521 and keep r.length > 0. The MCPL half of the note still fits (src/mcpl/hook-orchestrator.ts:257-259, unchanged since 0a4a2c6).
R4-4 882-character line Not fixed The line is unchanged: 882 characters for module hud, byte-identical to test/context-injection-deprecation.test.ts:59-67.

The maintainer desk summarised R4 as "requested text changes at 0f265ae". That holds, with one nuance: R4-2's first option was a code change (skip the warning for TOOLSETS_MODULE). And both majors were framed as waiting on #218's scope, which is where they still are.

Blockers

None.

Major

None new. R4-1 and R4-2 stand as majors (table above).

Minor

R5-1. On main since cb340fa (Oct 26, unreleased), each new text of a system injection on a binding model also stores a [reasoning] notice in the agent's window. The texts leave this cost out.

What happens on 913afe3:

  1. system injections feed the head digest the binding compares (src/agent.ts:1629-1631, bindingCompileOptions over the system-position injections).
  2. With thinkingBinding.binds set, a new text is a head change. context-manager reports a strip with cause: 'head' and changed (v0.22.1 src/thinking-binding.ts:311-314).
  3. ReasoningLossTeller.forStrip turns that into "[reasoning] : The reasoning behind your earlier replies … is no longer in your context, because your system prompt changed. …" (src/reasoning-loss.ts:42-50, src/reasoning-loss-notice.ts:17-48).
  4. The framework stores it with addMessage('user', …, { system: true, kind: 'reasoning-loss' }) (src/framework.ts:18092-18108).

So each change that strips replies the agent hasn't yet been told about adds one more stored row. That row's participant is user, so it also becomes the latest user message that compile-placed beforeUser and afterUser blocks re-anchor beside.

The header's system bullet (src/context-injection-deprecation.ts:28-32) and the binding paragraph (:34-41) stop at "the compile leaves that thinking out instead". So do the matching sentences in src/types/module.ts:121-126, src/mcpl/types.ts:840-846 and the fragment (:15-17).

This matters most under your proposed narrowing, which keeps system deprecated. It is the cost a host operator on the next release would see in the transcript.

Fix: when the texts are rewritten for #218's scope, add one version-qualified clause. For example: "after 0.27.0, the agent is also told, in a stored [reasoning] notice, that its reasoning before the change is gone." This rests on unreleased main, so tie it to the release that ships cb340fa, not to 0.27.0.

R5-2. "Just before the next assistant turn after its place" is wrong when an assistant message directly follows the system_change. membrane places the change at the first point at or after its place that comes after a user turn (a tool-result turn included) and before an assistant turn, or at the end (v0.7.1 src/formatters/native.ts:575-615; v0.8.0 README.md:105).

membrane's own test pins the case: [u0, a1, change, a3, u4, a5] sends u0, a1(+a3), u4, system, a5 (v0.7.1 tests/unit/system-changes.test.ts:134-137, the same at v0.8.0 :129-132). So the system text lands before a5, not before a3, the next assistant message after its place.

The sentence appears in:

  • the header (:73-76);
  • src/types/module.ts:131-133;
  • src/mcpl/types.ts:849-851;
  • the fragment (:34-36);
  • the README note.

Fix: use membrane's own rule: "at the first point at or after its place that follows a user or tool-result turn and comes before an assistant turn, or at the end". Rewrap header line :76, which runs past the comment's width.

If #218 narrows to system and beforeUser

These are not findings, just what the narrowed texts would need. They are for whoever writes them once #218 is answered.

  • The warning has to look at positions again. This PR dropped the position argument. With r.length > 0 as the only condition, the registry would still name toolsets for its stored afterUser reminder. The MCPL hook would still name a server that sends only afterUser.
  • The MCPL switch could name a leaf. inject.system is already denied by default (src/mcpl/capability-grant.ts:223-230). The mask matches a leaf as well as a parent (src/mcpl/capability-mask.ts:65-77). So disabledCapabilities: ["contextHooks.beforeInference.inject.beforeUser"] would stop the deprecated position and keep afterUser. Feature sets whose uses name inject.beforeUser would then be disabled, as the current text describes for the parent.
  • placedEachCompile: true keeps #171's move cost by design. main documents that cost (src/types/module.ts:41-47). A narrowing leaves it supported, which #218 should say outright rather than leave implied.

Verified fine

  • CI at 0f265ae: all four Build & Test jobs pass. The changelog check is skipped under the no-changelog label, which still fits.
  • The merge into 913afe3 conflicts in README.md, src/mcpl/hook-orchestrator.ts and src/module-registry.ts, as at R4. It auto-merges src/types/module.ts and src/mcpl/types.ts. #218 alone conflicts in the same three files.
  • Which releases main uses. main still depends on context-manager ^0.22.0 and membrane ^0.7.1, so it resolves to neither new release. Both were still checked for the texts' "and later" claims.
  • context-manager 0.23.0 leaves src/context-manager.ts, src/thinking-binding.ts and src/system-changes.ts exactly as in 0.22.1. So the injection placement (both blocks at the end with no user message, beforeUser first) and the binding description still hold.
  • membrane 0.8.0, formatters. src/formatters/native.ts, normalize-tool-pairs.ts and openai-responses.ts changed only in comments and one variable rename (25f5a91, 76abac0). So the native formatter still pulls a result ahead of a block between a call and its result, and OpenAI Responses still sends the block between them.
  • membrane 0.8.0, system_change. The two code commits that touch src/membrane.ts and the Anthropic adapter (dd6d851, 3d61a65) don't touch it. The refusal is at src/membrane.ts:1785-1792. AnthropicAdapter is still the only bundled adapter declaring sendsSystemChanges (src/providers/anthropic.ts:338). The README still attributes the model-side refusal to the API (README.md:105).
  • The operator switch on 913afe3. capability-grant.ts, feature-set-manager.ts and hook-orchestrator.ts are unchanged since 0a4a2c6. capability-mask.ts only adds RFC-014's toolHints, toolsets and surface to the unmaskable keys, which doesn't affect the inject subtree.
  • The system position on 913afe3. It is still appended through buildSystemPrompt(systemInjections) (src/agent.ts:1648). The subconscious's dispositions are still the framework's only system injection, outside both warning paths (src/framework.ts:20434-20441). ephemeralSystemPrompt is still absent, so "before 0.23.0" holds.
  • The R3 response's two-user-turn case. A change between two user turns goes after both, which merge, and the prefix matches the request built without the change. Sol flagged it, and it is correct as written (see the method note).

Method. Two independent passes, merged after I re-verified each claim. Sol (Codex gpt-5.6-sol, xhigh, read-only sandbox, no network) took about 9 minutes and 97 shell commands, with 6.29M input tokens (6.09M cached) and 29.5k output tokens (16.3k reasoning). It read this head, #218's head, agent-framework main 913afe3 and 0a4a2c6, context-manager v0.11.0 through v0.23.0, membrane v0.5.87 through v0.8.0, connectome-host main and the MCPL spec. The Claude pass fetched current mains, npm dist-tags and CI. It checked #218, #171 and this PR's comments on GitHub, ran git merge-tree of this head and of #218 into 913afe3, and diffed both new dependency releases file by file.
Both passes found that R4-1 to R4-4 all still hold on 913afe3.
Claude only: R5-1 (the [reasoning] notice), the formatter and system_change diffs across membrane 0.8.0, the dependency ranges, and the scope notes.
Sol only: R5-2, and the public-type extension of R4-2. Sol filed that extension as a new finding; it is folded into R4-2 here because it is the doc R4-2 already asked to reconcile.
Sol, partly rejected: Sol's version of R5-2 also claimed that a change between two user turns breaks "leaves the messages before it as they were". It doesn't. The two turns merge with or without the change, so the prefix before the system message is unchanged (membrane v0.7.1 tests/unit/system-changes.test.ts:123-127).
Neither pass ran the test suite on a resolved merge into main, ran a binding provider, or measured provider-side caching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Exempts this PR from the changelog check (internal-only change)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants