[Fix] preserve tool masks across sub-agent handoff#834
Conversation
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.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough将子代理工具掩码的生成改为异步、可感知会话与来源;权限服务在构建基础掩码时同时使用 canUseTool 与 isSessionAllowed,并异步生成 toolCallMask;运行时与子代理服务将该掩码传递并用于技能调用过滤(applyToolMask)。 Changes
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
📒 Files selected for processing (3)
packages/extension-agent/src/service/permissions.tspackages/extension-agent/src/service/sub_agent.tspackages/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.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/extension-agent/src/service/permissions.ts (2)
322-330:createToolCallMask声明为async但函数体中没有await。改为
async后返回值被包装为Promise<ToolMask>,这对createSubAgentToolMask和start()中的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
📒 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.
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
generateandstreamhandoffs so nested agent runs use the same effective permissionstoolCallMaskto sub-agent tool masks so downstream tool-call filtering matches the exposed tool listskilltoolOther Changes
createSubAgentToolMaskasynchronous so it can include session-aware tool-call filteringChatLunaAgentSubAgentServicefor delegated agent creation