Skip to content

[Fix] preserve tool masks across sub-agent handoff#834

Merged
dingyi222666 merged 4 commits intov1-devfrom
fix/agent-mcp-system
Apr 17, 2026
Merged

[Fix] preserve tool masks across sub-agent handoff#834
dingyi222666 merged 4 commits intov1-devfrom
fix/agent-mcp-system

Conversation

@dingyi222666
Copy link
Copy Markdown
Member

@dingyi222666 dingyi222666 commented Apr 16, 2026

This pr fixes sub-agent handoff tool masking so delegated runs inherit the effective session-aware tool permissions, including MCP and skill tool visibility.

New Features

None.

Bug fixes

  • build sub-agent tool masks with session and source aware filtering so delegated runs cannot see tools blocked for the current request
  • carry the computed tool mask and sub-agent context through both generate and stream handoffs so nested agent runs use the same effective permissions
  • attach the computed toolCallMask to sub-agent tool masks so downstream tool-call filtering matches the exposed tool list
  • prevent skill prompt injection when the computed sub-agent tool mask does not allow the skill tool

Other Changes

  • make createSubAgentToolMask asynchronous so it can include session-aware tool-call filtering
  • pass the resolved sub-agent tool mask back from ChatLunaAgentSubAgentService for delegated agent creation
  • keep fallback sub-agent contexts synchronized with the computed tool mask instead of rebuilding a separate default mask

Build sub-agent tool masks with session-aware tool-call filtering and carry them through delegated agent runs. This keeps MCP tools and the skill tool aligned with the parent agent's effective permissions during handoff.
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 16, 2026

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (2)
  • packages/extension-agent/package.json is excluded by !**/*.json
  • packages/extension-tools/package.json is excluded by !**/*.json

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 15fbc946-7c61-4dc5-9d9b-e04dab054c2f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

将子代理工具掩码的生成改为异步、可感知会话与来源;权限服务在构建基础掩码时同时使用 canUseTool 与 isSessionAllowed,并异步生成 toolCallMask;运行时与子代理服务将该掩码传递并用于技能调用过滤(applyToolMask)。

Changes

Cohort / File(s) Summary
权限服务更新
packages/extension-agent/src/service/permissions.ts
createSubAgentToolMask 从同步改为异步:签名变为 (info, session?, source = 'chatluna') => Promise<ToolMask)createToolCallMasksession 参数改为可选。基础 allow/deny 使用 canUseTool(info, tool.name)isSessionAllowed(session, source, tool),并异步生成 toolCallMask(通过 createToolCallMask(session, mask))。同时 _toolCacheKey 的计算改为基于按 name 排序且序列化的完整工具注册表投影。
子代理服务集成
packages/extension-agent/src/service/sub_agent.ts
_createTaskRuntime()get 返回值中加入 toolMask 字段,该字段由 this.permission.createSubAgentToolMask(info, ctx.session, ctx.source ?? 'chatluna') 异步计算并返回。
子代理运行时集成
packages/extension-agent/src/sub-agent/run.ts
引入 applyToolMaskToolMask,在运行时获取并传递 toolMask 到 agent 的 generate/stream 调用;技能解析使用 toolCallMask(取 toolMask.toolCallMask ?? toolMask)做短路判断;子代理上下文构造改为必须显式包含 toolMask,并将 toolMask 传入 createAgent(...)

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SubAgentService as SubAgent Service
    participant PermissionService as Permission Service
    participant SubAgentRuntime as SubAgent Runtime
    participant Agent as Agent

    Client->>SubAgentService: 请求子代理 (info, session, source)
    SubAgentService->>PermissionService: createSubAgentToolMask(info, session, source)
    PermissionService->>PermissionService: 使用 canUseTool 与 isSessionAllowed 过滤工具
    PermissionService->>PermissionService: 异步生成 toolCallMask (createToolCallMask)
    PermissionService-->>SubAgentService: Promise<ToolMask>
    SubAgentService-->>Client: 返回 { agent, toolMask }

    Client->>SubAgentRuntime: 触发 generate/stream (包含 session)
    SubAgentRuntime->>PermissionService: (必要时) 请求/验证 toolMask
    SubAgentRuntime->>SubAgentRuntime: applyToolMask('skill', toolCallMask) 进行技能允许校验
    alt 技能被允许
      SubAgentRuntime->>Agent: generate/stream(..., toolMask, subagentContext)
      Agent-->>SubAgentRuntime: 响应
      SubAgentRuntime-->>Client: 转发响应
    else 技能被拒绝
      SubAgentRuntime-->>Client: 返回 undefined / 阻止调用
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 会话轻敲,来源细闻,
掩码悄织,工具有门,
子代理遵规而行,
权限同行,调用稳心,
胡萝卜鼓掌,代码安宁。

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR标题准确总结了主要变更:通过会话感知的权限过滤和工具掩码传播来修复子代理切换时的工具可见性问题。
Description check ✅ Passed PR描述详细说明了修复的具体问题、实现方式和代码改动,与变更集内容紧密相关。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/agent-mcp-system

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements session-aware tool masking for sub-agents to ensure permissions and session restrictions are propagated during creation and handoff. Key feedback points out that the sub-agent context must update the agent identity and increment recursion depth during handoff to maintain correct tracking and limits. Furthermore, the tool mask creation should ideally intersect with parent masks to preserve dynamic runtime restrictions.

Comment thread packages/extension-agent/src/sub-agent/run.ts
Comment thread packages/extension-agent/src/service/permissions.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/extension-agent/src/sub-agent/run.ts (1)

88-95: 这几个独立异步步骤可以并行,避免把 handoff 延迟串起来。

createSubAgentToolMask()resolveModel()resolveEmbeddings() 彼此不依赖,当前串行执行会平白增加每次 delegated run 的启动时间。

♻️ 可选改法
     const source = input.source ?? 'chatluna'
-    const toolMask = await options.permission.createSubAgentToolMask(
-        options.info,
-        input.session,
-        source
-    )
-    const llm = await resolveModel(options.ctx, options.info, options.model)
-    const embeddings = await resolveEmbeddings(options.ctx)
+    const [toolMask, llm, embeddings] = await Promise.all([
+        options.permission.createSubAgentToolMask(
+            options.info,
+            input.session,
+            source
+        ),
+        resolveModel(options.ctx, options.info, options.model),
+        resolveEmbeddings(options.ctx)
+    ])

As per coding guidelines "ALWAYS USE PARALLEL TOOLS WHEN APPLICABLE".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/extension-agent/src/sub-agent/run.ts` around lines 88 - 95, The
three independent async calls (createSubAgentToolMask, resolveModel,
resolveEmbeddings) are being awaited sequentially which unnecessarily adds
latency; run them in parallel using Promise.all (or Promise.allSettled if you
need per-call error handling) and then destructure the results into toolMask,
llm, and embeddings so the rest of run.ts uses those values; ensure you still
compute source beforehand and pass options.info/options.model/options.ctx as
before when invoking createSubAgentToolMask, resolveModel, and
resolveEmbeddings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/extension-agent/src/sub-agent/run.ts`:
- Around line 225-228: The skill hint injection is using the outer toolMask so
authority-aware filtering is bypassed; replace the check applyToolMask('skill',
toolMask) with the authority-aware mask (the toolCallMask produced by
createSubAgentToolMask) so visibility matches actual call permissions—i.e.,
obtain the toolCallMask used for permission evaluation and call
applyToolMask('skill', toolCallMask) (and/or use permission.canUseTool(info,
'skill') against the same toolCallMask) before injecting the skill prompt,
ensuring service, applyToolMask, toolMask, createSubAgentToolMask, toolCallMask,
permission.canUseTool and info are the referenced symbols to modify.

---

Nitpick comments:
In `@packages/extension-agent/src/sub-agent/run.ts`:
- Around line 88-95: The three independent async calls (createSubAgentToolMask,
resolveModel, resolveEmbeddings) are being awaited sequentially which
unnecessarily adds latency; run them in parallel using Promise.all (or
Promise.allSettled if you need per-call error handling) and then destructure the
results into toolMask, llm, and embeddings so the rest of run.ts uses those
values; ensure you still compute source beforehand and pass
options.info/options.model/options.ctx as before when invoking
createSubAgentToolMask, resolveModel, and resolveEmbeddings.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d414e757-b5ab-4920-9ed4-89888479c5ed

📥 Commits

Reviewing files that changed from the base of the PR and between 28bbb2f and 08e3b88.

📒 Files selected for processing (3)
  • packages/extension-agent/src/service/permissions.ts
  • packages/extension-agent/src/service/sub_agent.ts
  • packages/extension-agent/src/sub-agent/run.ts

Comment thread packages/extension-agent/src/sub-agent/run.ts
Check the skill tool against the effective tool-call mask instead of the broader tool list mask. This keeps sub-agent skill prompt injection aligned with the tools that can actually be called during handoff.
Include tool metadata in the permission service cache key so tool list changes invalidate the cached entries correctly. This keeps sub-agent and MCP permission filtering in sync after tool registry updates.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
packages/extension-agent/src/service/permissions.ts (2)

322-330: createToolCallMask 声明为 async 但函数体中没有 await

改为 async 后返回值被包装为 Promise<ToolMask>,这对 createSubAgentToolMaskstart() 中的 await 调用点是兼容的,但函数内部并无异步操作,标记 async 仅用于签名统一。如果未来该函数确实需要异步(例如会话级异步校验),保留 async 合理;否则可考虑保持同步以避免不必要的 Promise 分配。建议在此函数预期会包含异步逻辑时再改为 async

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/extension-agent/src/service/permissions.ts` around lines 322 - 330,
The function createToolCallMask is declared async but contains no await; remove
the async keyword and change its return type to ToolMask (return
buildToolMask(...) directly) to avoid unnecessary Promise allocation, and update
any related typings if needed (caller sites like createSubAgentToolMask and
start() that currently await the result will still work with await on a
non-Promise value but adjust declared types where they expect
Promise<ToolMask>).

142-155: 缓存键计算应延后至需要时,而非每次都执行昂贵的序列化操作。

listTools() 在每次调用时都会执行 map + sort + JSON.stringify 来构造缓存键(第 144-152 行),即使缓存命中也要付出这部分开销。虽然代码已通过监听 'chatluna/tool-updated' 事件并调用 invalidateCache() 来实现缓存失效机制,但仍未解决根本的性能问题。

建议改为在缓存存在时直接返回,仅在需要重建缓存时才计算键值:

if (this._toolCache && this._toolCacheKey === key) {
    return this._toolCache
}

改为:

if (this._toolCache) {
    // 只有缓存失效才会进来,无需再计算键
    return this._toolCache
}

或使用轻量级版本号/hash 代替完整的 JSON.stringify,以减少每次调用的开销。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/extension-agent/src/service/permissions.ts` around lines 142 - 155,
The current listTools() always builds a JSON.stringify key via getRegistry()
(map+sort+stringify) even when this._toolCache exists; change it so the method
returns this._toolCache immediately when present (i.e., check this._toolCache
first) and only compute the expensive key/_toolCacheKey when rebuilding the
cache; update logic around _toolCacheKey and invalidateCache() (triggered by
'chatluna/tool-updated') so the key is computed lazily or replaced by a
lightweight counter/hash to avoid repeated serialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/extension-agent/src/service/permissions.ts`:
- Around line 322-330: The function createToolCallMask is declared async but
contains no await; remove the async keyword and change its return type to
ToolMask (return buildToolMask(...) directly) to avoid unnecessary Promise
allocation, and update any related typings if needed (caller sites like
createSubAgentToolMask and start() that currently await the result will still
work with await on a non-Promise value but adjust declared types where they
expect Promise<ToolMask>).
- Around line 142-155: The current listTools() always builds a JSON.stringify
key via getRegistry() (map+sort+stringify) even when this._toolCache exists;
change it so the method returns this._toolCache immediately when present (i.e.,
check this._toolCache first) and only compute the expensive key/_toolCacheKey
when rebuilding the cache; update logic around _toolCacheKey and
invalidateCache() (triggered by 'chatluna/tool-updated') so the key is computed
lazily or replaced by a lightweight counter/hash to avoid repeated
serialization.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7e55f80d-739b-4a57-a749-1fc2cd163fd3

📥 Commits

Reviewing files that changed from the base of the PR and between a53463c and b9763a2.

📒 Files selected for processing (1)
  • packages/extension-agent/src/service/permissions.ts

Release chatluna-agent 1.0.21 and update extension-tools to the matching peer dependency range.
@dingyi222666 dingyi222666 merged commit 159227c into v1-dev Apr 17, 2026
4 of 5 checks passed
@dingyi222666 dingyi222666 deleted the fix/agent-mcp-system branch April 17, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant