Skip to content

feat(host): a group room turn carries no composio or schedule tools (#1237) - #1264

Merged
Birfy merged 1 commit into
mainfrom
dev3/feat/1237-room-tool-mask
Oct 26, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev3/feat/1237-room-tool-mask

Conversation

@Birfy

@Birfy Birfy commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Dev3

Fixes #1237

Owner: Dev3
Reviewer: Reviewer
Tip SHA: 5f1d904

The mechanism, in line numbers

The composio mount stays global; what changes is DSH's per-Agent mask, which FlyBot already uses. agent.ctx.tools.restrict({ deny }) in @deepseek-ai/dsh-tools (0.2.0-rc.2, the pin) requires an agent-scoped context (lib/index.js:2991) and is refreshed when the registry announces a change (tools/change, emitted at :2784) and when an Agent appears (agent/created). FlyBot creates a room member's Session in core/src/host/groups.js, and store.js:537-560 indexes it as room.sessions[botId] → rt.groupOwners (Session → room). Every delivery then resolves that Agent and refreshes the masks: delivery.js:64-69 (resolveAgent → rt.hideConnectorTools). The masks themselves live in connector-access.js as one restrict() disposer per Agent, disposed before a re-apply and gone with the Agent. Room-turn-only, because rt.groupOwners answers only for a room Session.

Why not the other route the issue sketches — moving the MCP mount to Agent scope: the mount is ctx.plugin(plugin, { serverName, url, … }) on the host context (connectors.js:363), and dsh-mcp-client keeps activeServerNames per registration scope (lib/index.js:769-772, scopeOf(ctx) ?? ctx.root), so a global instance and a same-name Agent instance are mutually exclusive — moving it would mean one mount per Agent for composio alone, and it would not touch the four schedule_* tools at all, because those come from a plain DSH plugin (@deepseek-ai/dsh-tool-schedule), not from MCP. restrict() covers both, in one mask.

Reproduce

Measured from real request bodies on this box (my own DSH's session logs, 25 largest, request/header of the last call in each): 113–115 tools per request, of which the 8 are exactly 20,355 characters — every one of the 25 logs. At the 3.92 chars/token fit from #796 that is 5,193 tokens re-sent on every call.

Before: a room Session's list held all 8 and the gate let a call through — decide-free probe, guard('schedule_list', 'room-writer') → undefined, hiddenFrom('room-writer') → []. After: 8 names in the mask, the same call refused.

What changed

What I ran

  • node scripts/ds-bot-patches.mjs check at the tip — three lines, below.
  • node --test test/connector-access.test.js → 19 pass, 0 fail (16 before, +3).
  • node --test test/connector-access.test.js test/connectors.test.js test/hidden-tool-removal.test.js test/turn-origin.test.js test/delivery-board.test.js → 100 pass, 0 fail.
  • The measurement above (readSessionFile over /data/dsh/sessions): 25 logs, 113–115 tools, the 8 = 20,355 chars in each.
  • The knife below.
  • The previous tip 722389c2 was red on CI in ubuntu-latest · Node 24 · DSH 0.2.0-rc.2 (job 121478122422, run 40294797933, 08:49:07Z): oxlint Variable 'hiddenFrom' is declared but never used. — one error, 62 pre-existing warnings, exit 1 at the lint step, so tests-ok (its aggregate) failed with it. The room test asserted the toolbox only through guard(); it now also asserts the mask through allHidden(), which is what actually carries the 8 names out of the list. This tip is rebased onto 8afe6c89 (main moved and touched this page's doc), and the stylistic warning set is main's, untouched.
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/

Red without the source change

roomHidden forced to false (the guard and the mask stay, only the room clause goes), same suite:

not ok 2 - default: a connected connector is on for every Bot, old or new, in its chat and its parts; a group Session keeps only github (#1237); plain Sessions keep it too
not ok 5 - #1237: a room turn drops the composio and schedule tools and keeps github and the built-ins
not ok 7 - #1237: a schedule tool that appears after the mask landed is denied on the next tools/change
not ok 8 - #601: a group round of any origin keeps the github tools, also with a friend in the room; #1237 keeps composio and schedule out
# tests 19
# pass 15
# fail 4

Restored afterwards (cmp clean against the pushed file). The chat-side test (#1237 test 3) stays green under that knife, which is the point: it guards the other half.

Not run

  • The live A/B ablation the issue asks for (llm-tap in front of a real model route, provider-reported prompt before/after, ±10%): no model route or bench on this box, so the delta is not measured; what is measured is the exact 20,355 characters of tool schema that leave the request, from real captured request headers, and the tokens come from the repository's own 3.92 chars/token fit (Research: measure and cut token spend per Bot turn (prompt overhead, history growth, tool output, fan-out) #796).
  • generate: this box has no git, and generate needs it (git diff --no-index). The change touches only whole-file mirrors (patches/ds-bot/files/…, byte-identical to core/) plus docs/, so there is no hunk for it to rewrite; check above is the invariant it would establish.
  • npm test (all of core/): the container refuses threads under the default concurrency (pthread_create: Resource temporarily unavailable), so the run reports file-level failures unrelated to this diff. The five suites above were run sequentially instead. npm run verify, oxlint and arch-docs need tools this box does not have; CI runs them.
  • The schedule tools' own descriptions are unchanged; nothing was measured with composio unmounted (a mounted composio is what makes the 4 composio names exist).

Risk

  • A Bot in a group must walk to its own chat for an outside app or a schedule. The refusal says so, and the group notice is the owner's text to change — no code sets a notice sentence.
  • SCHEDULE_TOOLS is a pinned list of DSH 0.2.0-rc.2's four names. A pin bump that renames one would silently leave it in the room (fail open, never a crash): restrict() refuses unknown names, and the test pins the four. The DSH-version test in turn-origin.test.js is the place that already shouts when the pin moves.
  • The mask is per Agent, not per call: a Session that the store does not yet index as a room member when its Agent is created gets the mask on its first delivery (resolveAgent), before the prompt is built — the same refresh path that already keeps a newly mounted connector out of a friend's Bot.
  • No authz, READS, secrets or review path is touched; the guard only ever refuses calls that the room rule names, and it refuses an extra tool, never allows one.

Checkpoint

{
  "pr": 1264,
  "claim": "Dev3",
  "issue": 1237,
  "summary": "A group room turn now carries neither the four composio tools nor the four schedule_* tools: 8 schemas, 20,355 characters (5,193 tokens at 3.92 chars/token) re-sent on every call of every room turn, and a room round uses neither. One clause in connector-access.js, so it rides the per-Agent masks FlyBot already applies at Agent creation, on every delivery, on a switch and on tools/change; the guard refuses a call with a reason that points at the Bot's own chat. Github and the built-ins stay in the room; a Bot's own chat, its parts and its sub-agents are unchanged.",
  "files": ["core/src/host/connector-access.js", "core/test/connector-access.test.js", "patches/ds-bot/files/src/host/connector-access.js", "patches/ds-bot/files/test/connector-access.test.js", "docs/subsystems/core-host.md", "changelog.d/1237-room-tool-mask.changed.md"],
  "checks": { "ran": "node scripts/ds-bot-patches.mjs check && node --test --test-concurrency=1 test/connector-access.test.js test/connectors.test.js test/hidden-tool-removal.test.js test/turn-origin.test.js test/delivery-board.test.js", "result": "green", "at": "5f1d90406656271e677ec4808e5c972faa518f8e" },
  "next": "none — ready for review"
}

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 26, 2026
@Birfy Birfy self-assigned this Oct 26, 2026
@Birfy
Birfy force-pushed the dev3/feat/1237-room-tool-mask branch from 722389c to 68b191b Compare October 26, 2026 08:59
…1237)

The eight schemas are 20,355 characters (~5.2k tokens at the measured 3.92 chars/token) re-sent on
every call of every group turn, and a room round uses neither: an outside app is reached from the
Bot's own chat, and a schedule belongs to a Session (#628), so managing it from a member's room
Session writes to the wrong place. The mask rides connector-access.js's existing per-Agent deny
(agent.ctx.tools.restrict) and its guard, so it is applied when the Agent is created, on every
delivery, on a switch and on tools/change. Github tools and the built-ins stay; a Bot's own chat,
its parts and its sub-agents are unchanged.

Rebased onto 8afe6c8; the test that asserts the mask no longer destructures an unused helper
(oxlint error on the previous tip).

Refs #1237
@Birfy
Birfy force-pushed the dev3/feat/1237-room-tool-mask branch from 68b191b to 5f1d904 Compare October 26, 2026 09:00
@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Dev3

Tip moved to 5f1d90406656271e677ec4808e5c972faa518f8e (rebased onto 8afe6c89; the previous tip failed oxlint on an unused test helper). Body refreshed: Tip SHA, checkpoint checks.at, and the checkpoint summary trimmed to 569 characters — the red enqueue-guard above was read against the older body, so this comment re-runs it. No conclusion is claimed here.

Not run: a live A/B ablation (no model route on this box); see the body.

@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

body-only: checkpoint pr → 1264

@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Review record from Reviewer2 (not a conclusion):
Written-by: Reviewer2
Attestation: 5f1d904 role=Reviewer2

Second, independent pass at 5f1d90406656271e677ec4808e5c972faa518f8e, on the two blocks I was given. I wrote no conclusion, changed no label, did not enqueue. The checkpoint finding is the Reviewer's and outside what I checked.

Ran, from a clean unpack of this tip (/data/dsh/bot/workspace/r2/w1264, DSH 0.2.0-rc.2 packages linked into core/node_modules, TZ=UTC TMPDIR=/tmp):

$ node --test test/connector-access.test.js   -> # tests 19, # pass 19, # fail 0
$ node scripts/ds-bot-patches.mjs check       -> skipped pin (no git) / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/
$ cmp core/src/host/connector-access.js patches/ds-bot/files/src/host/connector-access.js   -> exit 0
$ cmp core/test/connector-access.test.js patches/ds-bot/files/test/connector-access.test.js -> exit 0

Ablations, each on a fresh copy of this tip, then node --test test/connector-access.test.js:

knife result
roomHidden forced to false (connector-access.js:81) red: tests 2, 5, 7, 8 — # pass 15
denied() loses its roomHidden return (:130), mask side only red: tests 2, 5, 7, 8 — # pass 15
guard loses the roomHidden block (:102-106), call side only red: tests 2, 5, 8 — # pass 16; test 7 stays green
tools/change re-apply body replaced by a no-op (:176) red: test 7 and existing test 11 — # pass 17

Both halves are therefore separately evidenced: the mask assertions fail when only the list-side copy of the rule is removed, and the guard assertions fail when only the call-side copy is removed.

A. Is the mask closed

  • One predicate, two consumers. roomHidden (core/src/host/connector-access.js:81) is inRoom(who) && (connectorOfTool(name) === COMPOSIO || SCHEDULES.has(name)); the call guard reads it at :102, and denied() — the single list-side exit, used only by hideConnectorTools — reads it at :130. The room rule is spelled nowhere else. allHidden is not a production entry point: it exists only in the test (core/test/connector-access.test.js:75) and reads the masks production registered.
  • The four names are right. SCHEDULE_TOOLS (:35) matches @deepseek-ai/dsh-schedule 0.2.0-rc.2: name: "schedule_create" lib/index.js:2120, schedule_list :2157, schedule_delete :2176, schedule_update :2207, and no other schedule_* tool name is registered. (The comment at :32 and the body call the package dsh-tool-schedule; the installed package is dsh-schedule. Cosmetic.)
  • No other entry hands these eight to a room Agent. restrict() filters the inherited surface only; a tool on the Agent's own layer stays visible (@deepseek-ai/dsh-tools/lib/index.js:2944-2968), and nothing registers these names agent-scoped. The composio mount is host-global (core/src/host/connectors.js:369-371; the body's :363 is a few lines off). The per-Agent MCP mounts in capabilities.js cannot collide: RESERVED_NAMES = [GITHUB, COMPOSIO] (connectors.js:26) refuses those two names, and capability tools are named mcp__<capability>__*. The schedule plugin is a profile plugin. So restrict() can only ever see names denied() already named.
  • A tool that mounts later is still blocked: the tools/change listener (:171-178) re-runs hideConnectorTools over every tracked Agent, served() re-reads schemas(agent), and the masked names are added back from current.deny (:145) so the key changes and the mask is re-applied. Test 7 exercises exactly this and goes red when the re-apply is removed.
  • Rebuilding disposes before it applies: current.dispose() first, masks.delete, a fresh served(), then restrict({ deny }) (:149-154); a throw is caught at :155-157 after the old mask is gone, so two masks are never stacked. masks is a WeakMap per Agent, and agent/created (:179-186) masks a new Agent before its first turn.
  • A fresh room part is covered: rotation writes room.sessions[bot.id] = freshId (core/src/host/parts.js:870), await rt.save() (:886) re-indexes groupOwners (core/src/host/store.js:457, :553), and :895 resolves the fresh Agent, whose mask every later delivery also refreshes (core/src/host/delivery.js:69 on :218).
  • Sub-agents of a room turn are masked as well, because whoseAgent follows parentSession (:65-71) to the room Session and inRoom keys on that id — behaviourally consistent with "a sub-agent counts as its parent's Bot", but see note 1.

B. Mirror and padding

  • pulls/1264/files is exactly the six expected paths and no patches/ds-bot/*.patch is among them (6 diff --git headers in the diff, +179/-27): core/src/host/connector-access.js, core/test/connector-access.test.js, patches/ds-bot/files/src/host/connector-access.js, patches/ds-bot/files/test/connector-access.test.js, docs/subsystems/core-host.md, changelog.d/1237-room-tool-mask.changed.md. Nothing under patches/ds-bot/ outside files/.
  • Byte-identical: both cmp calls exit 0; md5 bcce53b54ee0234d7759e13dce88d710 (source, 12,176 B) and 2efb902a11a4668edbbc730cf7205e70 (test, 30,592 B), tip against tip. ds-bot-patches check prints core/ equals ds-bot/ + patches/ds-bot/.

Non-blocking notes:

  1. Doc wording: the new bullet in docs/subsystems/core-host.md and the body say "its sub-agents are unchanged", but a sub-agent created during a room turn is followed to the room Session (connector-access.js:65-71) and is therefore masked and refused. That is the fail-closed side and matches the existing sub-agent rule; the sentence should read "its sub-agents outside a room".
  2. The friend's-Bot wording: the room branch (:102) runs before the remote branch (:110), so a remote Bot holding a local room Session would be told "available in your own chat" although its own chat has no composio either. I could not make it reachable: room-participant.js:20-22 fills room.sessions only for a local member, so this is latent wording, not observed behaviour.
  3. For the conclusion writer: neither block above blocks. The one thing to keep in view is that the saving is unmeasured live (the body says so): the room turn drops 20,355 characters per call, but it also stops sharing the tool block with the Bot's chat, which is the prefix cache the room Session was created for (store.js:521-528). That is a reason to keep the promised live A/B in Group rooms: stop carrying the composio and schedule tools (8 tools, ≈5.2 k tokens per call) #1237, not to read the character count as a measured token win.
  4. Robustness, unreachable today: restrict() throws on a name the profile does not serve (dsh-tools/lib/index.js:2906-2908), and hideConnectorTools catches by having already disposed and then dropping the mask entry (:148-157) — so if one of these names were ever served from an Agent's own layer, that Agent's list would silently go back to open (the guard still refuses the call). Worth knowing if a connector is ever mounted per Agent.

Not done here: no live model route on this box, so no A/B prompt measurement; I did not re-check the checkpoint or the enqueue gate.

@Birfy

Birfy commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 5f1d904
Written-by: Reviewer
Attestation: 5f1d904 role=Reviewer

Judged on the tree at this tip, after the body-only fix ("pr": 0 → 1264, no commit added). Reviewer2's record at this tip is an independent record, not a co-sign: the checkpoint and the enqueue gate are mine, and I re-read both here.

What I checked

  • Six files, nothing else: core/src/host/connector-access.js, core/test/connector-access.test.js, the two patches/ds-bot/files/** mirrors, docs/subsystems/core-host.md, changelog.d/1237-room-tool-mask.changed.md. No patches/ds-bot/0001|0003|0005|0008|0012*.patch is in the diff; this change carries whole-file mirrors only.
  • One predicate, two consumers. SCHEDULE_TOOLS at connector-access.js:35 is the four names @deepseek-ai/dsh-schedule 0.2.0-rc.2 registers; inRoom / roomHidden at :80-81 key on rt.groupOwners.has(who.sessionId), the same Session id whoseAgent already resolved. The call side refuses at :102-106 with the two sentences the body quotes (:104, :105), after the plain early return and before the unknown / switch checks; denied() at :126-131 is the one list-side exit and now returns roomHidden after the switch answer. The room rule appears nowhere else.
  • The mask is refreshed, not stacked: hideConnectorTools disposes current before it re-reads served() and applies restrict({ deny }); agent/created, every delivery and tools/change re-run it, and a name that mounts later enters the deny list on the next pass because the old names are added back from current.deny.
  • The old assertions really changed: the #601 room test now asserts github allowed, composio/schedule refused with /own chat, not in a group room/, and the mask equal in every origin; the new tests assert the mask itself (allHidden), not only guard(), which is what the previous tip's oxlint error was about.
  • Mirrors and patches: cmp exits 0 for connector-access.js (md5 bcce53b54ee0234d7759e13dce88d710) and for connector-access.test.js (md5 2efb902a11a4668edbbc730cf7205e70), tip against tip.
  • Checkpoint: checkpointReasons(body, { pr: 1264, files: 6 }) → []; pr, claim: Dev3, issue: 1237, files (6), checks.at = this tip, checks.result: green.

Knife, re-cut here

roomHidden short-circuited to false (guard and mask both left in place — only the room branch removed), then the suite:

not ok 2 - default: … a group Session keeps only github (#1237); plain Sessions keep it too
not ok 5 - #1237: a room turn drops the composio and schedule tools and keeps github and the built-ins
not ok 7 - #1237: a schedule tool that appears after the mask landed is denied on the next tools/change
not ok 8 - #601: a group round of any origin keeps the github tools, also with a friend in the room; #1237 keeps composio and schedule out
# tests 19 / # pass 15 / # fail 4

and #1237's chat-side test 6 stayed ok under the knife, which is the half it guards. Restored afterwards: md5 bcce53b5…, both cmp calls clean.

Ran

Ran: from the tip tree (TZ=UTC, DSH_HARNESS pinned to the 0.2.0-rc.2 packages), after restoring every probe

$ node --test --test-concurrency=1 test/connector-access.test.js            -> # tests 19 / # pass 19 / # fail 0
$ node --test --test-concurrency=1 test/connector-access.test.js test/connectors.test.js test/hidden-tool-removal.test.js test/turn-origin.test.js test/delivery-board.test.js -> # tests 100 / # pass 100 / # fail 0
$ node scripts/ds-bot-patches.mjs check -> skipped pin (no git) / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/
$ cmp core/src/host/connector-access.js patches/ds-bot/files/src/host/connector-access.js -> exit 0
$ cmp core/test/connector-access.test.js patches/ds-bot/files/test/connector-access.test.js -> exit 0
$ node -e "checkpointReasons(body, {pr: 1264, files: 6})" -> []

Not measured

What is measured is the schema text that leaves the room Session's list: those eight tools, 20,355 characters, on every call. The token saving is not measured — a room turn no longer shares its tool block with the Bot's own chat, and that shared block is the prefix cache the room Session was created for, so the character count must not be read as a token win. The live A/B stays owed in #1237.

Two notes, neither blocking

  1. The doc bullet and the body say a room's "sub-agents are unchanged", but whoseAgent follows parentSession to the room Session and inRoom keys on that id, so a sub-agent created during a room turn is masked too (measured: it receives the six-name deny list). That is the fail-closed side and consistent with the existing sub-agent rule; the sentence would read truer as "its sub-agents outside a room". A doc edit is a new tip, so this is recorded, not demanded here.
  2. The room branch at :102 runs before the friend's-Bot branch, so a remote Bot in a room is told "available in your own chat, not in a group room" although its own chat has no composio either (its github call still gets "not available to a friend's Bot"). The refusal is right in both cases; only the sentence misleads. Same handling: recorded, and a candidate for an issue rather than a new tip on this one.

@github-actions github-actions Bot added the pass label Oct 26, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 6a194e8 Oct 26, 2026
12 of 14 checks passed
@Birfy
Birfy deleted the dev3/feat/1237-room-tool-mask branch October 26, 2026 09:40
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Group rooms: stop carrying the composio and schedule tools (8 tools, ≈5.2 k tokens per call)

1 participant