Skip to content

fix(agent): 防止下游 harness 能力无感覆盖 Cindy - #1215

Merged
MagicLizi merged 17 commits into
mainfrom
xdt/harness-capability-routing
Jul 31, 2026
Merged

fix(agent): 防止下游 harness 能力无感覆盖 Cindy#1215
MagicLizi merged 17 commits into
mainfrom
xdt/harness-capability-routing

Conversation

@zqchris

@zqchris zqchris commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

这次改了什么

摘要

修复 Cindy 启动 Codex / Claude Code 后,下游 harness 自带的 Skill、插件、MCP 或 Computer Use 能力可能先于 Cindy 能力生效,导致用户明明在 Cindy 中操作,却被无感切到下游实现的问题。

本 PR 增加宿主能力路由层:保留用户本地、项目和官方插件提供的正常扩展能力;仅对与 Cindy 已接管能力发生冲突的下游内置来源做隔离或显式路由。Codex 的 Computer Use 在 Cindy 会话内由 Cindy cindy_computer 接管;Feishu Delegate 等下游同类能力仍可通过明确命名空间显式调用,不再静默抢路。

实现使用稳定 capability id 和开放的 harness id,不把分支写死为只有 Codex / Claude Code,后续增加第三个 harness 时可以复用同一套决策模型。

变更类型

  • feat 新功能
  • fix 缺陷修复
  • refactor / perf 重构或性能优化
  • docs / test / chore 文档、测试或工程维护
  • 其他:

范围

  • 关联 Issue / 需求:Cindy 会话中下游 harness 能力无感覆盖宿主能力
  • 本 PR 包含:统一 capability route;Codex 插件 overlay 与来源校验;Claude Code 本地/remote tool guard;manager 协议携带并锁定本轮路由;相关单元测试
  • 明确不包含:用户扩展能力的全局禁用;设置 UI;第三个 harness 的具体实现
  • 用户可见变化:在 Cindy 中默认使用 Cindy 已接管的同类能力;用户/项目 Skill、官方插件和普通 MCP 保持可用;下游冲突能力需显式点名才会走其命名空间
  • 是否存在 breaking change:无。新增协议字段均为可选/协商式;未命中 capability route 的能力保持原行为

UI 变化

不涉及:本 PR 只修改 Agent 启动、tool/MCP 暴露与本地 manager 协议,不改 Renderer UI、文案或样式。

  • 引用的设计规范:不涉及

怎么验证的

自动验证

pnpm --filter @cindy/maker-core exec vitest run \
  src/types/capability-routing.test.ts \
  src/agents/codex/capability-routing.test.ts \
  src/agents/codex/index.test.ts \
  src/agents/claude-code/__tests__/capability-routing.test.ts \
  src/agents/claude-code/__tests__/flag-settings.test.ts \
  src/agents/claude-code/__tests__/mcp-approval-policy.test.ts \
  --pool=forks --maxWorkers=1
结果:6 个文件、381 项通过

pnpm --filter @cindy/maker-cc-manager exec vitest run \
  __tests__/protocol.test.ts \
  __tests__/sdk-handlers.test.ts \
  __tests__/session-registry.test.ts \
  --pool=threads --maxWorkers=1
结果:3 个文件、47 项通过

pnpm --filter desktop exec vitest run \
  src/main/__tests__/codexGlobalPlugins.test.ts \
  --pool=threads --maxWorkers=1
结果:1 个文件、22 项通过

pnpm --filter desktop run --if-present typecheck
结果:通过

pnpm --filter @cindy/maker-cc-manager build
pnpm --filter @cindy/maker-cc-manager bundle
结果:通过

pnpm exec eslint <本次 maker-core capability routing 相关文件>
pnpm exec eslint <本次 Desktop capability routing 相关文件>
结果:通过

git diff --cached --check
结果:通过

手工验证

不涉及 UI。路由矩阵通过单元测试覆盖:用户/项目 Skill、官方插件、普通 MCP、同名用户 MCP、下游显式命名空间、Codex Computer Use 替换、Claude local/remote guard、ACK 后路由锁定及失败关闭。

未执行的验证

pnpm test:unit 已尝试,但同机资源争抢下 Desktop suite 出现 176 个分散在 billing/sidebar/review 等无关模块的 5 秒超时,随后 mobile worker ECONNRESET;按本地低并发门禁要求未重跑全仓。上述改动相关测试均在资源释放后以单 worker 全绿,完整仓库门禁交由 CI 验证。

未启动 Desktop 实机:本改动无 UI,行为路径由本地/remote manager 与插件 overlay 测试覆盖。

风险

风险分类

  • 无已知风险
  • SQLite / migration
  • system prompt
  • 协议兼容
  • 权限 / 安全 / 用户数据
  • 原生层 / fingerprint / OTA
  • 跨平台差异
  • 其他:

影响与回滚

  • 影响范围:Desktop 启动的 Codex / Claude Code 会话;Claude remote manager;Codex 全局插件 overlay。无数据库、用户数据、Mobile 原生层或 runtime fingerprint 变更
  • 协议兼容:manager 新字段使用版本协商和可选字段;路由在本轮 ACK 后锁定,旧端或缺失路由时采用失败关闭,不静默放行被 Cindy 接管的冲突能力
  • 权限 / 安全:仅隔离已登记的下游冲突来源;来源无法证明、overlay/symlink 校验失败时不暴露被接管能力。用户本地/项目 Skill、官方插件和普通 MCP 不被一刀切禁用
  • 跨平台:路径与 symlink 处理使用 Node 跨平台 API;未在 Windows 实机验证,由单测和 CI 覆盖
  • 回滚 / 降级方式:回滚本 PR 即恢复 harness 原生能力暴露;capability registry 未命中的第三方能力始终沿用原行为

提交前检查

  • 已 review 完整 diff
  • 每个 commit 都带 DCO 签名(git commit -s,见 DCO
  • UI 改动已在「UI 变化」注明引用的设计规范章节(不涉及 UI 则跳过)
  • 未提交凭证、令牌或授权文件
  • 已补充必要文档
  • 已确认测试结果或说明未执行原因

Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 10:54
@zqchris
zqchris requested a review from a team as a code owner July 31, 2026 10:54
@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown

Greptile Summary

本 PR 新增统一的宿主能力路由机制,避免下游 harness 的内置能力静默覆盖 Cindy:

  • 为 Codex 增加插件 overlay、Skill 隐式调用限制、MCP 重命名与 Computer Use 接管。
  • 为 Claude Code 的本地及 remote manager 链路增加 MCP tool guard 和来源识别。
  • 扩展 manager 协议,在会话 ACK 后锁定能力路由,并补充兼容与失败关闭处理。
  • 增加覆盖策略匹配、插件隔离、协议协商和会话生命周期的测试。

Confidence Score: 5/5

当前变更看起来可以安全合并。

未发现仍需阻止合并的故障。

Important Files Changed

Filename Overview
packages/maker-core/src/types/capability-routing.ts 新增稳定的 capability/source 数据模型,以及显式选择和 Claude MCP 来源匹配逻辑。
apps/desktop/src/main/maker-host/capability-routing.ts 定义 Desktop 对 Feishu 和 Computer Use 的宿主路由策略。
apps/desktop/src/main/maker-host/codex-global-plugins.ts 为 Codex 插件缓存构建隔离 overlay,并实施 Skill、MCP 和来源完整性策略。
packages/maker-core/src/agents/codex/capability-routing.ts 将宿主路由策略转换为 Codex 会话配置覆盖,并处理无 overlay 环境的失败关闭。
packages/maker-core/src/agents/claude-code/capability-routing.ts 为 Claude Code 构建本地 hook 和 remote tool guard。
packages/maker-cc-manager/src/session-registry.ts 在 manager 会话生命周期中应用并锁定 tool guard,同时维护 MCP 来源信息。
packages/maker-cc-manager/src/protocol.ts 扩展 manager 协议以协商并传递本轮能力路由信息。

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Cindy Desktop 能力策略] --> B{下游 Harness}
  B -->|Codex| C[插件 Overlay 与线程配置]
  C --> D[Skill 仅显式调用]
  C --> E[MCP 命名空间重路由]
  C --> F[Computer Use 由 cindy_computer 接管]
  B -->|Claude Code 本地| G[PreToolUse 与审批 Guard]
  B -->|Claude Code Remote| H[Manager 协议协商]
  H --> I[会话 ACK 后锁定路由]
  I --> J[Daemon Tool Guard]
  D --> K[保留用户和非冲突扩展]
  E --> K
  F --> K
  G --> K
  J --> K
Loading

Reviews (16): Last reviewed commit: "fix: distinguish harness MCP provenance" | Re-trigger Greptile

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6bf8377e84

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/maker-core/src/types/capability-routing.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Human review recommended

变更跨 maker-core/desktop/cc-manager 并涉及能力路由与远端协议升级(安全/权限边界与 fail-closed 语义),需要人工做端到端风险确认后再合并。

Pull request overview

本 PR 在 Cindy 启动 Codex / Claude Code 会话时引入“宿主能力路由(capability routing)”层,用于仲裁与下游 harness 内置能力(Skill/插件/MCP/Computer Use 等)发生冲突时的来源选择,避免下游实现静默抢占 Cindy 已接管能力;同时为远端 Claude Code(cc-manager)补齐 daemon 侧的强制闸门以防止绕过。

Changes:

  • 在 maker-core 定义中立的 capability routing 合约与显式选择解析,并分别落地到 Codex/Claude Code 适配层(含本地与远端分支)。
  • cc-manager 协议升级到 v2,引入 toolGuards 并在 daemon 侧重建 PreToolUse guard,确保远端也能在权限规则之前执行来源路由。
  • Desktop 侧新增产品级路由策略与 Codex 全局插件 overlay/隔离逻辑(显式-only 技能、MCP server runtime rename 等),并补齐相应单测。
File summaries
File Description
packages/maker-core/src/types/index.ts 导出 capability routing 类型与工具函数入口。
packages/maker-core/src/types/capability-routing.ts 新增 routing 合约、显式选择匹配与 Claude MCP 前缀匹配工具。
packages/maker-core/src/types/capability-routing.test.ts 覆盖显式 selector 匹配与路由查找行为。
packages/maker-core/src/agents/codex/index.ts Codex:注入 per-thread config overrides、在 MCP elicitation 时按路由策略 fail-closed/显式放行,并绑定 turn 级 selection text。
packages/maker-core/src/agents/codex/index.test.ts Codex:覆盖版本门槛、远端 fail-closed、以及显式选择下的 MCP 放行/拒绝。
packages/maker-core/src/agents/codex/capability-routing.ts 将中立路由策略转为 Codex thread config 的扁平 TOML path overrides。
packages/maker-core/src/agents/codex/capability-routing.test.ts 覆盖 Codex config override 生成与 overlay 不可用时的 fail-closed。
packages/maker-core/src/agents/claude-code/index.ts Claude Code:本地 hooks 预置路由 guard;远端下发 JSON-safe toolGuards;canUseTool 侧增加路由拒绝分支。
packages/maker-core/src/agents/claude-code/flag-settings.ts 将 host capability routing 翻译为 Claude 原生 skillOverrides
packages/maker-core/src/agents/claude-code/capability-routing.ts Claude:skillOverrides 映射、本地 PreToolUse guard、远端 toolGuards 序列化与 hook 合并工具。
packages/maker-core/src/agents/claude-code/tests/mcp-approval-policy.test.ts 覆盖本地 Full access(bypassPermissions)下仍注入 PreToolUse guard,以及远端 toolGuards 生效。
packages/maker-core/src/agents/claude-code/tests/flag-settings.test.ts 覆盖 capability routing → skillOverrides 的映射。
packages/maker-core/src/agents/claude-code/tests/capability-routing.test.ts 覆盖 Claude routing 辅助函数(skillOverrides/toolGuards/hooks merge)。
packages/maker-core/src/agents/base-agent.ts 在 AgentDeps 增加 capabilityRouting 注入点,并更新 hooks 说明。
packages/maker-cc-manager/src/session-registry.ts 远端:维护 turn 内 selection text,并基于 toolGuards 生成 daemon 侧 PreToolUse hooks。
packages/maker-cc-manager/src/sdk-handlers.ts query/start 校验并接入 toolGuards 参数。
packages/maker-cc-manager/src/protocol.ts 协议升级到 v2,引入 QueryToolGuard 并 bump bundle version。
packages/maker-cc-manager/src/index.ts 导出 QueryToolGuard
packages/maker-cc-manager/src/bin/cc-mgr.ts daemon 侧确保“daemon-owned hooks”覆盖 extraOptions.hooks(防绕过)。
packages/maker-cc-manager/src/async-queue.ts push 改为返回 boolean,用于识别 end 后的静默丢消息并显式报错。
packages/maker-cc-manager/tests/session-registry.test.ts 覆盖 toolGuards 的 deny/allow 与显式选择跨 steering 保留。
packages/maker-cc-manager/tests/sdk-handlers.test.ts 覆盖 toolGuards 参数校验(拒绝纯文本歧义 selector)。
packages/maker-cc-manager/tests/protocol.test.ts 断言 PROTOCOL_VERSION 必须为 v2。
apps/desktop/src/main/maker-host/index.ts Desktop:在 maker deps 注入 DESKTOP_CAPABILITY_ROUTING_POLICY
apps/desktop/src/main/maker-host/codex-global-plugins.ts Desktop:为 Codex marketplace 缓存生成 capability-routing overlay(skill 显式-only、MCP server rename、symlink 防护、快照重建与 fail-closed 报告)。
apps/desktop/src/main/maker-host/capability-routing.ts Desktop:定义产品级 capability routing 策略(Feishu delegate / Computer Use 等)。
apps/desktop/src/main/maker-host/auth-adapters.ts Codex 启动前强制检查插件 overlay/隔离结果;存在 routingFailures 时拒绝启动。
apps/desktop/src/main/tests/codexGlobalPlugins.test.ts 覆盖 overlay 不修改用户缓存、MCP rename、源变更触发重建、异常回滚与 fail-closed 语义。
Review details
  • Files reviewed: 27/28 changed files
  • Comments generated: 0
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 11:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43f9850ae6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/desktop/src/main/maker-host/codex-global-plugins.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

发现两处与路由守卫相关的安全边界问题(symlink 跟随导致越界扫描、toolGuards 前缀匹配可能选错 guard)需要先修复以确保 fail-closed 语义可靠。

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

apps/desktop/src/main/maker-host/codex-global-plugins.ts:218

  • treeSnapshot() 会直接对 root 调用 readdir();如果被 overlay 的插件目录本身是 symlink,readdir() 会跟随 symlink 读取目标目录,导致在后续 fail-closed 之前就可能扫描/记录了插件根目录之外的内容。建议在读取前用 lstat() 拒绝根目录 symlink,避免不必要的越界遍历。
async function treeSnapshot(root: string, relative = ''): Promise<unknown[]> {
  const dir = relative ? path.join(root, relative) : root;
  const entries = await fsp.readdir(dir, { withFileTypes: true });

packages/maker-cc-manager/src/session-registry.ts:1002

  • createToolGuardHooks()find() 选择第一个命中的 toolNamePrefix;如果未来出现可嵌套/重叠的前缀(例如 server id 含 __),可能会匹配到更短的前缀,从而应用了错误的 guard(出现误放行或误拦截)。建议改为选择“最长前缀匹配”的 guard 以保证确定性和安全性。
    const toolName = input.tool_name;
    const guard = toolGuards.find((candidate) =>
      toolName.startsWith(candidate.toolNamePrefix),
    );
  • Files reviewed: 27/28 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread apps/desktop/src/main/maker-host/codex-global-plugins.ts
@MagicLizi

Copy link
Copy Markdown
Contributor

@zqchris 👋 这个 PR 还有 2 条 review conversation 没 resolve(apps/desktop/src/main/maker-host/codex-global-plugins.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。

如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。

Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 12:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

Codex MCP 路由在 provenance 缺失(pluginId 为 undefined)时仍可能被当作目标来源处理,和注释/安全语义的 fail-closed 要求不一致,需先修正后才能合并。

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details
  • Files reviewed: 27/28 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread packages/maker-core/src/agents/codex/index.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 51e8ca383d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/maker-core/src/agents/codex/index.ts
Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 13:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

capability 路由/守卫存在“前缀重叠时按数组顺序误匹配”的确定性 bug,可能导致错误路由或误拦截,需要先修正为最长前缀匹配再评估合入。

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

packages/maker-cc-manager/src/session-registry.ts:1003

  • createToolGuardHooks 里用 Array.find() 选第一个 prefix 命中的 guard,会让结果依赖 toolGuards 的顺序;当存在前缀重叠(例如 serverId 归一化后形成 mcp__x__mcp__x__y__)时,较短前缀可能误拦截更具体的工具来源。建议改为选择“最长前缀匹配”的 guard,避免顺序敏感。
    const toolName = input.tool_name;
    const guard = toolGuards.find((candidate) =>
      toolName.startsWith(candidate.toolNamePrefix),
    );
    if (!guard || guard.invocation === 'auto') return { continue: true };

packages/maker-core/src/types/capability-routing.ts:176

  • findClaudeMcpCapabilityRoute 目前用 overrides.find() 找到“第一个” prefix 命中的 directive;当两个 harness serverId 归一化后的 mcp__<id>__ 前缀存在包含关系时(例如 xx__y),会因为顺序原因把更具体的 server 误判成较短前缀的路由。建议改为扫描并选取最长前缀匹配,保证路由确定性。
  return policy?.overrides.find(
    (directive) =>
      isHarnessOwnedCapabilitySource(directive.source) &&
      directive.source.harness === 'claude-code' &&
      directive.source.surface === 'mcp' &&
  • Files reviewed: 27/28 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e54d18107

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/maker-core/src/agents/codex/index.ts
Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 13:32
Signed-off-by: zqchris <chrisz83@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

目前仍存在已确认的路由安全边界问题需要修复(Codex MCP provenance 反查在多匹配场景下不确定,以及 toolGuards 前缀校验允许 whitespace-only 导致闸门静默失效)。

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

packages/maker-core/src/agents/codex/index.ts:4408

  • mcpServerElicitation 的入参没有 itemId / toolUseId,这里用 (turnId, serverName, tool_name) 去反查 activeToolContexts 时可能命中多个 mcpToolCall(同 turn 内多次调用同一 server/tool 很常见)。当前实现会“取最后一个匹配项”的 pluginId,在多匹配场景下不具备确定性,也可能把本次 elicitation 绑定到错误的 tool call。建议:若出现多个匹配且 pluginId 不一致则直接返回 undefined(让上层按缺少 provenance fail-closed);若一致则返回该值。
      return !Array.isArray(availableDecisions) || availableDecisions.includes('acceptForSession');
    }

    function metaContainsSessionPersist(meta: Record<string, unknown> | null): boolean {
      const persist = meta?.persist;
  • Files reviewed: 27/28 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread packages/maker-cc-manager/src/sdk-handlers.ts
Copilot AI review requested due to automatic review settings July 31, 2026 13:39

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e997fd960

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/desktop/src/main/maker-host/auth-adapters.ts
@MagicLizi

Copy link
Copy Markdown
Contributor

@zqchris 👋 这个 PR 还有 2 条 review conversation 没 resolve(apps/desktop/src/main/maker-host/codex-global-plugins.ts / apps/desktop/src/main/maker-host/auth-adapters.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。

如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。

Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 18:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Human review recommended

Changes span cross-harness security routing, protocol versioning, and daemon-side enforcement across multiple packages, requiring careful human end-to-end review.

Review details

Suppressed comments (3)

packages/maker-core/src/agents/claude-code/capability-routing.ts:151

  • 当 route.invocation 为 "disabled" 时,这里的 permissionDecisionReason 仍提示 "not explicitly selected",与 disabled 的“不可通过显式选择解锁”语义不符,容易误导用户/排障。建议根据 invocation 输出更准确的拒绝原因(disabled vs explicit-only)。
        permissionDecisionReason: route.replacement
          ? `This downstream source was not explicitly selected. Use Cindy capability ${route.replacement.id}.`
          : 'This downstream source was not explicitly selected.',

packages/maker-core/src/agents/claude-code/capability-routing.ts:99

  • 当路由策略的 invocation 为 "disabled" 时,这里的 denialMessage 仍使用 "not explicitly selected" 文案,会让用户误以为通过显式 selector 仍可解锁该来源;但 disabled 语义应是无条件禁用。建议按 invocation 分支生成拒绝文案(explicit-only vs disabled),并在有 replacement 时给出对应提示。

This issue also appears on line 149 of the same file.

        denialMessage: directive.replacement
          ? `This downstream source was not explicitly selected. Use Cindy capability ${directive.replacement.id}.`
          : 'This downstream source was not explicitly selected.',

packages/maker-core/src/agents/claude-code/index.ts:1499

  • capability route 被拒绝时的提示文案目前固定为 "not selected";当 invocation 是 "disabled" 时,这会暗示用户可能通过显式 selector 重新选择该来源,但 disabled 应是不可解锁。建议按 invocation 生成更精确的拒绝原因,并与 PreToolUse guard 的文案保持一致。
          behavior: 'deny',
          message: capabilityRoute.replacement
            ? `This downstream source was not selected. Use Cindy capability ${capabilityRoute.replacement.id}.`
            : 'This downstream source was not selected.',
        };
  • Files reviewed: 28/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 448ca941cd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/maker-core/src/types/capability-routing.ts Outdated
Comment thread apps/desktop/src/main/maker-host/codex-global-plugins.ts Outdated
@MagicLizi

Copy link
Copy Markdown
Contributor

@zqchris 👋 这个 PR 还有 2 条 review conversation 没 resolve(packages/maker-core/src/types/capability-routing.ts / apps/desktop/src/main/maker-host/codex-global-plugins.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。

如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。

Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 19:02

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5906c30eb4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/maker-core/src/types/capability-routing.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

maker-cc-manager 的远端 tool guard 冲突判定在缺失 sourceServerId 时会退化为“不做冲突保护”,可能误拦截非 harness MCP,需要修正后再合入。

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

packages/maker-cc-manager/src/session-registry.ts:1061

  • hasToolGuardMcpPrefixCollision()sourceServerId 缺失时直接返回 false,会让 guard 在存在 MCP 前缀归一化冲突(例如用户/设置 MCP 与 harness 插件 MCP 归一化到同一 mcp__…__ 前缀)时仍继续生效,从而可能误拦截本应走正常权限链的非 harness MCP。由于 validateToolGuards() 允许 sourceServerId 为可选字段,这个分支在真实输入下可触发;更安全的做法是:无论是否提供 sourceServerId,只要 init registry 中存在同前缀的 connected server,就把它视为不可判定并跳过 guard。
function hasToolGuardMcpPrefixCollision(
  guard: QueryToolGuard,
  mcpServerNames: ReadonlySet<string>,
): boolean {
  if (!guard.sourceServerId) return false;
  // SDK init reports names without settings/plugin provenance. Even an exact
  // id may therefore be a user MCP shadowing the harness source.
  for (const serverId of mcpServerNames) {
    if (claudeMcpToolPrefix(serverId) === guard.toolNamePrefix) return true;
  }
  return false;
  • Files reviewed: 28/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@MagicLizi

Copy link
Copy Markdown
Contributor

@zqchris 👋 这个 PR 还有 1 条 review conversation 没 resolve(packages/maker-core/src/types/capability-routing.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。

如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。

Signed-off-by: zqchris <chrisz83@gmail.com>
Copilot AI review requested due to automatic review settings July 31, 2026 19:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Human review recommended

变更跨 maker-core/desktop/远端 cc-manager 协议与执行链路且直接影响权限与能力隔离语义,需要人工复核整体安全边界与回滚风险。

Review details
  • Files reviewed: 28/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@MagicLizi MagicLizi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

审查通过。安全加固设计合理:fail-closed 语义贯穿全路径,explicit-only sources 精确控制工具选择权,用户/项目 Skills 与 MCPs 不受影响,协议版本升级防止旧 daemon 静默忽略 tool guard。2589 行新增测试覆盖路由矩阵、冲突处理、turn 边界和失败模式。25 处危险写法软命中逐条定性为良性(regex 已正确转义、文件操作范围受限于托管目录、crypto 仅用于 overlay 缓存校验)。

Verification: 依赖漏洞扫描不适用(无新增依赖);semgrep 静态扫描未执行(环境未安装);恶意代码与数据外发人工排查通过。

@MagicLizi
MagicLizi merged commit 038b0c3 into main Jul 31, 2026
13 checks passed
@MagicLizi
MagicLizi deleted the xdt/harness-capability-routing branch July 31, 2026 20:43
@MagicLizi

Copy link
Copy Markdown
Contributor

能力路由这层做得扎实——fail-closed 贯穿每条路径,版本协议卡得精准,测试矩阵把冲突、边界和故障模式全覆盖了,给下游 harness 生态留出了安全的扩展空间。

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.

3 participants