Repository navigation
feat(skills): require roles for configured namespaces - #1840
Conversation
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
读了 66d6848c 的源码改动(src/resources/desired.ts、src/types.ts)和中英文指南。实现很集中:roleAllowedSkills 在 resolveDesiredSkills 里先按有效角色筛掉受限命名空间,再交给角色扫描、sharedFirst 和标签匹配。所以项目和标签都绕不过去,字段缺省或为 [] 时行为不变。字段用现有的 NamespaceSegmentSchema 校验。指南写清楚了“团队依赖它之前,所有成员先升级”。
几点意见,都不阻断:
- [P3 可选] 只要有一个有效角色 id 在
manifest/roles.yaml里找不到(比如角色已被删掉),roles.length === ids.length不成立,allowed就是空集,所有受限命名空间都会被跳过,连其它有效角色本该收到的也一样。fail-closed 本身没问题,但提示说的是 “Your effective roles do not receive them”,会让成员以为是自己的角色映射错了。建议点名那个找不到的角色。 - [P3 可选]
warnOnce放在resolveDesiredSkills的路径上,所以除了 pull,teamai doctor、dashboard 的交付视图和 benchmark 的team-without也会打印这条提示,而它的下一步写的是 “thenteamai pull”。如果只想在 pull 时提示,可以把skipped返回给调用方,由 pull 来说。 - [P3 可选] CHANGELOG 这条没写“先升级所有成员”。旧版 CLI 会忽略这个字段,照旧全量下发,正好和团队配置的意图相反。指南里有这句,CHANGELOG 里也值得加上。
CI 和 Codex review 在这个 head 上还在跑。
@jeff-r2026 your call on this one. 这是新的团队配置项(sharing.skills.roleRequiredNamespaces),只管 skills,不管 rules、agents、knowledge,算产品层面的取舍:是否接受这个键名和这个范围,需要你来定。
|
未发现指定 diff 中有充分证据支持的缺陷。 PR 描述已提供隔离 verify world 中的真实 CLI 验证记录,覆盖角色切换、旧副本回收、dry-run、后台 hook 分发及卸载,满足运行时变更的测试记录要求。其余工具、provider 和 Windows 覆盖已明确留给 CI,不构成阻断项。 本次仅阅读指定 diff,未运行、构建或安装 PR 中的代码;未独立复验描述中的测试结果。 |
|
CI 的 7 个 doctor 报的是: 原因在测试 fixture,不在实现。 建议在这 8 处补上 English The 7 doctor reports: The cause is the test fixtures, not the implementation. Add |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
感谢 @YogeLiu。为了推进合并,我在这个分支上推了两个提交:
c10e68a8合并了 main,只有CHANGELOG.md冲突,两边的条目都保留了;c7c36fa0修正了doctor.test.ts的 fixture:10 处sharing: { env: … }补上skills: {},基础的mockTeamConfig补上sharing: { skills: {} }。desired.ts没有改动,也没有加?.。
修正前,合并后的代码上有 7 个用例失败,和 CI 一致。其中 2 个来自没有 sharing 的 mockTeamConfig,所以只改 env fixture 不够。修正后 92 个全部通过。typecheck、lint 和完整单元测试也通过,只有 checkout-roles.test.ts 一个已知用例在路径含 "review" 的克隆里失败,与本 PR 无关。
在 c7c36fa 上批准。之前的三个 [P3 可选] 不阻断合并。CI 和 CI review 在这个 head 上通过后合并。
合并风险: 低。本次推送只改了测试 fixture 和一次 main 合并。
|
未发现指定 diff 中有充分证据支持的缺陷。此前审查未提出具体问题,因此没有需要确认已解决的旧 finding。 PR 描述提供了隔离 verify world 中的真实 CLI 验证记录,覆盖角色切换、旧副本回收、dry-run、后台 hook 分发和卸载,满足运行时变更的测试记录要求。记录基于较早提交不构成阻断;其余工具、provider 和 Windows 覆盖已明确留给 CI 或标注未运行。 本次仅阅读指定 diff(含上下文),未运行、构建或安装 PR 代码,也未独立复验描述中的测试结果。 |
# Conflicts: # CHANGELOG.md
|
未发现指定 diff 中有充分证据支持的缺陷。此前两次审查没有提出具体 finding,因此没有需要确认已解决的旧问题。 PR 描述提供了隔离 verify world 中的真实 CLI 验证记录,覆盖角色切换、旧副本回收、dry-run、后台 hook 分发和卸载,满足运行时变更的测试记录要求。记录基于较早提交不构成阻断;其它工具、provider 和 Windows 的覆盖边界已明确说明。 本次仅阅读指定 diff(含上下文),未运行、构建或安装 PR 代码,也未独立复验描述中的测试结果。 |
未选择角色的成员会走兼容的全量同步路径,收到团队按业务线划分的技能。本 PR 增加
sharing.skills.roleRequiredNamespaces,使指定命名空间必须出现在有效角色的resources.skills中才能分发。成员仍自行执行teamai roles set developer;CLI 不自动选择或分配角色。Closes #1838.
改动
[]时保持原有分发行为。验证
npm run typecheck:通过。npm run lint:通过,包含文档检查。git diff --check:通过。desired-skills、types、workspace-children、pull-namespace-override、pull-tombstone)。roles-assigned-cmd、pull-skip-sync)。合计 254 项通过。真实 CLI / verify world
macOS、Node v22.22.3,基于
origin/main的6ea3ee5db414加本 PR runtime 改动。源码包版本仍为 0.22.0,没有改发布版本。通过npm run verify:world构建、打包并安装到隔离 HOME,使用本地 bare 团队仓库;未调用真实远端 provider,也未更改本机 TeamAI 安装或团队配置。在该 world 中通过
npm run -s verify -- sh <run> ...运行 fixture 和真实teamai命令,逐步断言 Claude 技能文件:pull --forceroles set developer后 pullroles set ebao后pull --dry-run --forceroles set health后 pullhook-dispatch session-start --tool claude --bg-only紧接手动 pull 的首次 hook 正常复用了既有 60 秒刷新冷却,所以没有立即拉取新提交;到期后的重试完成了上述后台下发检查。没有更改冷却实现。
最后重新 init/pull 并运行
npm run -s verify -- ledger <run>,卸载后的LEDGER-2.md结果为:首次 ledger 报出的
home/Library是 fixture 使用系统 Python 生成的com.apple.python缓存。核对后仅将该测试产物移到 world 的 tmp,随后重新验证 init/pull/uninstall;没有修改 ledger 规则或 TeamAI 清理实现。宿主文件检查未报告变更;两个 Unix socket 不可读取,未作验证。覆盖边界