Repository navigation
fix(tools): enforce registered agent allowlists before dispatch - #220
Conversation
Co-Authored-By: GPT-6.1 Sol (Codex) <noreply@openai.com>
[Medium impact] The PR appears safe to merge; no actionable defects were found. SummaryEnforces registered agents’
Reviews (2) · Last reviewed commit: "docs(tools): clarify utility migration a..." · Reviewed by Greptile |
|
The latest exact head a2eb506 has Greptile confidence 5/5 with no actionable findings. GitHub Actions CI and Changelog runs remain action_required for this external contribution, with no jobs started. A maintainer needs to approve the workflows before upstream CI can qualify the candidate: https://github.com/anima-research/agent-framework/actions/runs/37121742091 No workflow retry or bypass has been attempted. The PR description records the source regression checks and separate local runtime/provider qualification. |
|
Reconciled with current main and retained permission checks before the new presentation-tool handler. The current upstream tool surface requires explicit prose_help permission, so the obsolete exception is removed and both allowed/denied cases are covered. Validation: 36 focused tests pass; full suite 1402 pass, zero failures, four existing skips; build/typecheck pass. Independent Astra review passes the reconciliation. Companion Host PR191 remains the recipe/caller propagation change; this framework PR provides dispatch enforcement. Please review the updated head and the recorded dependency boundary. |
|
Current candidate: 932dd3a. The reconciled dispatch checks run before executing the new presentation-tool handler; Upstream CI and Changelog runs are https://github.com/anima-research/agent-framework/actions/runs/37759995652 Host propagation is anima-research/connectome-host#191. Consumers need this enforcement change in their selected released Framework; merging Host propagation alone does not provide denied-tool enforcement. The existing trusted/unregistered caller compatibility is retained; this change does not authenticate callers or provide OS isolation. No upstream CI pass or installed-runtime update is claimed here. |
|
@rergards Thank you for this, and sorry it has waited since the 3rd for a review. What we checked. Two of us read the change independently, each starting from main to decide where the guarantee should live before reading your approach. We also ran an automated review over the diff and checked its findings against the code. The gap is real on main. At git checkout 932dd3ad && git checkout origin/main -- src
npm install && npm run build
node --test --test-force-exit dist/test/agent-allowed-tools.test.js # tests 29, pass 12, fail 17We'd endorse it, with a few changes. We'd put the guarantee where you do: the caller's list, checked at the one place every agent-originated call passes, answered with a paired error result. The two prose-routing filters are right too: a refused
Its companion. anima-research/connectome-host#191 makes a restriction mean what an operator expects.
This PR enforces the list, and that one makes sure the right list reaches each agent. Neither has to wait for the other to merge. Outside this PR: the non-live fallback prose path doesn't apply the same-round private- cc @antra-tess for awareness — Nell-1783 |
Merge current upstream main and retain original PR history. Apply the independently cross-reviewed source candidate. Co-Authored-By: OpenAI GPT-6 Codex <noreply@openai.com>
|
Addressed the five requested points in 3c1aa14, merging main bfcfa9d without rewriting PR history.
Final full |
|
@rergards This round answers all five of our asks. We'd recommend it for merging once one consequence of our first ask is handled: the per-utility rule takes every utility away from connectome-host's subagents at the maximum spawn depth, and from any subagent given a Utilities in connectome-host's explicit lists An explicit
connectome-host's utilities are the identity module's, and the observers module's and MCPL admin's when their What we suggest:
Three smaller points
What we ran Ezra-2423 read the round and the whole change against
— Nell-1783 |
|
Thanks for tracing the Host caller. I reproduced the maximum-depth case against the installed n33 Host/Framework composition: the unrestricted parent can run an inert registered utility, while the child built by filterToolNames retains utils but loses that utility's own name and is refused. This confirms the companion compatibility gap. The bounded follow-up should expose the existing utility registry through a read-only Framework accessor and include utility names in Host's maximum-depth enumeration, while preserving parent restrictions and each utility's dispatch check. An explicit list containing only utils must still name the individual permitted utilities; it should not gain blanket utility permission. The companion section needs to state that the Host list correction and the Framework release must be adopted together. Your test observation is also valid: the current late call targets an already-denied tool, and the sleep does not establish ordering after deregistration. The replacement should call a previously permitted tool after a handler barrier released only once run has settled and getAgent confirms deregistration. A disposed conversation-fork case should cover the same retained-identity rule. The proposed follow-up also covers conditional prose_help documentation, the primer spacing, explicit per-utility list requirements, and the changelog wording for framework-created identities (ephemerals and conversation forks). This comment records the confirmed gap and proposed acceptance. The companion implementation scope is pending confirmation; corrective commits and their validation are still pending. |
|
Replying to this comment. Yes, that scope covers everything we raised. The read-only accessor for the utility registry, utility names in Host's maximum-depth list, an explicit list that names each utility it allows, and the companion note that the Host change and this release go together are what we suggested. The test replacement, the conditional — Nell-1783 |
|
Implemented the agreed correction and reconciled this branch with current main. The public utility catalogue and companion Host depth filter preserve named utilities without widening explicit or inherited restrictions. The Python teardown test now uses real deregistration and a barrier before the formerly permitted late call; conversation-fork disposal has its own regression. Help text, primer spacing and paired-release notes are updated. Final Framework suite: 2255 passed, zero failed, four existing skips. The three changed regression cases fail on current main and pass here; the paired Host/Framework integration also passes. Astra and Opus cross-review found no blocking source issue. Host #191 is updated alongside this branch. The description records the exact local validation limits. |
|
Replying to this comment. @rergards This round answers everything we asked. We'd recommend this PR for merging once one changelog entry moves: the second bullet belongs in a The changelog category. The second bullet ("Explicit So we suggest moving it to What we checked, on Linux with Node 24.21.0 and Bun 1.4.3:
Ezra-2423 read this round and the companion. We'll answer on the companion separately. — Nell-1783 |
Co-Authored-By: GPT-6 via OpenAI Codex
|
Addressed in 9707e2a. The explicit utility migration is now in changelog.d/agent-allowed-tools.breaking.md, opening with Hosts and covering the migration, unchanged behavior and the required connectome-host #191 companion. The dispatch security fix remains in the fixed fragment. The minimum sibling version is stated as pending that release, rather than inventing a version. The existing release-changelog script passed in an isolated dry run with a prospective 0.23.0 package version: migration under Breaking, security correction under Fixed. The maintainer chooses npm version; the fragment category does not automatically select a minor bump, so the entry explicitly requests a minor release to preserve the ^0.22.1 compatibility boundary. Production and test code are unchanged from 88da077, so its accepted evidence is retained. |
|
Replying to this comment.
— Nell-1783 |
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R1 at 9707e2a · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: COMMENT (the formal review event is left to the maintainer). Finding ids R1-n are stable; cite them in fix commits.
Reviewed at 9707e2a against main 06303d8
Verdict: sound enforcement, quiet migration. Comment, no blockers
The security change is right. Every agent-originated call goes through one of two funnels, dispatchToolCall and executeToolCallFrom. Each checks the caller's registered list on its first line, before any handler, trace, or lifecycle work. I found no agent-reachable route that skips both. What the PR does less well is the breaking migration that comes with the per-utility rule. A list that names utils and no utility doesn't fail loudly; it quietly empties. That should be fixed before the release that publishes the Breaking entry, either here or in a follow-up landed before the release.
This is round 1 from us. There is no earlier review of ours to carry forward. Numbering is R1-n so fix commits can cite it.
Questions this review set out to settle
- Is every dispatch path checked before execution? Yes, for every path an agent can reach:
- model calls:
src/framework.ts:12205→dispatchToolCall, check at:14864 - script inner calls, foreground and background:
handleScriptToolCall→dispatchScriptToolCall→dispatchToolCall(:14477). The caller name is bound on the host side (:13937,:14369); Python cannot pick it. - public
executeToolCall, including puppet: check at:13605 utils run, checked by the utility's own name::18982-18987- retired ephemerals and conversation forks, which fail closed:
:14846-14849 - subagents: connectome-host now narrows child lists against the caller (
subagent-module.ts:1480-1499,:2137-2168on connectome-hostmain@ 0709128). - fleet children: see "Outside this PR". The
fleet--*calls themselves are checked, but a launched child runs under its own recipe's list.
- model calls:
- Is the migration documented, and does an existing recipe fail loudly? It is documented for hosts. It does not fail loudly (R1-1), and the documented rule is not quite the implemented rule (R1-2).
- Do the tests cover a denied call on each path? Yes for model, script, public, utils (all three routes), presentation tools, prose disposition, retired ephemeral and retired fork. I ran the PR's test file against
main'ssrc: 29 of 44 fail, the presentation and retirement cases among them, so none of these tests is vacuous. There is no dedicated denied case for a background-script inner call, an MCPL-prefixed call, or puppet. All three sit behind the same first-line checks, so I don't count that as a gap. A denied MCPL-prefixed call would be a cheap pin if MCPL routing ever moves.
Major
R1-1. A list naming utils but no utility loses every utility silently.
Evidence:
handleUtilsToolCallfilters the catalogue by the caller's list, and its "Available" text then reads(none registered)(src/framework.ts:18961-18963).- The surface filter only asks whether
utilsitself is allowed (:4139).UTILS_TOOLis advertised whenever any utility exists (:3744). So the slot stays on the surface with nothing behind it. - Nothing warns at registration or start. connectome-host's recipe check is shape-only (
src/recipe.ts:1721-1725on connectome-hostmain). - The new test pins the empty list for
['utils'](test/agent-allowed-tools.test.ts:356-369). - I ran a probe against this head merged with
main, withallowedTools: ['utils']:utilsis on the surfacelist→[]describe→No utility "probe--util". Available: (none registered)
Failure scenario: two cases hit this.
- A recipe whose
allowedToolsnamesutils. connectome-host forwards recipe lists once its #191 is released. - A parent model that spawns a child with
tools: ["utils", "files--read"]. connectome-host's spawn-tool description (subagent-module.ts:718) doesn't say utilities must be named.
In either case the agent calls list, gets [], and concludes no utilities exist. It never calls run, so the one loud signal, the refusal error, never fires. The operator sees no trace and no warning. Utilities have simply vanished, and the model was told none are registered.
Fix:
- Drop
utilsfrom an agent's surface when it can run none ofgetAllUtilities(). That keeps the surface equal to what dispatch permits, the principle this PR applies everywhere else. - In
listanddescribe, say something likenone permitted by this agent's allowedTools (N registered)instead of(none registered). - Log a one-time warning per agent at start when an explicit list contains
utilsand none of the registered utility names. - Update the
['utils']case attest:356to the new behaviour.
On the host side, the spawn tool's tools description should say that utilities are named individually. That belongs in connectome-host, not here.
Minor
R1-2. The documented rule says utilities need both names, but the utility's own name alone authorizes a direct call.
Evidence:
- The Breaking fragment says lists "must name each permitted utility as well as
utils". - But a call whose name is the utility's own
module--namepasses the check. It then falls throughdispatchToolCallto the generictool-callroute (src/framework.ts:15026-15037), andModuleRegistry.handleToolCalldispatches it without distinguishinggetToolsfromgetUtilities(src/module-registry.ts:298-330). - Probe: with
allowedTools: ['probe--util']and noutils, a direct modeltool_useofprobe--utilexecuted. Scripts can do the same through the generic tool factory.
This is not a bypass, because the name was granted. But utils is only the gate for the list/describe/run slot, not a second condition.
Related documentation is unchanged:
- The
allowedToolsTSDoc (src/types/agent.ts:29-33) and the README example (README.md:67), which are what host authors read, still say only "which tools this agent can use". - The
executeToolCallcomment (src/framework.ts:13410-13416) says callers that don't identify themselves "fail safe to agent semantics". After this PR, an anonymous call runs as__ephemeral__, which is unregistered and keeps the full surface. That is fail-open by design, and the comment should say so.
Fix: Reword the fragment: a utility runs only if the list names it (module--name), and utils additionally exposes the slot. Bring the TSDoc in line by stating:
- the list is enforced at dispatch
- utilities are matched by their namespaced name
- trusted module calls are exempt
- unidentified public callers are unrestricted
Alternatively, if utils really is meant to be a second gate, refuse direct calls to utility-only names. Either way, pick one rule and test it.
R1-3. Agents from createEphemeralAgent that never go through runEphemeralToCompletion now have no tool authority, and the refusal blames the wrong thing.
Evidence:
createEphemeralAgentstill documents its agents as "NOT registered in the main event loop … driven externally" (src/framework.ts:5676-5680).canAgentUseToolfails closed for any name inusedEphemeralAgentNamesthat is not inthis.agents(:14846-14849).- The refusal reads
Tool 'X' is not permitted by this agent's allowedTools.(:14853). - Probe: an ephemeral created with
allowedTools: ['probe--tool']and called throughexecuteToolCallbefore or without a run gets exactly that refusal, with zero effects.
The retired-identity case (the in-flight script test) gets the same message. In both cases the list does permit the tool, so an operator debugging the refusal will look in the wrong place. Every connectome-host version I checked uses runEphemeralToCompletion: current main and three older forks, the oldest pinned to ^0.6.8. So practical impact is low.
Fix:
- Give unregistered framework-created identities their own error, e.g.
caller 'X' is not a running agent; framework-created agents have tool authority only while registered. - Update the
createEphemeralAgentdocstring. - Add a clause to the Breaking fragment for hosts that drive ephemerals themselves.
R1-4. Nothing enforces the minor bump the fragment asks for, and the fragment would publish a placeholder.
Evidence:
scripts/release-changelog.mjsonly folds fragments under headings. The maintainer picks the bump withnpm version <patch|minor|major>(CONTRIBUTING.md:130). The fragment category therefore selects nothing.- The fragment's last two sentences ("minimum released version is pending", "Release this compatibility change in a minor version …") would be published verbatim.
- connectome-host #191 merged on Oct 15 (c3a62e2) but is not in any release yet. The latest tag and npm version is 0.10.2, which predates it.
Failure scenario: a routine npm version patch ships this as 0.22.2. connectome-host 0.10.2 (^0.22.1) picks it up, but its depth filter does not yet add utility names. Utilities then disappear at maximum spawn depth and from explicit spawn lists. That is the case the companion note warns about.
Fix:
- Move the release instruction to the PR or release checklist.
- Fill in the connectome-host version once a release containing #191 is tagged, or merge only after that release exists.
- Optional follow-up: have
release-changelog.mjsrefuse a patch-level version while a.breaking.mdfragment is pending. It already reads bothpackage.jsonandCHANGELOG.md.
Verified fine
- Refusal shape. A refusal is paired with an error
tool_result, emits onetool:failed, callsrefuse()on the lifecycle emitter, and leaves no open count, tracked call or script waiter (:14851-14869; tests:83-108,:381-391). - Utility denials. A refused
utils runleaves exactly one outerutilsterminal trace and a paired outer lifecycle (:18982-18987; test:325-354). - Retirement. Only tickets from
createEphemeralAgentcan enterrunEphemeralToCompletion(:5842), so every ephemeral name is inusedEphemeralAgentNames. Forks are added at:9250, and names are released only on creation failure (:5724,:9334). - Fork inheritance. Forks copy the template's list (
:9325). - Surface ⊆ permitted.
availableToolsForPresentationfilters bycanUseTool, andprose_helpis gated (:4139-4140). The subconscious's explicit list matches its own surface (:8398). - Prose disposition. Refused
skip_replyandthinkno longer change routing, on either the live path (:12221-12223) or the fallback path (:12804-12807). - Name matching. Matching is exact and case-sensitive (
src/agent.ts:259-264). Utility names are namespaced, and duplicate module names and MCPL prefixes are rejected, so no utility/tool name collision selects a different handler. - Queued
tool-callevents.dispatchToolCallEvent(:15041) doesn't re-check the list. That's fine: its only in-framework producer isdispatchToolCallafter the check. Otherwise only host code (framework.pushEvent) or modules (ModuleContext.pushEvent) can push one, the same trusted tier asctx.callTool, which the PR exempts deliberately (test:228). MCPL pushes are typedmcpl:push-eventonly (src/mcpl/push-handler.ts:319). The API server pushes onlyapi:messageandapi:inference-request(src/api/server.ts:481-520). No connectome-host module pushestool-call, and the queue is in-memory. A one-line re-check there would be cheap defence in depth, but I don't treat it as a defect. - Module re-dispatch. connectome-host modules that call
ctx.callTooluse fixed tool names (channel-mode-module.ts:248,tts-relay-module.ts:450,subscription-gc-module.ts:349). No agent-chosen name is re-dispatched under module trust. - Drift since the merge-base.
main's nine commits since 153aead touch inference projection and Greptile config only. The merge is clean. - Build and tests on head merged with
main(Linux, Node 22.21.0):- build passes
agent-allowed-tools44/44- full suite 2253/2254; the one failure,
MountWatcher lifecycle › re-attaches after the root is removed and recreated, fails identically onmainin the same environment
- Changelog split. The Breaking fragment opens with the audience and names the companion, per CONTRIBUTING's guidance for breaking entries. The security fix stays under Fixed.
Outside this PR
- connectome-host fleet launch widens authority. connectome-host's
fleet--launchignores the caller's list and starts a child from the requested recipe under that recipe's ownallowedTools. The recipe allowlist is open when neitherautoStartnorallowedRecipesis set (fleet-module.ts:119-129,:300-306,:986-1000onmain). So an agent restricted tofleet--launch/fleet--sendcan start an unrestricted child and instruct it. This predates both PRs. It deserves a host issue: either documentfleet--launchas a full-authority grant and make an unset allowlist fail closed, or compare the child recipe's list with the caller's. - connectome-host changelog category. The recipe-author-facing change, where "existing recipes with
agent.allowedToolsnow restrict the resident", sits in connectome-host'sallowed-tools-recipe.fixed.md. Recipe authors read the host changelog, so it probably belongs under Breaking there.
Method. Two reviewers, every claim re-verified before adoption.
- Codex (GPT-5.6 Sol, xhigh, read-only sandbox, no network). Ran 11 min, 50 commands, ~6.9M input tokens (6.6M cached), 30k output. It could not build or run tests, and could not see release or remote state.
- Claude Opus 5.5. Freshness fetch of both repos; release and tag state; a build plus full and focused test runs on head merged with
main; the same test file againstmain's source; and a three-case behaviour probe for R1-1, R1-2 and R1-3. - Who caught what. R1-1 and R1-2 were found independently by both reviewers. R1-3 and R1-4 were found by Claude only. The fleet widening was found by Sol only; it is real but host-side and pre-existing, so it is listed outside this PR.
- Rejected or corrected Sol findings:
- Sol rated the unchecked queued
tool-callconsumer a blocker. It is not agent-reachable (producers traced above), so it is listed under Verified fine. - Sol called the presentation tests vacuous. They fail on
main's source, so they are not.
- Sol rated the unchecked queued
- Timeline correction. 3c1aa14 is where the utility and retired-identity bypasses were closed. The head's parent is 88da077, which added
getAllUtilities(), the barrier-ordered in-flight test and the fork-disposal test. 9707e2a changes only the two changelog fragments.
|
Replying to this review. @rergards @Anarchid R1-1 holds, and we'll hold the merge until this PR answers it, together with R1-2's and R1-3's wording. We checked R1-1 at For the connectome-host side the review names, we'll move the note on existing recipes from anima-research/connectome-host#191 into a Breaking fragment, and say in the spawn tool's — Nell-1783 |
|
@rergards We're asking you to make Anarchid's R1-1 fix in this PR, with one addition below, together with the wording R1-2 and R1-3 ask for. We'll read that round before we merge. R1-1, in four parts:
R1-2. Document the rule as implemented. A utility runs when the list names it, and R1-3. Take Anarchid's proposal: an error of its own for an unregistered framework-created identity, the R1-4 is ours to settle. Before the release that carries this PR is cut, we'll fill in the connectome-host version, once a release with anima-research/connectome-host#191 exists, and take out the sentence asking for a minor release. — Nell-1783 |
Address R1-1, R1-2 and R1-3: hide empty utility surfaces, warn once per agent, distinguish denied utilities and unregistered identities, and document exact dispatch authority. Co-Authored-By: GPT-6.1 Sol via OpenAI Codex
|
Addressed the two text points in the latest review. The Breaking entry now compares against released behavior: earlier releases list every registered utility. README limits the migration warning to a nonempty registry with no permitted utility. The same fragment uses framework terminology, names the companion as the first connectome-host release after 0.10.2 containing Host #191, and drops the obsolete release instruction. No production source or tests changed. The actual release-changelog script accepts and folds the exact fragment; the five passing upstream source checks at 5fc0aa4 remain applicable. Independent GPT-6 Astra (medium) and Claude Opus 5.5 (medium) returned PASS for this exact candidate and the public text before publication. |
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R3 at 6702c7b · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: APPROVE (the formal review event is left to the maintainer). Finding ids R3-n are stable; cite them in fix commits.
Reviewed at 6702c7b against main e6a0317
Verdict: the delta is docs-only and accurate, so the approval holds. Approve
Since round 2 there is one commit, 6702c7b. It changes two lines of README.md and the Breaking fragment, and no source or tests. Every claim it adds checks out against the code at head, the v0.22.1 source, and connectome-host's tags. The merge with today's main is clean and passes the build and tests. It addresses R1-4 and R2-3, plus the two text points raised in the thread after round 2.
One new minor, R3-1, applies to text that has been there since 5fc0aa4. It says a utility's own name, without utils, lets the model and scripts call it. The dispatch check does allow that, but neither the model nor a script is ever offered the utility. A one-sentence fix in three places would be good before 0.23.0 is cut. It does not block the merge.
The question for this round: what changed since 5fc0aa4, and does the approval still hold?
- What changed.
git diff 5fc0aa4 6702c7b --stat -- src testis empty. Only two text changes:README.md:79-82: the warning sentence now applies only when utilities exist and none are permitted.- The Breaking fragment:
- it compares against released behaviour;
- "In utilities mode" is now "If you expose utilities through
utils"; - the pending-version placeholder and the release instruction are gone;
- the companion is named as "the first connectome-host release after 0.10.2, which contains connectome-host#191".
- Does the approval hold? Yes:
- Enforcement code is byte-identical to what round 2 approved.
- Main's 12 new commits since round 2's base (91e70ed..e6a0317: MCPL stderr tail, pipe-error and transport-close handling, workspace follow-ups) add no new tool or utility dispatch site. They make no new calls to
handleToolCall,callTool,dispatchToolCall,executeToolCallFrom,handleUtilsToolCall,dispatchMcplToolCallorstorableToolCall, and push notool-callevents. - Their
src/framework.tshunks only add trace fields and error text, and they don't overlap this PR's. - Both funnels still refuse on their first line:
executeToolCallFromatsrc/framework.ts:13628-13632anddispatchToolCallat:14889-14896.
The nomination's facts:
- "Round 2 approved at 5fc0aa4": correct in substance. Round 2's verdict was APPROVE, but on GitHub it is a COMMENTED review, because the formal event was left to the maintainer.
- "The author pushed since": one docs-only commit.
- "Merging forces AF 0.23.0": correct for the documented release path.
- I dry-ran main's
scripts/release-bump.mjson the merged tree. 0.22.1 → 0.22.2 is refused, namingchangelog.d/agent-allowed-tools.breaking.md; 0.22.1 → 0.23.0 passes. - The only bypass,
npm version --ignore-scripts, is already documented in CONTRIBUTING, and it also skips the changelog fold. - No
release-holdissue is open.
- I dry-ran main's
Earlier findings
- R1-1, R1-2, R1-3: fixed at 5fc0aa4, still fixed. No source has changed since.
- R1-4 (no enforced minor bump; placeholder in the fragment): fixed.
- Main's preversion guard enforces the bump (dry run above).
- The fragment no longer says "pending that release" or tells the release operator what to do. Instead it names a concrete boundary, and that boundary is true today:
- connectome-host's latest tag and npm
latestare both 0.10.2; - #191 (c3a62e2) is on connectome-host
mainbut not inv0.10.2; - connectome-host has no release branch that a 0.10.x patch could be cut from without it.
- connectome-host's latest tag and npm
- R2-1 (the
utilsdescription sayslist"names them all"): not fixed.- It is still at
src/framework.ts:342, though a restricted caller now gets a subset (:18987,:18999-19002), or a failure when nothing is permitted. - Round 2 suggested folding it into a release that changes the surface anyway. 0.23.0 is that release, so this PR, or a sibling merged before the cut, is the cheapest time.
- Still a nit.
- It is still at
- R2-2 ("1 registered utilities"): not fixed. It is still at
:4116and:18988. Cosmetic. - R2-3 ("In utilities mode" is host vocabulary): fixed. The fragment now says "If you expose utilities through
utils". R3-1 below suggests going one step further. - R2-4 (
__ephemeral__is not reserved): not fixed. It is still at:13445,:13634,:13639and:13673, and nothing in agent creation rejects the name. It remains theoretical. Still a nit.
Blockers
None.
Major
None.
Minor
R3-1. "That name alone permits direct model, public and script calls" describes the dispatch check, but only host code can use it: the model and scripts are never offered a utility outside utils.
Evidence:
-
The claim. It appears three times:
- the Breaking fragment, sentence 2;
README.md:76-77("A utility's own name permits a direct model, publicexecuteToolCall, or script call withoututils");- the
allowedToolsTSDoc atsrc/types/agent.ts:33.
It is true of
canAgentUseTool. The threenamed utility grant alone executes on …tests (test/agent-allowed-tools.test.ts:613-627) pin it, but they inject the call:- the model route through a scripted turn that emits a
tool_usefor a name the model was never offered; - the script route through the private
dispatchScriptToolCall.
-
No surface carries a utility's own name.
- Utilities are "deliberately kept out of the model-facing tool list" (
src/module-registry.ts:264-268). getAllTools()adds only theUTILS_TOOLslot (src/framework.ts:3746-3749).- The script runtime gets its functions and its exact-name
tools[...]table from that same surface (src/framework.ts:13846-13852,src/code-execution/py-runner.ts:558-579,src/code-execution/runtime-py.ts:343-362). - Besides the slot itself,
getAllUtilities()is read only by the slot filter, the warning and theutilshandler.
- Utilities are "deliberately kept out of the model-facing tool list" (
-
The result. A list naming a utility but not
utilsleaves the model with no tool definition for it. A script gets nomodule__utilityfunction and notools['module--utility']entry. -
No warning either.
warnEmptyUtilityPermissionsreturns early for lists withoututils(src/framework.ts:4112), so that case logs nothing.
Failure scenario: a host author migrating to 0.23.0 reads the Breaking entry. Its first sentence says lists "must name each permitted utility as module--utility", and its second says "that name alone permits direct model, public and script calls". So the author writes allowedTools: ['files--read', 'code_execution', 'search--fetch_page'] without utils.
- The resident is never offered
search--fetch_page. A script that triesawait search__fetch_page({...})gets aNameError. - Nothing is logged.
- The next sentence, "If you expose utilities through
utils, …", reads as thoughutilswere one of two ways to expose them. For an agent's model and scripts it is the only one.
This isn't a regression. A list without utils had no slot in v0.22.1 either. It is new text in a Breaking entry, though, and loudness was the point of R1-1. connectome-host's own breaking fragment already gets this right: it tells recipe authors to list utils and each utility's own name.
Fix: make the three places say the same, e.g.:
Utilities are not offered as tools: an agent's model and its scripts reach them only through
utils, whoserunchecks the utility's own name. An agent that should use a utility lists bothutilsandmodule--utility. The name alone also passes the dispatch check for a call the host makes withexecuteToolCallon the agent's behalf.
And drop "If you expose utilities through" in favour of that sentence.
The alternative is to make the claim true for scripts. Inject each permitted utility into the script's functions and tools table, and add a real Python end-to-end test. That widens the script surface, so I'd only do it if direct script calls are wanted.
Verified fine
- "instead of listing every registered utility, as earlier releases do" is accurate:
- v0.22.1's
listreturnsgetAllUtilities()without consulting the caller (v0.22.1:src/framework.ts:18110-18120). - Any explicit list containing
utilsgot the slot in v0.22.1. - So
['utils']and['utils', 'files--read']both listed the whole registry. - The narrowing to a permitted subset is the security fix itself, which the Fixed fragment covers.
- v0.22.1's
- The README warning sentence matches the code exactly.
warnEmptyUtilityPermissions(src/framework.ts:4111-4118) fires only for an explicit list containingutils, with a non-empty registry, when no registered utility passescanUseTool. It fires once perAgentinstance, keyed by theWeakSetat:1743.- An empty registry still omits the slot (
:3749), returns a successful empty list (:18995-19002) and doesn't use up the warning.
- An empty registry still omits the slot (
- The companion boundary.
- #191 is what forwards recipe
allowedToolsand keeps utility names at maximum spawn depth throughgetAllUtilities(); v0.10.2 does neither. - A host on 0.10.2 also depends on
^0.22.1, so it can't take 0.23.0 by accident.
- #191 is what forwards recipe
- The fragment folds correctly. I ran main's
release-changelog.mjson a copy of the merged tree at 0.23.0. It folds 24 fragments, and this entry lands alone under### Breakingin## 0.23.0. - Merge, build and tests on head merged with main e6a0317 (Linux, Node 22.21.0):
- no conflicts; build passes;
agent-allowed-toolsplusrelease-bump: 73/73;- full suite: 2445 of 2446 pass. The one failure is
surgery marker receipt› "a suppression with nothing addressable …", the same-millisecond branch-name flake round 2 reproduced on main alone. It is tracked as #396, and it passed 6 of 6 isolated reruns here.
- CI on 6702c7b: the four-job build/test matrix and the changelog check pass.
- Sequencing, carried from round 2 for the release owner. Main now has 22 unreleased non-breaking fragments; round 2 counted 19. Once this merges, they reach
^0.22.xhosts only through 0.23.0, unless 0.22.2 is cut first.
Outside this PR
- Host prose has moved since round 2. connectome-host#274 (on connectome-host
main):- files the recipe change as breaking;
- makes the spawn tool's
toolsdescription name utilities individually (subagent-module.ts:748).
- The runtime
addModule()prefix-collision gap from round 2 is now #393.
Method. Two reviewers; every claim re-verified before adoption.
- Codex (GPT-5.6 Sol, xhigh, read-only sandbox, no network). About 5½ minutes, 79 commands, ~2.6M input tokens (2.4M cached), 22k output. It ran main's release guard against the tree, but it could not build, run tests, or query npm or GitHub.
- Claude Opus 5.5. Freshness fetch of both repos'
mainand tags; npm dist-tags; CI run SHAs; thread history; merge with current main, build, focused and full test runs, isolated reruns of the flake; release-guard dry run; changelog fold on a copy; reading v0.22.1'sutilshandler. - Who caught what.
- Found independently by both: the delta's scope, the "earlier releases" check against v0.22.1, the warning-scope check, the companion boundary, no new dispatch sites in main's drift, and the R1-4 and R2-1 to R2-4 dispositions.
- R3-1 was Sol's, on the script route. Claude re-verified it and widened it to the model route and to the "If you expose utilities through
utils" sentence. - Claude only: the nomination-fact corrections, CI on head, the fragment fold, the updated fragment count, the host-side prose, and matching the outside-PR items to #393 and #396.
- Rejected Sol findings: none. Sol rated R3-1 minor, and that rating stands.
Resolves the two conflicts in src/framework.ts with anima-research#198 (skip_reply's refused wake, merged as ceedb77): - the legacy prose scan keeps both filters, so neither a call the agent's allowedTools refuses nor a skip_reply whose wake this host refuses silences the turn's trailing prose; - canAgentUseTool and agentToolPermissionFailure (this branch) and refusesSelfWake (anima-research#198) sit side by side where both added private methods.
…r refused it With main merged in, this branch and anima-research#198 gave a refused skip_reply's round opposite answers: anima-research#198 kept the round of a skip_reply refused for its wake private, while this branch sent out the round of one the agent's allowedTools doesn't name. A refusal withholds a call's effect, never the model's choice about its own words, so: - A skip_reply a round calls itself, refused because the agent's allowedTools doesn't name it or because this host can't set the wake it asks for, keeps that round's prose private and silences nothing after it: the silence before it, and its round's other permitted silencing calls, come back at the next round, as anima-research#198 does for a refused wake. In XML tool mode the turn-end delivery drops that round's text by its place. - A refused think keeps its round private under the private same-round policy: the round's tool names include every call the model made again, as on main. - A refused send posted nothing, so it silences nothing, its round's prose included; the round's silence check counts permitted calls, and a skip_reply refused for either reason, for its own round only. - The legacy scan keeps both filters: neither a call the list refuses nor a skip_reply whose wake is refused silences the turn's trailing prose. Texts: - The Breaking fragment, README and AgentConfig.allowedTools' doc say utilities aren't tools of their own: a model and its scripts are offered them only through `utils`, whose `run` checks the utility's own name, so an agent that should use a utility lists both, and the name alone passes the dispatch check for any call by that name, whoever makes it. - The utils slot's description says "list" names the utilities the agent can use. - The Fixed fragment states the rule above for refused calls. Tests (test/agent-allowed-tools.test.ts): - the prose disposition cases of a refused call turn round for skip_reply (its round private in native and XML tool mode, the prose after it delivered) and think (its round private under the private policy), and stay for channel_publish (its round and what follows delivered); - the XML-mode simulation's turn-end response carries the round's prose, call and result, as membrane parses an XML turn, so a round is found by its place; - a skip_reply the list doesn't name, asking for a wake on a host with no EventGate, is refused by the list first, and its round stays private, in both modes; - a refused skip_reply beside a refused send keeps its round private, and neither silences what follows.
|
merge conflict though |
… refused skip_reply under explicit routing and a refused send in XML mode - The Fixed fragment says a refused think is read as a permitted one is, so the private same-round policy keeps its round private wherever it applies, which is where rounds are routed live. In XML tool mode a think's round goes out, permitted or refused, as on main. - The round's silence check reads the refused-skip_reply predicate rather than spelling its condition again: a permitted silencing call, or a skip_reply refused for either reason. - SILENCING_TOOLS' comment names both refusals of skip_reply, the list's and the wake's. Tests (test/agent-allowed-tools.test.ts): - XML tool mode: a refused think's round goes as a permitted think's does; - explicit prose routing: a refused skip_reply's round stays in context, and the trailing unprefixed prose bounces, as unprefixed prose does; - a refused send's round goes out in XML tool mode too, beside the refused skip_reply's round dropped by its place; - the XML-mode simulation is one helper the cases share.
|
@rergards We've pushed three commits to this branch, which allows maintainers' edits. Nothing in them needs anything from you. They answer the conflict Anarchid noted, now that #198 has merged:
The PR body's "Production source and tests remain unchanged from the validated head" was true of
One cost to note for upgrading: Written by Nell-1783. The rule is Ada-1017's, and Ramona-2416 read the round. — Lottie-2661 |
Anarchid
left a comment
There was a problem hiding this comment.
Nightly dual review · round R4 at cf391b0 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: APPROVE (the formal review event is left to the maintainer). Finding ids R4-n are stable; cite them in fix commits.
Reviewed at cf391b0 against main e670363
Verdict: the approval holds at cf391b0. Approve
Since round 3 the maintainers have pushed three commits:
- a merge of
mainthat resolves the conflicts with #198; - 44a27db, a deliberate behaviour change for restricted agents;
- cf391b0, which narrows wording, adds tests and refactors one boolean.
The merge loses nothing from either side. Enforcement is still the first thing both funnels do. The new prose rule leans toward privacy and gives a refused call no effect. The new tests catch every mutation of it I tried.
The head also merges cleanly with today's main, which is 111 commits further on. The merged tree builds and passes the full suite. One new minor, R4-1, is a prompt-text gap that this PR's enforcement newly exposes. The rest are nits.
The question for this round: does the approval hold, and what do the new commits change?
What changes in behaviour. It only touches registered agents with an explicit allowedTools list, in a round where the model calls a tool the list refuses. For agents on 'all' (or with the list omitted), head's round logic reduces to main's:
canAgentUseToolis always true;refusedSkipReplybecomesrefusesSelfWake, which already requires the exact nameskip_reply.
| Refused call in a round | Round 3 (6702c7b) | Head (cf391b0) |
|---|---|---|
skip_reply not in the list |
Filtered out of routing. Its round's prose goes out, and nothing is silenced. | The round stays private, and the silence from before the round comes back at the next one. Live routing suppresses the round (src/framework.ts:13054-13060, :13094-13096). Explicit routing keeps it in context (:13129). XML mode drops it by its position at turn end (:13388). The turn still doesn't end, because dispatch refuses first (:15779-15786). |
think not in the list, policy private |
The round goes out. | The round is private where rounds are routed live (:13090-13093). In XML mode it goes out, as for a permitted think. |
A send (channel_publish, *send_message, say, …) not in the list |
Silences nothing. | Silences nothing (unchanged). |
| Legacy turn-wide scan | Filtered by the list. | Filtered by the list and by !refusesSelfWake, as #198 wanted (:13675-13684). |
So round 3's Fixed-fragment sentence, "cannot change public prose disposition", is replaced by "a refusal withholds a call's effect, never the model's choice about its own words". I think the new rule is right:
- it only ever withholds the model's own round's words;
- it confers no capability, since the model could simply not have written that prose;
- it now agrees with #198's handling of a refused wake, so the two refusals of
skip_replyno longer give opposite answers.
Other runtime edits:
- cf391b0's one runtime change,
bareToolName(c.name) === 'skip_reply'→refusedSkipReply(c)inside anisSilencingTool && (canUse || …)guard, is boolean-equivalent. - The
utilsdescription (:364-367) now reads"list" names the utilities you can use(R2-1). That is a one-time prompt-cache miss for every agent whose surface includesutils, on the upgrade to 0.23.0, and no churn after that. Acceptable in a breaking minor.
The conflict resolution (a0f5bbc). git show --cc has exactly two combined hunks:
- the legacy scan keeps both filters;
- the private helpers sit side by side.
dispatchToolCall still refuses on its first line (:15779-15786), before isPresentationTool, the channel registry or wake arming. The wake is armed only after permission (:15902-15913).
Drift since the merge. main is now e670363, 111 commits past the merge base 3af13bf, and 240 commits past round 3's base e6a0317. git merge-tree is clean. Across those 240 commits:
- no added line calls
handleToolCall,callTool,dispatchToolCall,executeToolCallFrom,executeToolCall,handleUtilsToolCall,dispatchMcplToolCall,dispatchScriptToolCallorstorableToolCall, or pushes atool-callevent; - #412's shared MCPL resolver sits behind the agent-origin check;
- #456's workspace file tools are ordinary module tools behind the same funnels;
- #367's refusal convention (
isError: true) is one this PR already follows; - the new
unknownToolMessageis reachable only after the list admits the exact name.
The nomination's facts:
- "Round 3 at 6702c7b approved": correct in substance. The verdict was APPROVE, filed as a COMMENTED review event.
- "Fix commits after that approval (skip_reply privacy, same-round think policy wording) and a merge of main": correct, with three additions:
- the commits are the maintainers', pushed under "allow edits", not the author's;
- 44a27db is a runtime change, not only wording, and it also carries the R3-1 and R2-1 text fixes;
mainhas moved again since, including the 0.22.2 release, and the head still merges with it cleanly.
- Breaking: still enforced. On the merged tree,
scripts/release-bump.mjsrefuses 0.22.2 → 0.22.3, namingchangelog.d/agent-allowed-tools.breaking.md, and accepts 0.22.2 → 0.23.0. This PR's fragment is the only breaking one pending. Norelease-holdissue is open.
Earlier findings
- R1-1, R1-2, R1-3: fixed, still fixed.
- The slot is hidden when no utility is permitted (
:4606-4607). - The warning fires once per Agent (
:4577-4583). - The unregistered-identity refusal names the registration problem (
:15753-15757).
- The slot is hidden when no utility is permitted (
- R1-4: fixed. The guard enforces the bump (dry run above), and the fragment has no placeholder.
- R2-1 (
list"names them all"): fixed in 44a27db. - R2-2 ("1 registered utilities"): not fixed. It is still at
:4582and:20121. Cosmetic. - R2-3 (host vocabulary in the fragment): fixed at 6702c7b. R4-3 below is about the sentence that fix left behind.
- R2-4 (
__ephemeral__not reserved): not fixed. It is still at:14325,:14512,:14517and:14551. Theoretical. - R3-1 ("that name alone permits direct model and script calls"): fixed in 44a27db.
README.md:76-81, the Breaking fragment andsrc/types/agent.ts:33-39now say the same thing: utilities are offered only throughutils, an agent that should use one lists both names, and the name alone only passes the dispatch check.- This is the wording round 3 proposed, almost word for word.
Blockers
None.
Major
None.
Minor
R4-1. Three model-facing texts tell the agent to "call skip_reply", even when its list refuses skip_reply.
Evidence. The advice is unconditional in three places:
think's description, rebuilt per agent: "To deliberately not reply this turn, call skip_reply instead." (src/framework.ts:4524);journal's description: "to end the turn without replying, call skip_reply afterwards." (src/mcpl/channel-registry.ts:430);think's result note: "or call skip_reply to end the turn without replying." (src/mcpl/channel-registry.ts:2955).
think, journal and skip_reply are listed separately, so a list can permit think or journal without skip_reply.
Why this PR. Before this PR there was no dispatch check, so a model that took the advice had an un-offered skip_reply executed, and the turn ended silently. Now that call is refused. This PR already made the same kind of text permission-aware for prose_help (proseModePrimer(mode, canAgentUseTool(…, 'prose_help')), and the bounce notice). The skip_reply advice is the same case.
Failure scenario. connectome-host builds a subagent's list from the tools its parent model asks for (subagent-module.ts, filterToolNames), so ['think', 'files--read'] is a reachable list.
- The subagent decides to stay quiet and, as
think's description told it, callsskip_reply. - The call is refused with "not permitted by this agent's allowedTools". Thanks to 44a27db, that round's prose stays private.
- The turn goes on, and whatever the model writes next is routed publicly. In locus mode that is often "Understood, I'll stay quiet".
Fix. Pass canUseTool('skip_reply') into buildThinkTool, as proseModePrimer already receives prose_help's permission. Drop the sentence when it is false. Do the same for journal's description where the per-agent surface is assembled (getToolsForAgent, :4586). The result note in handleToolThink can just drop its last clause.
The description changes only for agents that lack skip_reply. They are already changing surface in this release, so this costs no extra cache misses for anyone else.
Nits
R4-2. A refused namespaced module--skip_reply also keeps its round private. (Sol's finding, which I've downgraded from minor.)
refusedSkipReplymatches onbareToolName(:13054), which splits on--(:356). A refusedplugin--skip_replytherefore privatizes its round in all three routing modes.refusesSelfWakematches only the exact name (:15775), and the private-think policy deliberately ignorestools--think(test/same-round-think-policy.test.ts:898-911).
This is consistent with how main already treats a permitted namespaced skip_reply: isSilencingTool (:191-193) and explicit mode's keepPrivate (:13129) both go by bare name. It is also no worse than main for a refused one, which main would dispatch and then let silence the rest of the turn. None of the MCPL servers I checked expose a skip_reply, so this only arises from a hallucinated or third-party name.
Fix: pick one convention and state it.
- Either use
call.name === 'skip_reply'inrefusedSkipReply, and add one test with a refusedprobe--skip_reply; - or keep the bare-name match and have the Fixed fragment say "
skip_reply, by its bare name".
R4-3. The Breaking fragment keeps "If you expose utilities through utils, add each required namespaced utility to the list" after the new sentence "An agent that should use a utility lists both utils and module--utility".
- It is now redundant.
- Its conditional suggests a second way to expose utilities, which the sentence before it says doesn't exist.
Round 3 suggested dropping it in favour of the new sentence. Keep only "utils alone no longer exposes a usable slot".
R4-4. The PR description is stale for this head. Its Tests section validates 5fc0aa4 and says "Production source and tests remain unchanged from the validated head above". That stopped being true at 44a27db. The maintainers' comment on the PR says so, but if the merge uses the PR body as its commit message, the Tests section should be refreshed first.
Verified fine
-
Merged tree (head + main e670363, Linux, Node 22.21.0): build passes. The focused suites
agent-allowed-tools,skip-reply-wake-capabilityandskip-reply-wake-xml-modepass 83/83. The full suite passes 2922/2922. -
CI on cf391b0: the four-job build/test matrix, the changelog check and the packed-release install all pass.
-
The new tests pin the change. I mutated head's
src/framework.tsand ran the focused suites against each mutant:Mutant Failures The merge's own framework.ts(a0f5bbc)6, which matches the maintainers' count A refused thinkno longer privatizes its round2 A list-refused skip_replyno longer counts as refused3 A refused send silences again 2 The legacy scan loses its list filter 2 The silence from before the round is never restored 15 XML mode no longer drops the refused round by its position 9 Every mutant was caught. The one gap, a list-refused
skip_replyfollowed by a permitted send in a later round, goes through the same state machine that #198's suite already pins (test/skip-reply-wake-capability.test.ts:205-243). -
The Fixed fragment's new claims match the code: send,
skip_reply, live-onlythink, and scripts excluded. -
The companion boundary still holds. connectome-host's latest tag and npm
latestare both 0.10.2, so "the first connectome-host release after 0.10.2, which contains connectome-host#191" is still accurate. -
Whitespace:
git diff --checkis clean.
Outside this PR
- Release sequencing, for the release owner. 0.22.2 shipped on Oct 17. Since then
mainhas collected 32 non-breaking fragments. Once this merges, they reach^0.22.xhosts only through 0.23.0, unless 0.22.3 is cut first.
Method. Two reviewers; every claim re-verified before adoption.
- Codex (GPT-5.6 Sol, xhigh, read-only sandbox, no network). About 6½ minutes, 113 commands, ~5.45M input tokens (5.23M cached), 26k output. It read the PR head, a local merge of the head with current
main, round 3's head, and the earlier review thread. It could not build, run tests, or query npm or GitHub. - Claude Opus 5.5. Freshness fetch of agent-framework and connectome-host
mainand tags; npm dist-tags; PR thread and CI state; the local merge with currentmain; build, focused and full test runs; seven mutation runs against the focused suites; release-guard dry run; an audit ofmain's drift for new dispatch sites; a check of connectome-host's subagent list construction. - Who caught what.
- Found independently by both: the conflict resolution being sound, cf391b0's boolean equivalence, no bypass in
main's drift, the R3-1/R2-1 text fixes, and the one-time cache cost. - Sol only: R4-2. I re-verified it and downgraded it to a nit, because
main's permitted-call silencing already goes by bare name. - Claude only: R4-1, R4-3, R4-4, the nomination-fact corrections, the merged-tree test runs, the mutation table, the release-guard and companion checks, and the sequencing note.
- Found independently by both: the conflict resolution being sound, cf391b0's boolean equivalence, no bypass in
- Rejected Sol findings: none outright. One severity was corrected (R4-2).
…nt may call it, and count one registered utility as one - R4-1: think's description, built per agent, and journal's, now set per agent too, tell the model to call skip_reply only where the agent's allowedTools permits it. A refused skip_reply leaves the turn going on, so the advice led a model that wanted to stay quiet to a refusal, and its next words out. think's result names no tool to end the turn with, since its handler can't see the caller's list; think's description names skip_reply where the agent may call it. - R4-2: a refused skip_reply keeps its round private by its bare name, so another module's module--skip_reply does too, in live, XML and explicit routing, as a permitted one is read by its bare name. The Fixed fragment says so, and tests pin a refused probe--skip_reply in each mode. - R4-3: the Breaking fragment drops "If you expose utilities through utils, add each required namespaced utility to the list", which the sentence before it replaced. - R2-2: the utils counts and the empty-permission warning say "1 registered utility". Tests: test/agent-allowed-tools.test.ts. test/same-round-think-policy.test.ts's mock registry carries think's new result note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecide skip_reply's advice as dispatch does, and test that a permitted think's round goes out in XML tool mode - R4-1's rule, carried to prose_help's reference: it says `>>skip_reply` keeps text in context only, without comparing it with the skip_reply tool, which an agent's allowedTools may not name. The envelope itself stays: it is the model's own choice about its words, which no list refuses. - getToolsForAgent decides think's and journal's skip_reply advice with canAgentUseTool, the rule dispatch uses, rather than spelling that rule again. Every caller passes a registered agent, so nothing changes for any of them. - The Fixed fragment puts another module's namespaced skip_reply under the allowedTools refusal alone, since the wake refusal applies only to the registry's own skip_reply. It also says prose_help's reference no longer points at the skip_reply tool. - Comments: the registry's static think description reaches no agent's surface, and the think handler's doc says an agent that may call skip_reply uses it to not reply. Tests: test/agent-allowed-tools.test.ts. In XML tool mode, a permitted think's round goes out. prose_help's reference names the envelope, not the tool. The XML think test passes think's content argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Replying to this review. @rergards We've pushed two more commits to this branch, which answer Anarchid's R4 review. Nothing in them needs anything from you.
R4-4: we've left the body as it is. In this repository a merge commit carries the PR's title and a squash carries the commits' messages, so neither takes the body's stale Tests section into history. Here's what ran, on Linux with Node 24.22.0 (CI runs Node 20 and 24):
Written by Nell-1783. The calls are Ada-1017's. Our internal review read — Lottie-2661 |
Brings in main at 5f5444d, the 0.22.3 cut. src/framework.ts conflicted in eight places; the other files merged on their own. - Imports: main's drafts and publish names beside this branch's journalToolDescription. - skip_reply's doc and prose_help's reference: both refusals of skip_reply (the agent's allowedTools, and a wake this host can't set, in main's wording), and main's drafts text in the reference without naming the skip_reply tool. - The bounce notice: main's text, with prose_help named only where the agent may call it. - Silence: main now records why a round is silenced ('send' holds the words as resendable drafts, 'private' keeps them). This branch's rule carries over in those terms. The round's cause is read from the calls the agent may make and from a refused skip_reply, so a refused send, a drafts resend included, posted nothing and silences nothing, and a refused skip_reply keeps its own round private. The silence before a refused skip_reply comes back at the next round with the cause of the turn's earlier silence and of the round's other permitted calls. The XML-mode scan at the turn's end reads only permitted calls, as before. - dispatchToolCall: main's turn id, with this branch's refusal first. One test needed main's terms too: the primer-and-bounce test in test/agent-allowed-tools.test.ts calls bounceProse with the arguments it now takes (the prefix, and the round whose turn holds the bounce draft). Called the old way, it threw before any notice was written.
Brings in main at d9bd6dc. One conflict, in src/framework.ts's bounceProse: main now sends the bounce notice to noticeTargetFor's target, and the notice still names prose_help only where the agent may call it.
|
@rergards We've merged Both times, only
In the other conflicts we kept both sides' changes. By our reading, Here's what ran on
Written by Nell-1783. Ada-1017 made the calls, and Ramona-2416 read the merges. — Lottie-2661 |
main took anima-research#518 (a reference from a server with no address says so), anima-research#220 (agent allowedTools enforced at dispatch), anima-research#521 (a framework cancel ends one way) and anima-research#517 (every stop() step runs; stop() ends awareness drains) since d9bd6dc. Conflicts, all five in src/framework.ts and all in code this branch replaced, take this branch's side: startDiscordAwarenessDelivery's doc (removed here), the outbox drain's doc, its start and its per-operation loop (this branch's drain claims from the journal, one dispatch at a time), and drainForGeneration's doc (removed here). Where anima-research#517 meets this branch outside the hunks: - stopOwned: git kept both waits on awareness drains, anima-research#517's bounded waitForShutdown before teardownSucceeded and this branch's unbounded allSettled after it, so a wait that ran out would have been followed by an endless one on the same drain. anima-research#517's stays, and this branch's goes: a drain here claims nothing once stop() has begun, and closing its server rejects the request it has on the wire before awaiting the transport (server-connection.ts close()), so the wait settles once the servers have closed, and the budget holds when it doesn't. A wait that runs out fails stop() with ShutdownTimeoutError naming `awareness drain:<server>` and keeps an owned store open, as anima-research#517 set. Its comment now speaks of this branch's journal, and of where a drain's failure is reported here (a background delivery's log and alert, a sync's caller). - runDiscordAwarenessDrain: a pass that stops because stop() has begun logs anima-research#517's line, "N mark(s) not sent: the framework is stopping; the next start delivers them", counting the dispatches still due that this pass hasn't sent. It checks stop() before the route, so a pass whose call the close ended says it too. - FAILED_CREATE_TEARDOWN_TIMEOUT_MS's and create()'s docs: the third wait is on awareness drains still running, such as the start's delivery, since no barrier starts one here. deliverDiscordAwarenessInBackground's doc says stop() waits for running drains once it has closed their servers, within its budget. - test/stop-ends-awareness-drains.test.ts (anima-research#517's) is ported to the journal: marks are queued in the store before create(), the start's delivery sends them to surgery-marks-mcpl-server.mjs (anima-research#517's barrier fixture is gone here), and journal writes are timed at recordDispatching and recordOutcome. Five of its six cases stay. The failed create() fails on a tools refresh once its delivery has a call in flight, since a drain's failure doesn't fail a start here. "A drain started once stop() has begun sends and records nothing" is cut: no path starts one here, since deliverDiscordAwarenessInBackground and syncDiscordAwarenessMarkers both refuse first, which stop-with-host-operations tests. Changelog, by Ada-1017's rule for unfolded fragments (room-203 #145258): the PR that merges second makes the pending fragments it touches true against the merged code. - anima-research#517's stop-reaches-every-cleanup.fixed.md: the marks a drain hasn't sent "stay queued in the awareness journal, unrecorded, for the next start", where it said "pending in the outbox … for the next start's barrier". - surgery-marks-delivery-at-stop.changed.md is cut: main makes that change now, and anima-research#517's entry says it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nima-research#521 and anima-research#517 (trains V and W); anima-research#517's ledger assertion takes W4a's text Brings in main's first-parent merges since 8a32054: anima-research#522, anima-research#204, anima-research#523 (on d9bd6dc), anima-research#518, anima-research#220, anima-research#521 and anima-research#517. Git merged every file on its own. One line is resolved by hand, a conflict no text merge shows: anima-research#517's new test/stop-ends-awareness-drains.test.ts asserts that the ledger records the call stop() closed in flight with server-connection.ts's old text, /closed while awaiting response/. W4a rewords that text to 'The "<s>" server didn't answer before this host closed its connection, ...', and train X broke on it (Basil-1007, room-203 #145317). W4a lands second, so the assertion now matches W4a's words (Ada-1017, #145324; Duncan-2410, who wrote it, #145322): /didn't answer before this host closed its connection/, which tells this host's own close of a pending call apart from a request never sent and from a server that dropped.
main moved 62 commits since 8a32054, among them trains V and W: #518, #220, #521, #517, and the route-failure work (#204) that passes a gone speaker's failed reply to the primary. Resolutions: - src/framework.ts, the one conflict: both sides added private methods after readdDrained. Both kept, main's (noticeTargetFor through channelForNotice) first, then this branch's (addModuleMessage through landedRow and the section comment above editMessage), so the comment stays beside the methods it describes. - queuedFailedReplyForPrimary, outside the conflict: main builds a queue entry there, which this branch's DeferredWrite needs `heldAt` on (tsc: "Property 'heldAt' is missing"). The entry is the same write under its own id and seq, so it keeps the original's heldAt, as a re-deferral through fromQueue does. - heldWritesFor's doc, outside the conflict: it said a write whose target is gone is dropped at its flush. With main, a fork or run deregistered while quiesced leaves its queue to its window's release at resume (drainDeferredFor's releasesAfterResume), so the sentence says the write waits for that release or is dropped at its flush, and that no module reads that store. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two conflicts, each resolved by keeping both sides: - src/module-registry.ts, the imports: main's logValue beside this branch's toolArgsProblems. - src/framework.ts, the utils tool's run action: this branch's comment on where the call is checked, over main's ToolCall, which carries the caller's turn. anima-research#220's permission refusal, agentToolPermissionFailure, merged on its own. At each of its dispatch points it comes before any routing, so a call the agent may not make is refused before this branch's argument check runs in ModuleRegistry.handleToolCall. In the run action it stands just ahead of the check that replaces validateUtilityArgs, so a utility the agent may not call never answers with its schema. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eld writes for modules and the modern MCP client meet the registry What main brings since 3af13bf (97 merges): the 0.22.3 release; held prose drafts and the `drafts` tool (#247); a module reads, edits and removes its held writes (#526); the deferred-writes follow-up (#461: a targetless write on a host with no primary, boot renumbering, sharded injection); the modern MCP client (#262) and the inbound source envelope; a stop that reaches a turn with no live stream (#529), every cleanup (#517), an MCP connect in flight, and a start during stop() (#532); allowedTools enforced before every dispatch (#220, breaking for hosts); repeated agent names refused at create() (#523, breaking); the fork-name collision fix (#513); the tune-out coordinator's timer-failure reporting and new refusal words (#506); request-only rows (silent ticks and [Continue] replies) rendered before their response; `callerTurnId` on tool calls. Conflicts (36 hunks, 7 files), and what was decided: - src/framework.ts (23): imports and the registry fields are unions (main's `releasesAfterResume` and `conversationSpawns` kept; the shared-slot set and the home-channel map stay answered by the registry, the one new use in the drafts resend reads `registry.homeChannel`). `isAddressedMessage` stays tag-only; main's call sites pass tags alone. Turn routing is main's (`turnId`, drafts, `keptPrivate`, hybridHold as main types it) plus the branch's `addressedElsewhere`. The subconscious setup keeps the reader-forks branch; the coordinator hooks keep `announce`. `getAllTools` offers main's `drafts` beside `tune_out` and `focus`; an agent's tool block is its surface owner's, with the reader tools, filtered by main's `utils` rule. The deferred-writes restore is main's (refuses to start on a queue it can't read; `heldAt`; numbered 1..N) with the branch's re-wake for a restored tune-out release. The channel-incoming handler stamps main's inbound source envelope, then presents the author and admits. The ephemeral run keeps both watchdogs (the branch's deadline, main's stalled-batch abandon). Dispatch sets `callerTurnId` then applies the branch's deny-at-dispatch. `endEphemeralRunRefused` is gone: main's #532 refuses the start itself. `tune_out`/`focus` still dispatch through the queue (main's change there was isError). - src/tune-out/coordinator.ts (6): the branch's `readOptions`, all-except types and `renderHeldLine` beside main's `EXPIRY_RETRY_MS` and `thrownReason`; `enter` still allows no reader (silent hold) and takes main's words for a bound or already-tuned-out channel; the coalesced wake timer wraps the branch's `fireWake` in main's `timerFailed`; `cancel_tuneout` keeps the server-qualified lookup and the all-except path, with main's `notTunedOut` words; `speak_in_channel` has main's words and the branch's caller fallback. - src/agent.ts (2): main's `requestSettings()` plus `assistantParticipant`; `presentOwnTurns` before main's request-only rows, which render under `presentAs`. - src/mcpl/channel-registry.ts (2): main's acceptance step keeps only its transport confirmation; the locus is admission's. `onRouteFailure` is main's type (with `outcome`). - src/index.ts, src/types/trace.ts, test/tune-out-timers.test.ts: unions. Outside the hunks, where main's tests met the branch's rules: - main's #506 rule (a refusal is an error result) is applied to `tune_out` and `focus`, whose refusals were `isError: false`; an unknown channel is refused in main's words (`unknown channel: <id>`). - a tune-out with no reader is a silent hold only where the host offers `tune_out` without one: the coordinator gains `holdWithoutReader`, the framework passes `tuneOutOffered`, and main's tune-out-messages test holds as written; the branch's §3.3 row opts its host in. - main's tests that reached `conversationAgentHomes` use the registry (`bindAsConversationFork`, a registry stub); the ended-agents log is registered through `ensureStateRegistered`, as main's registration test requires; main's content-parity test counts the placeholder to the end of its text field, since the channel lane stores it under its author; main's route-failure rows that route by the registry's default locus run under `speech.locus: 'latest-inbound'`, the rule they assume; the branch's present-while-acting routing literal gains main's `turnId`, `drafts` and `keptPrivate`. - main's new `@modelcontextprotocol` dependencies installed (rig relinked after), and the merge fragment extended. Composed rig (the branch chronicle at 03b7bc1 and context-manager at bce0c8b linked, one membrane copy), Node 26.5.0: `npm test` 4707 tests, 4679 pass, 0 fail, 28 skipped. The skips are main's: 22 workspace-watcher rows (chokidar on macOS), two case-folding disk rows, the Linux setuid row, the Python eager-task-factory row, and two deferred-window-only rows that context-manager 0.15 makes moot. Typecheck clean. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Brings in 22 pull requests merged into main since ce1002b, among them the skip_reply wake reader (anima-research#501), the interpreter's runtime directory (anima-research#193), registered agents' allowlist enforcement (anima-research#220) and the stop that reaches every cleanup (anima-research#517). Conflicts: - src/framework.ts: - The wake reader replaced requestedSelfWakeSeconds. This branch's changesRouting and ROUTING_TOOLS stay. - stop(): this branch's flush of each stream's waiting complete goes through step(), as each step of the teardown does since anima-research#517, one call per complete. So one that throws neither ends the teardown nor keeps the other streams' completes from going out. - The tool-calls round: main marks a refused skip_reply, and leaves a call the allowlist refuses out of the round's cause, where it reads a round's direct calls. Here a round's calls, direct or from its foreground script, are counted in one place, roundCallOf, with that predicate: a skip_reply is refused when the agent's allowedTools doesn't name it or its wake can't be set, and keeps only its own round private; any other call the allowlist refuses posted nothing, so it gives no cause. So a call a script makes counts as the same direct call does, refused or not. - src/code-execution/py-runner.ts: this branch's interpreter counter and main's spawnablePath sit side by side. The interpreter's number is still set where it's spawned. - test/skip-reply-wake-xml-mode.test.ts: both sides' helper changes, and the gated case passes { gate: true }. Texts: - skipReplyRefusal's doc names a script's calls too. - The allowlist fragment no longer leaves out a skip_reply a script makes. - This branch's fragment names both refusals of an inner skip_reply, and says a refused inner send silences nothing. Tests: - code-execution: a skip_reply and a send the allowedTools refuses inside a script, each reached by forging the call as the allowlist's own tests do: the skip_reply keeps only its own round private, and the send silences nothing. - stop-step-throws: a stream's waiting complete that throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
Restricted agents could reach omitted tools through public dispatch or utilities, and scripts could retain access after their framework-created caller was disposed. This PR enforces registered callers' existing allowlists at dispatch. The R1 follow-up also makes the utility migration visible and distinguishes an unregistered identity from a disallowed tool.
Changes
utils. Addingutilsexposes the list/describe/run slot; run still checks the utility name. No second direct-call gate is added.utilsfrom per-agent surfaces when no registered utility is permitted. An explicit list containingutilswith no permitted registered utility warns once per Agent instance, after modules are registered or on a later surface read.Tests
Validation on 5fc0aa4:
--types node); no source cast, tsconfig or verifier was weakened.The documentation follow-up changes only README.md and the Breaking fragment: it describes the released
utils listbaseline and limits the warning statement to a nonempty registry with no permitted utility. The exact fragment passes the actual release-changelog script in an isolated fixture. Production source and tests remain unchanged from the validated head above.Not verified
No new Linux, live-provider or installed-runtime result is claimed. These restrictions govern Framework dispatch and do not establish OS isolation. Dependencies are unchanged.
Companion PRs
Host anima-research/connectome-host#191 is merged. Hosts using its utility-name depth filtering need the release containing that change before adopting own-name enforcement. The companion is the first connectome-host release after 0.10.2, containing that PR. The changelog no longer carries a pending-version placeholder or an instruction to the release operator; the existing release guard remains unchanged.
AI-assisted: OpenAI Codex.
What each part is for
The house added this section on October 18. rergards' text above is unchanged, and its Tests section describes
5fc0aa48. The house's comments on this thread give the runs since.rergards' parts
agentToolPermissionFailureruns first indispatchToolCall, which the model's calls and scripts' inner calls go through, and inexecuteToolCallFromfor a call made under an agent's name. A refused call runs nothing and returns an error result saying why.createEphemeralAgentnever gave out, such as host code's call that names no agent, is checked against no list, as before. A namecreateEphemeralAgentgave out is refused while that agent isn't registered, so a script still running when its ephemeral agent retires gets no tools rather than every tool.utils'describeandrun, with module calls exempt, so a list grants utilities one at a time.utilsslot, the failedlistand the warning each answer a list that namesutilsbut no utility the agent may use, for a different reader. The model isn't offered a slot it can't use. A caller that reacheslistanyway gets both counts and the fix. The host author, who can fix the list, gets one warning per agent.getAllUtilities()lets a host build an explicit list that names every utility, as connectome-host does for a subagent at its deepest spawn level.prose_helpis named only where the agent may call it, in the mode primer and the bounce notice, so the model isn't pointed at a tool it can't call.The house's parts
skip_replykeeps its own round private and silences nothing after it, whether the list refuses it or the host can't set its wake. A round sent out that the model meant to keep can't be taken back. A round held back can still be said, since the refusal leaves the turn open. One predicate serves both refusals.think's andjournal's descriptions nameskip_replyonly where the agent may call it. A refusedskip_replyleaves the turn going on, so advice to call it led a model that meant to stay quiet to a refusal, and its next words went out.think's result note names no tool to end the turn with, since its handler can't see the caller's list.utils' description sayslistnames the utilities you can use, so a restricted agent doesn't take its list for the whole registry. By our reading, since the slot's text changes, each agent with the slot writes its cached prompt prefix once after upgrading.AgentConfig.allowedTools' doc say that a utility's name alone passes the dispatch check for any call by that name.maincarry these rules intomain's record of why a round's words are held back.Left out: reserving
__ephemeral__as a name (R2-4), which Anarchid's R2 review called theoretical.Written by Nell-1783 (Claude Opus 5.5) from the house's rent pass, on Ada-1017's design calls, using the Rookery's Pi tooling.