mirror of
https://github.com/Tencent/teamai-cli.git
synced 2026-10-02 03:14:40 +08:00
feat(doctor): run the checks after a pull, and check what actually landed (#625)
* feat(pull): report failing checks at the end of an interactive pull Every line a pull prints reports what it did; none reported what is on disk. That gap is the shape of #574, #525, #342 and friends: "Synced N skills" and the tool receives nothing. An explicit `teamai pull` now re-runs the doctor registry and prints only the checks that failed, with the fix each one already carries. The SessionStart hook path (`pull({ silent: true })`) and `--dry-run` run no checks at all, so session startup is unchanged. Checks now declare `source: 'local' | 'provider'`. The post-pull pass runs the local ones only: the pull just used the provider successfully, so re-probing `gh auth status` would add a subprocess to every sync and prove nothing new. `teamai doctor` still runs the full registry. For #598. * feat(doctor): fail when an enabled tool is not installed buildHookChecks skipped any tool whose settings directory was missing — the same silent skip #574 reports in pull, reproduced inside doctor. With `claude` installed and `codex` not, the report was all green while codex received nothing. A tool listed in `enabledAgents` is the user's own claim that they use it, so it now yields a failing `<tool> is installed` check with a fix that points at `teamai uninstall --agent <tool>`. Without `enabledAgents` the team's tool list is aspirational and an absent tool stays silent, so no existing install grows a new red line. For #598. * refactor(pull): extract resolveDesiredSkills from pullForScope pullForScope computed the desired skill set — role namespaces union the subscribed tags, minus the exclusions — and dropped it when the run ended. The delivery check needs the same set, and re-deriving it there would put that policy in a second place that drifts on its own. The block moves to an exported, read-only resolveDesiredSkills(), called from where it stood. roleContext stays an explicit argument: pullForScope already holds one, and null means "no roles configured", not "not looked up yet". No behaviour change. For #598. * feat(doctor): check that the desired skills actually landed on disk Every other check verifies plumbing — provider CLI, clone, config, hooks, env. None verified the payload, which is what #574, #525, #342 and #372 are actually about: the run reports success and the agent finds nothing. `Skills delivered to <tool>` compares the desired set (role namespaces union subscribed tags, minus exclusions) against what is on disk for each installed tool, and names the skills that are missing. It catches what a write-time gate cannot: per-tool skips, and drift after a correct pull — a directory deleted by hand, a tool reinstalled, a role changed. Destination resolution moves into skillTargetForTool(), so pull writes and doctor checks the same paths, including Codex's shared .agents/skills directory. A tool that is not installed is asked for nothing; enabledAgents covers that case with its own check. For #598. * feat(doctor): report a skill that landed but stays invisible A copy can arrive intact and still never be discovered: SKILL.md deleted, frontmatter that does not parse, or a `name` that does not match its directory (#372's class). The write succeeded, so no write-time gate has anything to report. The delivery check now separates the two causes — "not delivered" from "delivered but unreadable" — and the fix says which one `teamai pull` can repair and which one needs the team repo fixed. For #598. * fix(doctor): check every enabled tool, not only the ones with hooks Hanging "is this tool installed" off the hook registry made it invisible for exactly the tools most likely to be declared and absent: OpenCode and CodeBuddy ship skills and no hook configuration, so `buildHookChecks` returned before the question was ever asked. Found driving the real CLI: `enabledAgents: [claude, opencode]` with no OpenCode root printed nothing. The check moves to its own builder over ctx.toolPaths, and probes a resource path rather than the settings path — resources land under resolveToolBaseDir (the project root in project scope), which is the root a pull would have to write into. For #598. * fix(doctor): survive a team repo it cannot resolve a desired set from Two findings from the review pass. A repo with the same skill in two active namespaces makes scanRoleAwareSkills throw. pullForScope catches it and the post-pull pass catches it, but `teamai doctor` called buildChecks unguarded: the command whose job is explaining bad state stack-traced on it. It now reports a failing "Skills to deliver can be resolved" check carrying the collision. `copilot is installed` could never fail — isToolInstalledForConfig counts Copilot as installed as soon as enabledAgents names it — so that dead check is gone. Copilot's delivery check still reports what did not arrive. Also: pull and doctor now share formatCheckResult instead of two copies of the same glyphs, the timeout message interpolates its constant, and the DesiredSkills block no longer sits between skillSafeToRemove's doc comment and its function. For #598. * docs: document the post-pull checks and the delivery check Both guides, both languages, same positions: the manual-pull block, the doctor section, the exclusion and tag-subscription paragraphs, and the packages-section one-liner that enumerated what doctor checks. No README change: the `teamai doctor` row still describes it, and touching it would cost five synchronized translations. For #598. * feat(doctor): check the team docs bundle landed Docs are the one payload with a single destination instead of one per tool, so the check is a tree comparison rather than a per-tool loop: every non-dot file under the team repo's `docs/` against `sharing.docs.localDir`, with the same filter the copy uses. The destination resolution moves out of DocsHandler.pullItem into resolveDocsDestination(), so pull writes and doctor checks the same directory — including the project-scope rule that a `~/` prefix means the project root, not HOME. Found while validating it: a doc deleted by hand is not restored by the next pull, because the rev fast-path skips the scope. The check is what makes that visible. For #598. * fix(doctor): stop telling people to run the pull they just ran The delivery fixes said "Run `teamai pull`" — printed at the end of a `teamai pull`, and wrong besides: a scope whose team repo has not moved is skipped by the revision fast-path, so a plain pull cannot restore a resource deleted after a correct sync, which is the main case these checks exist to catch. Both fixes now say `teamai pull --force` and why. The underlying gap — that a plain pull does not heal drift — is filed as its own issue. For #598. * fix(pull): do not repeat, at the end of a pull, what the pull already said #621 landed a "Contributed learnings are published" check on the same registry this pass now runs. A pull with a stuck queue therefore said the same thing twice, and contradicted itself doing it: pullForScope warns "run `teamai doctor` for what to check", then the post-pull block answers with the check's own fix, "Run `teamai pull` to publish them" — the pull that had just run. The warning is the better of the two and has to stay: it carries the push error, which the check cannot learn without attempting a push of its own, and `doctor` is a read-only diagnostic. Rewording the fix is no good either, because in `teamai doctor` — where the queue publish runs before the revision fast-path, so a plain pull really is the retry — that advice is correct. So `Check` gains `reportedByPull`, and the post-pull pass skips a check whose topic this run reported. Evidence, not a declaration: pullForScope returns before the publish step when the team repo fails to refresh, and swallows a publish throw into a debug line. On both paths the pull says nothing about the queue, so suppressing the check unconditionally would leave a stuck queue reported by nobody. The topic is a union rather than a boolean for the same reason: the pull proves what it reported by naming it, so a second tagged check cannot be silenced by the first one's evidence. For #598. * fix(doctor): cap the delivery fix's name list, as the docs one already does `nameList` was written for the docs check and used only there, while the delivery check — the one most likely to have a long list, since a fresh machine is missing every skill at once — joined its names unbounded. A member with forty desired skills got all forty pasted into one fix line. Both now go through the helper, which moves above its first caller. Also drops a stray blank line that a rebase left between `skillSafeToRemove`'s docstring and the function, detaching the two. For #598. * fix(skills): stop reporting a Codex conflict nobody can act on `resolveSkillDestination` warns when a skill exists in both `.agents/skills` and `.codex/skills`, unless it can prove the two are identical. That proof needs the team copy, so the check is written as `sourcePath && ...` — and without a sourcePath the guard short-circuits into the warning instead of past it. The read-only callers are the ones that omit it. `teamai remove skills` already did on main; this branch added `buildDeliveryChecks`, so the warning now fires once per skill on every `teamai doctor` and at the end of every pull, for copies the write path silently reconciles. Omitting sourcePath now returns the shared destination before the reconciliation branch, which is what the function's own docstring already promised. The write path is untouched: with a source, an unprovable pair still warns. For #598. * fix(doctor): own the post-pull evidence, bound the whole pass, report both ways Three things a review of the post-pull pass turned up. The set of what a run already said was a module-level `const` cleared at the top of `pull()`, and the topic it held was a union with one member: two pieces of machinery where one value does. `pull()` now owns the set and passes it down. It is a required parameter of `pullForScope` rather than a field on its optional `policy`, because a call site that forgot it would stop recording silently, which is the failure the mechanism exists to prevent. `PullReportedTopic` is gone and `Check.reportedByPull` is a plain string. The 5s budget wrapped `runChecks` only, while the I/O is in `buildChecks`: the delivery checks stat every desired skill for every tool as the registry is built. Both are inside it now. And the pass no longer goes quiet when it gives up — silence after spending the whole budget is the same "reported success, nothing happened" shape these checks exist to catch, so it says one line and points at `teamai doctor`. The reason stays on the debug channel. `<tool> is installed` only pushed a check when it already failed, so an installed tool had no entry at all. `doctor --json` is consumed by hooks and CI, where a missing entry cannot be told apart from one that passed, and no other check in the registry behaves that way. It now reports both ways. Docs in both languages and the CHANGELOG follow, including a note that the checks at the end of a pull cover the scope resolved from the current directory. For #598. * fix(doctor): judge a tool where the sync writes, and keep off a busy clone Three findings from the Codex review. A pull that found a scope's lock held by another process drops that scope from every stage that reads the shared clone, because the other process may have it on a transient branch. The post-pull checks resolve their own context from that same clone and ran anyway, so a diagnostic could report a failure about someone else's work in progress. They now stay out entirely when any scope was contended; `teamai doctor` runs them once the other process is done. `<tool> is installed` probed the tool root while skill delivery asks `skillTargetForTool`, which sends OpenClaw to its workspace directory, Hermes to its home, and Copilot through `enabledAgents`. A `~/.openclaw` with no workspace therefore passed the check while delivery skipped the tool and its delivery check vanished — "reported success, received nothing" inside the command written to catch it. The probe is now `skillsReachTool`, which asks that same resolver; a tool that configures no skills path keeps the generic one, having no such resolver to ask. `Team docs delivered` called `pathExists`, which follows symlinks and says yes to a directory, so a name occupied by something other than the document passed while the document was no more readable than a missing one. It now requires a file. Reading each one would cost more than the job needs on a bundle of hundreds of documents, so this stats rather than reads, and the guides no longer claim the docs check does everything the skills check does. For #598.
This commit is contained in:
Saul Moro
@@ -6,6 +6,8 @@ All notable changes to this project will be documented in this file. See [standa
|
||||
|
||||
### ✨ Features
|
||||
|
||||
- A manual `teamai pull` ends by running the `teamai doctor` checks and printing each one that failed, with its fix. It prints nothing when they all pass, the exit code is unchanged, and the SessionStart hook path (`--silent`) and `--dry-run` run no checks, so session startup is untouched. Provider authentication checks are left to `teamai doctor`: the pull just used the provider. So is any check that pull already reported in its own words on that run — the queued-learnings warning is not immediately repeated as a check telling you to run the pull you just ran. A check the pull stayed silent about is still printed (for [#598](https://github.com/Tencent/teamai-cli/issues/598)).
|
||||
- `teamai doctor` now checks what landed, not only the plumbing. `Skills delivered to <tool>` compares the skills your roles, tag subscriptions and exclusions resolve to against each installed tool's directory, reporting a skill that never arrived separately from one that arrived unreadable (`SKILL.md` missing, unparseable frontmatter, or a `name` that does not match the directory, which keeps the agent from discovering it). `Team docs delivered` does the same for the docs bundle against `sharing.docs.localDir`. `<tool> is installed` fails when `enabledAgents` lists a tool with no directory here, instead of skipping it silently, and reports an installed one as passing so `--json` carries an entry either way. Resolving a skill's destination without a team copy to compare against no longer warns about a Codex shared-directory conflict, so a read-only `doctor` stops reporting one for copies the pull treats as identical. The installed check asks the same resolver the sync uses, so OpenClaw is judged at its workspace directory rather than its tool root. `Team docs delivered` requires each expected document to be a readable file, not merely a name that exists. And a pull that found a scope locked by another process runs no checks at the end, since they would read a clone that process may have mid-write (for [#598](https://github.com/Tencent/teamai-cli/issues/598)).
|
||||
- `teamai remove` accepts `--force` to skip its confirmation prompt, spelled the same way as `teamai uninstall --force`. Without a TTY the prompt answers itself with no, so this is the only way to remove a resource from a script or a test (for [#591](https://github.com/Tencent/teamai-cli/issues/591)).
|
||||
- MCP servers in `mcp/mcp.yaml` and hooks in `hooks/hooks.yaml` accept an optional `roles:` list and ship only to members holding one of those roles; a role change removes the previous role's entries on the next pull, and `teamai mcp list` / `teamai hooks list` show the restriction (for [#563](https://github.com/Tencent/teamai-cli/issues/563)).
|
||||
- Agents can be scoped by role or project: `agents/<namespace>/` ships only to members whose `roles.yaml` / `projects.yaml` entry lists that namespace under a new optional `agents:` key, and a role change removes the previous namespaces' agents on the next pull (for [#563](https://github.com/Tencent/teamai-cli/issues/563)).
|
||||
|
||||
+10
-4
@@ -470,6 +470,8 @@ teamai pull # Manual pull
|
||||
teamai pull --dry-run # Dry run, no actual changes
|
||||
```
|
||||
|
||||
A manual `teamai pull` ends by running the `teamai doctor` checks and printing each one that failed, with its fix — including whether the skills it just reported syncing are readable on disk for every enabled tool. It prints nothing when they all pass, and the exit code is unchanged. The SessionStart hook path and `--dry-run` run no checks at all, so session startup stays as fast as before. Provider checks (`gh`/`gf` authentication) are left to `teamai doctor`: the pull just used the provider.
|
||||
|
||||
> Project scope is isolated by default. When the current working directory contains a project-scope `.teamai/config.yaml`, `pull` processes that project and skips user scope unless the local config has `inheritUserScope: true`; in that case it first refreshes the safe user-resource channel. Without a project config in the current directory, `pull` processes user scope. User `env`, MCP definitions, sources, reporting, and writes remain isolated in project mode. Hooks are the one exception: a project scope's hooks are injected into your **HOME** tool settings (`~/.claude/settings.json`, …), not `<projectRoot>`, because the built-in hooks gate on the `cwd` handed to `hook-dispatch` and `~/.claude` always exists so the "installed tool" gate passes (see the Hooks section). Self single-repo mode keeps its hooks in the business repo so they travel on clone.
|
||||
|
||||
With role-based skills enabled, `pull`'s skill sync source becomes the contents of `skills/<namespace>/`, expanded according to `primaryRole + additionalRoles` and flattened into each local AI tool's skills directory. `rules/` and `docs/` keep their original sync behavior; `agents/<namespace>/` follows the role's `agents` namespaces (see [Agents Resource Type](#agents-resource-type)). `learnings/` at the root is shared with everyone, while `learnings/<project-id>/` subdirectories sync only for the directory's active projects (see [Multi-project](#multi-project-project-as-a-dimension-orthogonal-to-role)).
|
||||
@@ -511,7 +513,7 @@ The existing SessionStart hook runs `teamai pull`. When the `packages` declarati
|
||||
```bash
|
||||
teamai packages # Install every team declaration
|
||||
teamai packages --dry-run # Preview native commands without installing or writing files
|
||||
teamai doctor # Check runtimes and declared package/marketplace/plugin status; exits 1 when any check fails
|
||||
teamai doctor # Check runtimes, declared package/marketplace/plugin status, and what actually landed on disk; exits 1 when any check fails
|
||||
```
|
||||
|
||||
After a successful install, TeamAI writes a local snapshot to `teamai.lock` under the active scope's `.teamai` directory. The lock records installed versions and the declaration hash used by the SessionStart hint; it is not stored in the team repository. In user scope, machine-wide npm tools and Claude plugins are acknowledged once, while project npm dependencies are acknowledged separately for each working directory so installing in one repository cannot silence another repository's hint.
|
||||
@@ -563,7 +565,7 @@ excludedSkills:
|
||||
- using-superpowers
|
||||
```
|
||||
|
||||
Exclusion rules take effect after role and tag filtering. When running `teamai pull`, excluded skills are not synced, and any copies previously installed by `pull` are cleaned up.
|
||||
Exclusion rules take effect after role and tag filtering. When running `teamai pull`, excluded skills are not synced, and any copies previously installed by `pull` are cleaned up. `teamai doctor` checks the resulting set against what is on disk, and asks nothing of an excluded skill.
|
||||
|
||||
### Push local resources
|
||||
|
||||
@@ -670,7 +672,7 @@ teamai tags subscribe frontend testing
|
||||
teamai tags unsubscribe testing
|
||||
```
|
||||
|
||||
Admins can manage resource tags with `teamai tags add` and `teamai tags remove`. Run `teamai pull` after changing your subscriptions; it does a full sync even when the team repo has not changed, so newly matched resources are installed and unsubscribed ones are removed.
|
||||
Admins can manage resource tags with `teamai tags add` and `teamai tags remove`. Run `teamai pull` after changing your subscriptions; it does a full sync even when the team repo has not changed, so newly matched resources are installed and unsubscribed ones are removed. The checks at the end of that pull verify the newly matched skills reached every enabled tool.
|
||||
|
||||
---
|
||||
|
||||
@@ -1509,7 +1511,11 @@ teamai remove mcp <name>
|
||||
teamai remove rules <name> --force # Skip the prompt, for scripts and CI
|
||||
```
|
||||
|
||||
`teamai doctor` exits with code 0 only when every check passes, and code 1 when any check fails. Before initialization, it reports the missing configuration without assuming a Git provider.
|
||||
`teamai doctor` exits with code 0 only when every check passes, and code 1 when any check fails. Before initialization, it reports the missing configuration without assuming a Git provider. The same checks run at the end of a manual `teamai pull`, minus the provider ones and minus any check that pull already reported in its own words on that run.
|
||||
|
||||
Besides the provider, clone, config, hook and env checks, `doctor` verifies three things about what reached your machine. `<tool> is installed` fails when `enabledAgents` lists a tool that nothing would be delivered to, which is the case where a pull reports success and that tool receives nothing. It asks the same resolver the sync uses, so a tool that keeps its skills somewhere other than its tool root, as OpenClaw does with its workspace directory, is judged where the sync would actually write. It reports an installed tool as passing too, so `--json` carries one entry per enabled tool either way. The checks at the end of a pull cover the scope that pull resolved from the current directory; run `teamai doctor` in another scope to check that one. `Skills delivered to <tool>` compares the skills your role namespaces, tag subscriptions and exclusions resolve to against what is on disk for each installed tool: it reports a skill that was never delivered separately from one that arrived unreadable — `SKILL.md` missing, its frontmatter unparseable, or its `name` not matching the directory, which keeps the agent from ever discovering it. `Team docs delivered` compares the docs bundle against `sharing.docs.localDir`, which has one destination rather than one per tool; each expected document has to be a file that can be read, so a directory or a dangling link sitting on the name counts as missing. Rules, agents and MCP servers are not checked yet.
|
||||
|
||||
`Contributed learnings are published` fails while `teamai contribute` has notes queued that could not be pushed. A manual `teamai pull` does not repeat it at the end when the pull has already said it: the pull tries to publish the queue and reports the outcome itself, with the push error that made it fail — more than this check can tell you. If the pull never got that far, because the team repo failed to refresh, the check is printed as usual.
|
||||
|
||||
`--json` prints the same report as one object on stdout and routes every log line to stderr, so `teamai doctor --json 2>/dev/null` parses whole. The exit code is unchanged. Each check carries the fix suggestion it prints in human mode:
|
||||
|
||||
|
||||
@@ -448,6 +448,8 @@ teamai pull # 手动拉取
|
||||
teamai pull --dry-run # 试运行，不实际修改
|
||||
```
|
||||
|
||||
手动执行 `teamai pull` 会在结束时运行 `teamai doctor` 的检查，并逐条打印失败项及其修复建议——包括它刚刚报告同步的 skill 是否真的落到每个启用工具的磁盘上、且可被读取。全部通过时不会有任何额外输出，退出码也不变。SessionStart hook 路径和 `--dry-run` 完全不运行检查，会话启动速度保持不变。托管平台相关的检查（`gh`/`gf` 认证）留给 `teamai doctor`：这次 pull 刚刚用过该平台。
|
||||
|
||||
> Project scope 默认与 user scope 隔离。当前工作目录包含 project scope 的 `.teamai/config.yaml` 时，`pull` 会处理该项目并跳过 user scope；仅当本地配置包含 `inheritUserScope: true` 时，才会先刷新安全的 user 资源通道。当前目录没有 project 配置时，`pull` 处理 user scope。project 模式下，user 的 `env`、MCP 定义、sources、reporting 和写入行为仍保持隔离。hooks 是唯一例外：project scope 的 hooks 会注入到你的 **HOME** 工具设置（`~/.claude/settings.json` 等），而非 `<projectRoot>`——因为内置 hooks 依据传给 `hook-dispatch` 的 `cwd` 门控，且 `~/.claude` 恒存在、能通过「已安装工具」门槛（详见 Hooks 章节）。self 单仓模式则把 hooks 保留在业务仓库里，随 clone 传播。
|
||||
|
||||
启用角色化 skills 后，`pull` 的 skills 同步来源会变成 `skills/<namespace>/` 中的内容，按 `primaryRole + additionalRoles` 展开对应的 namespace，拍平安装到本地各 AI 工具 skills 目录。`rules/`、`docs/` 仍然保持原有同步逻辑；`agents/<namespace>/` 按角色的 `agents` namespace 同步（见 [Agents 资源类型](#agents-资源类型)）。`learnings/` 根目录对所有人共享，而 `learnings/<project-id>/` 子目录只对本目录激活的项目同步（见 [多项目](#多项目project-作为与-role-正交的维度)）。
|
||||
@@ -489,7 +491,7 @@ Claude 插件 target 使用 `plugin@marketplace` 格式。`claude-plugins-offici
|
||||
```bash
|
||||
teamai packages # 安装团队声明的全部包和插件
|
||||
teamai packages --dry-run # 预览底层命令，不安装也不写文件
|
||||
teamai doctor # 检查运行环境及声明的包、marketplace、插件状态；任一检查失败时退出码为 1
|
||||
teamai doctor # 检查运行环境、声明的包/marketplace/插件状态，以及磁盘上实际落地的资源；任一检查失败时退出码为 1
|
||||
```
|
||||
|
||||
安装成功后，TeamAI 会在当前 scope 的 `.teamai` 目录下写入本地快照 `teamai.lock`。该文件记录已安装版本，以及供 SessionStart 提示比对的声明哈希，不会写入团队仓库。在 user scope 下，全局 npm 工具和 Claude 插件只需确认一次；项目 npm 依赖会按工作目录分别确认，避免在一个仓库安装后错误关闭另一个仓库的提示。
|
||||
@@ -541,7 +543,7 @@ excludedSkills:
|
||||
- using-superpowers
|
||||
```
|
||||
|
||||
排除规则在角色和标签过滤之后生效。执行 `teamai pull` 时，被排除的 skill 不会同步，并且会清理由之前 pull 安装的副本。
|
||||
排除规则在角色和标签过滤之后生效。执行 `teamai pull` 时，被排除的 skill 不会同步，并且会清理由之前 pull 安装的副本。`teamai doctor` 会把最终结果集与磁盘实际内容比对，并且不会要求被排除的 skill 存在。
|
||||
|
||||
### 推送本地资源
|
||||
|
||||
@@ -643,7 +645,7 @@ teamai tags subscribe frontend testing
|
||||
teamai tags unsubscribe testing
|
||||
```
|
||||
|
||||
管理员可通过 `teamai tags add` 和 `teamai tags remove` 管理资源标签。修改订阅后运行 `teamai pull`，即使团队仓库没有变化也会执行全量同步，新匹配的资源会被安装，取消订阅的资源会被清理。
|
||||
管理员可通过 `teamai tags add` 和 `teamai tags remove` 管理资源标签。修改订阅后运行 `teamai pull`，即使团队仓库没有变化也会执行全量同步，新匹配的资源会被安装，取消订阅的资源会被清理。该次 pull 结束时的检查会确认新匹配的 skill 已送达每个启用的工具。
|
||||
|
||||
---
|
||||
|
||||
@@ -1469,7 +1471,11 @@ teamai remove mcp <name>
|
||||
teamai remove rules <name> --force # 跳过确认，用于脚本和 CI
|
||||
```
|
||||
|
||||
仅当所有检查通过时，`teamai doctor` 才以状态码 0 退出；任一检查失败时以状态码 1 退出。尚未初始化时，它只报告缺少配置，不会臆测 Git 托管平台。
|
||||
仅当所有检查通过时，`teamai doctor` 才以状态码 0 退出；任一检查失败时以状态码 1 退出。尚未初始化时，它只报告缺少配置，不会臆测 Git 托管平台。手动执行 `teamai pull` 结束时会运行同一批检查（不含托管平台相关的检查，也不含本次 pull 已经自行报告过的检查）。
|
||||
|
||||
除了托管平台、clone、配置、hook 和 env 检查之外，`doctor` 还会验证落到本机上的三件事。`<tool> is installed` 在 `enabledAgents` 列出了不会收到任何内容的工具时失败——这正是 pull 报告成功、而该工具什么都没收到的情况。它使用与同步相同的解析逻辑，因此像 OpenClaw 这样把 skills 放在 workspace 目录而非工具根目录的工具，会在同步真正写入的位置被判断。工具已安装时也会作为通过项报告，因此 `--json` 无论哪种情况都会为每个已启用工具给出一条记录。pull 结束时的检查只覆盖它从当前目录解析出的那个 scope；其他 scope 请在对应目录下运行 `teamai doctor`。`Skills delivered to <tool>` 会把角色命名空间、标签订阅与排除规则解析出的 skill 集合，与每个已安装工具磁盘上的内容比对：从未送达的 skill 与送达但不可读的 skill 会分别报告——后者指 `SKILL.md` 缺失、frontmatter 无法解析，或其 `name` 与目录名不一致，导致 agent 永远发现不了它。`Team docs delivered` 将 docs 包与 `sharing.docs.localDir` 比对（它只有一个目标目录，而非每个工具一个）；每个应有的文档都必须是可读取的文件，因此占用了该名字的目录或断链接也算缺失。rules、agents 和 MCP server 目前尚未检查。
|
||||
|
||||
`Contributed learnings are published` 会在 `teamai contribute` 写下、但尚未推送成功的笔记仍在队列中时失败。当本次 pull 已经说过时，手动 `teamai pull` 结束时不会再重复它：pull 会尝试发布队列并自行报告结果，还会带上导致失败的推送错误——这是该检查本身给不出的信息。如果 pull 因为团队仓库刷新失败而根本没走到那一步，该检查会照常打印。
|
||||
|
||||
`--json` 把同一份报告作为单个对象打印到 stdout，并将所有日志改走 stderr，因此 `teamai doctor --json 2>/dev/null` 可以整体解析；退出码不变。每个检查都会带上人类模式下显示的修复建议：
|
||||
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import fse from 'fs-extra';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
import { resolveDesiredSkills } from '../pull.js';
|
||||
import type { RolePullContext } from '../pull.js';
|
||||
import type { LocalConfig, TeamaiConfig } from '../types.js';
|
||||
|
||||
/**
|
||||
* The desired set is what `pull` installs and what `doctor` checks landed. Both
|
||||
* read it from here, so this is the one place role namespaces, tag subscriptions
|
||||
* and exclusions are combined (#598).
|
||||
*/
|
||||
describe('resolveDesiredSkills', () => {
|
||||
let tempDir: string;
|
||||
let repoPath: string;
|
||||
let localConfig: LocalConfig;
|
||||
let teamConfig: TeamaiConfig;
|
||||
|
||||
/** A role context activating the given skill namespaces. */
|
||||
function rolesOver(namespaces: string[]): RolePullContext {
|
||||
return {
|
||||
activeNamespaces: { knowledge: [], skills: namespaces, learnings: [], agents: [] },
|
||||
activeSkillNames: new Set(),
|
||||
inactiveSkillNames: new Set(),
|
||||
inactiveSkillSources: new Map(),
|
||||
};
|
||||
}
|
||||
|
||||
async function writeSkill(namespace: string, name: string): Promise<void> {
|
||||
const dir = path.join(repoPath, 'skills', namespace, name);
|
||||
await fse.ensureDir(dir);
|
||||
await fse.writeFile(path.join(dir, 'SKILL.md'), `---\nname: ${name}\ndescription: d\n---\n`);
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-desired-skills-'));
|
||||
repoPath = path.join(tempDir, 'team-repo');
|
||||
|
||||
await writeSkill('common', 'shared-skill');
|
||||
await writeSkill('backend', 'backend-skill');
|
||||
await writeSkill('frontend', 'frontend-skill');
|
||||
await fse.writeFile(
|
||||
path.join(repoPath, 'tags.yaml'),
|
||||
'skills:\n frontend-skill: [ui]\n backend-skill: [server]\n',
|
||||
);
|
||||
|
||||
localConfig = {
|
||||
repo: { localPath: repoPath, remote: 'owner/repo' },
|
||||
username: 'tester',
|
||||
scope: 'user',
|
||||
additionalRoles: [],
|
||||
};
|
||||
teamConfig = {
|
||||
team: 'test',
|
||||
description: '',
|
||||
repo: 'owner/repo',
|
||||
provider: 'github',
|
||||
reviewers: [],
|
||||
sharing: {
|
||||
skills: {}, rules: { enforced: [] }, docs: { localDir: '' }, env: { injectShellProfile: true },
|
||||
},
|
||||
toolPaths: { claude: { skills: '.claude/skills' } },
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await fse.remove(tempDir);
|
||||
});
|
||||
|
||||
it('takes the union of the active role namespaces and the subscribed tags', async () => {
|
||||
localConfig.subscribedTags = ['ui'];
|
||||
|
||||
const { items } = await resolveDesiredSkills(teamConfig, localConfig, rolesOver(['common']));
|
||||
|
||||
expect(items.map((i) => i.name).sort()).toEqual(['frontend-skill', 'shared-skill']);
|
||||
});
|
||||
|
||||
it('drops excluded skills from the union', async () => {
|
||||
localConfig.subscribedTags = ['ui'];
|
||||
localConfig.excludedSkills = ['frontend-skill'];
|
||||
|
||||
const { items } = await resolveDesiredSkills(teamConfig, localConfig, rolesOver(['common']));
|
||||
|
||||
expect(items.map((i) => i.name)).toEqual(['shared-skill']);
|
||||
});
|
||||
|
||||
it('without a role context, every skill in the repo is desired', async () => {
|
||||
const { items } = await resolveDesiredSkills(teamConfig, localConfig, null);
|
||||
|
||||
expect(items.map((i) => i.name).sort())
|
||||
.toEqual(['backend-skill', 'frontend-skill', 'shared-skill']);
|
||||
});
|
||||
|
||||
it('reports the whole team repo separately, so cleanup knows what it may prune', async () => {
|
||||
const { items, teamItems, skippedByTags } = await resolveDesiredSkills(
|
||||
teamConfig,
|
||||
localConfig,
|
||||
rolesOver(['common']),
|
||||
);
|
||||
|
||||
expect(items.map((i) => i.name)).toEqual(['shared-skill']);
|
||||
expect(teamItems.map((i) => i.name).sort())
|
||||
.toEqual(['backend-skill', 'frontend-skill', 'shared-skill']);
|
||||
expect(teamItems.every((i) => i.sourcePath.startsWith(repoPath))).toBe(true);
|
||||
// No subscriptions active, so nothing was filtered out by the tag channel.
|
||||
expect(skippedByTags).toBe(0);
|
||||
});
|
||||
|
||||
it('counts what the tag filter left out', async () => {
|
||||
localConfig.subscribedTags = ['ui'];
|
||||
|
||||
const { skippedByTags } = await resolveDesiredSkills(teamConfig, localConfig, rolesOver(['common']));
|
||||
|
||||
// backend-skill carries a tag the user is not subscribed to.
|
||||
expect(skippedByTags).toBe(1);
|
||||
});
|
||||
|
||||
it('writes nothing: doctor calls it on a machine it must not change', async () => {
|
||||
const before = await fse.readdir(tempDir);
|
||||
|
||||
await resolveDesiredSkills(teamConfig, localConfig, rolesOver(['common']));
|
||||
|
||||
expect(await fse.readdir(tempDir)).toEqual(before);
|
||||
expect(await fse.pathExists(path.join(tempDir, '.claude'))).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,396 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import fse from 'fs-extra';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
vi.mock('../config.js', () => ({
|
||||
detectProjectConfig: vi.fn().mockResolvedValue(null),
|
||||
loadLocalConfig: vi.fn(),
|
||||
loadTeamConfig: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/logger.js', () => ({
|
||||
log: {
|
||||
debug: vi.fn(), error: vi.fn(), info: vi.fn(), success: vi.fn(), warn: vi.fn(), dim: vi.fn(),
|
||||
},
|
||||
setStderrOnly: vi.fn(),
|
||||
}));
|
||||
|
||||
import { loadLocalConfig, loadTeamConfig } from '../config.js';
|
||||
import { log } from '../utils/logger.js';
|
||||
import { buildChecks, resolveDoctorContext, type Check } from '../doctor.js';
|
||||
import type { LocalConfig, TeamaiConfig } from '../types.js';
|
||||
|
||||
/**
|
||||
* The delivery check: what `pull` said it synced, against what an agent can
|
||||
* actually read on disk (#598). Everything else in the registry verifies
|
||||
* plumbing; this one verifies the payload.
|
||||
*/
|
||||
describe('doctor — skills delivered on disk', () => {
|
||||
let tempDir: string;
|
||||
let homeDir: string;
|
||||
let repoPath: string;
|
||||
let localConfig: LocalConfig;
|
||||
let teamConfig: TeamaiConfig;
|
||||
|
||||
const CLAUDE_SKILLS = ['.claude', 'skills'];
|
||||
|
||||
async function writeTeamSkill(name: string): Promise<void> {
|
||||
const dir = path.join(repoPath, 'skills', name);
|
||||
await fse.ensureDir(dir);
|
||||
await fse.writeFile(path.join(dir, 'SKILL.md'), `---\nname: ${name}\ndescription: d\n---\n`);
|
||||
}
|
||||
|
||||
/** A correctly delivered copy, the way pullItem leaves one. */
|
||||
async function deliver(segments: string[], name: string): Promise<void> {
|
||||
const dir = path.join(homeDir, ...segments, name);
|
||||
await fse.ensureDir(dir);
|
||||
await fse.writeFile(path.join(dir, 'SKILL.md'), `---\nname: ${name}\ndescription: d\n---\n`);
|
||||
}
|
||||
|
||||
async function deliveryCheck(tool = 'claude'): Promise<Check> {
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
const check = (await buildChecks(ctx)).find((c) => c.name === `Skills delivered to ${tool}`);
|
||||
if (!check) throw new Error(`no delivery check for ${tool}`);
|
||||
return check;
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-delivery-'));
|
||||
homeDir = path.join(tempDir, 'home');
|
||||
repoPath = path.join(tempDir, 'team-repo');
|
||||
vi.stubEnv('HOME', homeDir);
|
||||
|
||||
await writeTeamSkill('alpha');
|
||||
await writeTeamSkill('beta');
|
||||
await fse.ensureDir(path.join(homeDir, ...CLAUDE_SKILLS));
|
||||
|
||||
localConfig = {
|
||||
repo: { localPath: repoPath, remote: 'owner/repo' },
|
||||
username: 'tester',
|
||||
scope: 'user',
|
||||
additionalRoles: [],
|
||||
};
|
||||
teamConfig = {
|
||||
team: 'test',
|
||||
description: '',
|
||||
repo: 'owner/repo',
|
||||
provider: 'git',
|
||||
reviewers: [],
|
||||
sharing: {
|
||||
skills: {}, rules: { enforced: [] }, docs: { localDir: '' },
|
||||
env: { injectShellProfile: false },
|
||||
},
|
||||
toolPaths: { claude: { skills: '.claude/skills' } },
|
||||
};
|
||||
|
||||
vi.mocked(loadLocalConfig).mockResolvedValue(localConfig);
|
||||
vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.unstubAllEnvs();
|
||||
vi.clearAllMocks();
|
||||
await fse.remove(tempDir);
|
||||
});
|
||||
|
||||
it('passes when every desired skill is on disk', async () => {
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
await deliver(CLAUDE_SKILLS, 'beta');
|
||||
|
||||
const check = await deliveryCheck();
|
||||
|
||||
expect(await check.check()).toBe(true);
|
||||
expect(check.source).toBe('local');
|
||||
});
|
||||
|
||||
it('fails and names the skill the tool never received', async () => {
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
|
||||
const check = await deliveryCheck();
|
||||
|
||||
expect(await check.check()).toBe(false);
|
||||
expect(check.fix).toContain('beta');
|
||||
expect(check.fix).not.toContain('alpha');
|
||||
// Not a plain `teamai pull`: this check is printed at the end of one, and a
|
||||
// scope whose team repo has not moved is skipped, so it cannot restore this.
|
||||
expect(check.fix).toContain('teamai pull --force');
|
||||
});
|
||||
|
||||
it('counts the rest rather than printing every name', async () => {
|
||||
// A fresh machine is missing everything. The fix is a line a human reads,
|
||||
// not the whole desired set pasted into the terminal.
|
||||
for (let i = 0; i < 9; i += 1) await writeTeamSkill(`extra-${i}`);
|
||||
|
||||
const check = await deliveryCheck();
|
||||
|
||||
expect(await check.check()).toBe(false);
|
||||
expect(check.fix).toContain('and 6 more');
|
||||
expect(check.fix).not.toContain('extra-8');
|
||||
});
|
||||
|
||||
it('does not warn about a Codex conflict while only reading', async () => {
|
||||
// buildDeliveryChecks resolves destinations without a sourcePath. Codex's
|
||||
// shared-directory reconciliation needs the team copy to prove two copies
|
||||
// are identical, so without one there is nothing to decide: a read-only
|
||||
// `doctor` must not report a conflict the write path would have settled.
|
||||
teamConfig.toolPaths = { codex: { skills: '.codex/skills' } };
|
||||
await fse.ensureDir(path.join(homeDir, '.codex', 'skills'));
|
||||
for (const name of ['alpha', 'beta']) {
|
||||
await deliver(['.agents', 'skills'], name);
|
||||
await deliver(['.codex', 'skills'], name);
|
||||
}
|
||||
|
||||
const check = await deliveryCheck('codex');
|
||||
|
||||
expect(await check.check()).toBe(true);
|
||||
expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('Codex skill conflict'));
|
||||
});
|
||||
|
||||
it('asks the skills write path whether a tool is installed', async () => {
|
||||
// OpenClaw lives at its workspace directory, not at the tool root. A tool
|
||||
// root with no workspace passes a generic probe while skill delivery skips
|
||||
// the tool entirely, which is "reported success, received nothing" inside
|
||||
// the command whose job is to catch it (#598).
|
||||
teamConfig.toolPaths = { openclaw: { skills: '.openclaw/skills' } };
|
||||
localConfig.enabledAgents = ['openclaw'];
|
||||
await fse.ensureDir(path.join(homeDir, '.openclaw'));
|
||||
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
const installed = (await buildChecks(ctx)).find((c) => c.name === 'openclaw is installed');
|
||||
|
||||
expect(installed).toBeDefined();
|
||||
expect(await installed!.check()).toBe(false);
|
||||
});
|
||||
|
||||
it('passes once that tool\'s workspace is there', async () => {
|
||||
teamConfig.toolPaths = { openclaw: { skills: '.openclaw/skills' } };
|
||||
localConfig.enabledAgents = ['openclaw'];
|
||||
await fse.ensureDir(path.join(homeDir, '.openclaw', 'workspace', 'skills'));
|
||||
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
const installed = (await buildChecks(ctx)).find((c) => c.name === 'openclaw is installed');
|
||||
|
||||
expect(installed).toBeDefined();
|
||||
expect(await installed!.check()).toBe(true);
|
||||
});
|
||||
|
||||
it('reports each installed tool separately', async () => {
|
||||
teamConfig.toolPaths = {
|
||||
claude: { skills: '.claude/skills' },
|
||||
codex: { skills: '.codex/skills' },
|
||||
};
|
||||
await fse.ensureDir(path.join(homeDir, '.codex', 'skills'));
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
await deliver(CLAUDE_SKILLS, 'beta');
|
||||
await deliver(['.codex', 'skills'], 'alpha');
|
||||
|
||||
expect(await (await deliveryCheck('claude')).check()).toBe(true);
|
||||
expect(await (await deliveryCheck('codex')).check()).toBe(false);
|
||||
});
|
||||
|
||||
it('counts a codex skill in the shared .agents/skills directory as delivered', async () => {
|
||||
teamConfig.toolPaths = { codex: { skills: '.codex/skills' } };
|
||||
await fse.ensureDir(path.join(homeDir, '.codex', 'skills'));
|
||||
await deliver(['.agents', 'skills'], 'alpha');
|
||||
await deliver(['.agents', 'skills'], 'beta');
|
||||
|
||||
expect(await (await deliveryCheck('codex')).check()).toBe(true);
|
||||
});
|
||||
|
||||
it('asks nothing of a tool that is not installed', async () => {
|
||||
teamConfig.toolPaths = {
|
||||
claude: { skills: '.claude/skills' },
|
||||
codex: { skills: '.codex/skills' },
|
||||
};
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
await deliver(CLAUDE_SKILLS, 'beta');
|
||||
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
const names = (await buildChecks(ctx)).map((c) => c.name);
|
||||
|
||||
expect(names).toContain('Skills delivered to claude');
|
||||
expect(names).not.toContain('Skills delivered to codex');
|
||||
});
|
||||
|
||||
it('asks nothing of an excluded skill', async () => {
|
||||
localConfig.excludedSkills = ['beta'];
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
|
||||
expect(await (await deliveryCheck()).check()).toBe(true);
|
||||
});
|
||||
|
||||
// The write succeeded, so no write-time gate has anything to report — and the
|
||||
// agent still never discovers the skill (#372's class).
|
||||
describe('delivered but invisible', () => {
|
||||
it('fails when SKILL.md is gone', async () => {
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
await fse.ensureDir(path.join(homeDir, ...CLAUDE_SKILLS, 'beta'));
|
||||
|
||||
const check = await deliveryCheck();
|
||||
|
||||
expect(await check.check()).toBe(false);
|
||||
expect(check.fix).toContain('beta');
|
||||
expect(check.fix).toMatch(/unreadable/i);
|
||||
});
|
||||
|
||||
it('fails when the frontmatter does not parse', async () => {
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
const beta = path.join(homeDir, ...CLAUDE_SKILLS, 'beta');
|
||||
await fse.ensureDir(beta);
|
||||
await fse.writeFile(path.join(beta, 'SKILL.md'), '---\nname: [oops\n---\n# beta\n');
|
||||
|
||||
expect(await (await deliveryCheck()).check()).toBe(false);
|
||||
});
|
||||
|
||||
it('fails when the frontmatter name does not match the directory', async () => {
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
const beta = path.join(homeDir, ...CLAUDE_SKILLS, 'beta');
|
||||
await fse.ensureDir(beta);
|
||||
await fse.writeFile(path.join(beta, 'SKILL.md'), '---\nname: renamed\ndescription: d\n---\n');
|
||||
|
||||
const check = await deliveryCheck();
|
||||
|
||||
expect(await check.check()).toBe(false);
|
||||
expect(check.fix).toContain('beta');
|
||||
});
|
||||
|
||||
it('separates what was never delivered from what is unreadable', async () => {
|
||||
const beta = path.join(homeDir, ...CLAUDE_SKILLS, 'beta');
|
||||
await fse.ensureDir(beta);
|
||||
await fse.writeFile(path.join(beta, 'SKILL.md'), '---\nname: renamed\n---\n');
|
||||
|
||||
const check = await deliveryCheck();
|
||||
const fix = check.fix ?? '';
|
||||
|
||||
// alpha never arrived; beta arrived broken. Same tool, different cause.
|
||||
expect(fix.indexOf('alpha')).toBeGreaterThan(-1);
|
||||
expect(fix.indexOf('beta')).toBeGreaterThan(-1);
|
||||
expect(fix).toMatch(/not delivered/i);
|
||||
expect(fix).toMatch(/unreadable/i);
|
||||
});
|
||||
|
||||
it('accepts extra frontmatter fields', async () => {
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
const beta = path.join(homeDir, ...CLAUDE_SKILLS, 'beta');
|
||||
await fse.ensureDir(beta);
|
||||
await fse.writeFile(
|
||||
path.join(beta, 'SKILL.md'),
|
||||
'---\nname: beta\ndescription: d\nallowed-tools: [Read]\nversion: 2\n---\n',
|
||||
);
|
||||
|
||||
expect(await (await deliveryCheck()).check()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// Docs are the one payload with a single destination instead of one per tool:
|
||||
// DocsHandler copies the whole bundle into `sharing.docs.localDir`.
|
||||
describe('team docs', () => {
|
||||
const CHECK = 'Team docs delivered';
|
||||
|
||||
async function writeTeamDoc(...segments: string[]): Promise<void> {
|
||||
const file = path.join(repoPath, 'docs', ...segments);
|
||||
await fse.ensureDir(path.dirname(file));
|
||||
await fse.writeFile(file, '# doc\n');
|
||||
}
|
||||
|
||||
async function docsCheck(): Promise<Check | undefined> {
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
return (await buildChecks(ctx)).find((c) => c.name === CHECK);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
teamConfig.sharing.docs.localDir = '~/team-docs';
|
||||
});
|
||||
|
||||
it('passes when the bundle is on disk', async () => {
|
||||
await writeTeamDoc('guide.md');
|
||||
await writeTeamDoc('api', 'reference.md');
|
||||
await fse.ensureDir(path.join(homeDir, 'team-docs', 'api'));
|
||||
await fse.writeFile(path.join(homeDir, 'team-docs', 'guide.md'), '# doc\n');
|
||||
await fse.writeFile(path.join(homeDir, 'team-docs', 'api', 'reference.md'), '# doc\n');
|
||||
|
||||
const check = await docsCheck();
|
||||
|
||||
expect(check).toBeDefined();
|
||||
expect(await check!.check()).toBe(true);
|
||||
});
|
||||
|
||||
it('fails and names what is missing from the bundle', async () => {
|
||||
await writeTeamDoc('guide.md');
|
||||
await writeTeamDoc('api', 'reference.md');
|
||||
await fse.ensureDir(path.join(homeDir, 'team-docs'));
|
||||
await fse.writeFile(path.join(homeDir, 'team-docs', 'guide.md'), '# doc\n');
|
||||
|
||||
const check = await docsCheck();
|
||||
|
||||
expect(await check!.check()).toBe(false);
|
||||
expect(check!.fix).toContain('api/reference.md');
|
||||
expect(check!.fix).not.toContain('guide.md');
|
||||
expect(check!.fix).toContain('teamai pull --force');
|
||||
});
|
||||
|
||||
it('does not accept a directory sitting on a doc\'s name', async () => {
|
||||
// pathExists follows symlinks and says yes to a directory, so on its own
|
||||
// it cannot tell a delivered document from a name occupied by something
|
||||
// else. The bundle is no more readable than if the file were missing.
|
||||
await writeTeamDoc('guide.md');
|
||||
await fse.ensureDir(path.join(homeDir, 'team-docs', 'guide.md'));
|
||||
|
||||
const check = await docsCheck();
|
||||
|
||||
expect(check).toBeDefined();
|
||||
expect(await check!.check()).toBe(false);
|
||||
expect(check!.fix).toContain('guide.md');
|
||||
});
|
||||
|
||||
it('asks nothing when the team repo ships no docs', async () => {
|
||||
expect(await docsCheck()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// The command whose job is reporting bad state must not stack-trace on it.
|
||||
it('reports a team repo it cannot resolve a desired set from, instead of throwing', async () => {
|
||||
// The same skill in two active role namespaces: scanRoleAwareSkills throws.
|
||||
localConfig.primaryRole = 'dev';
|
||||
await fse.ensureDir(path.join(repoPath, 'manifest'));
|
||||
await fse.writeFile(path.join(repoPath, 'manifest', 'roles.yaml'), [
|
||||
'version: 1',
|
||||
'roles:',
|
||||
' - id: dev',
|
||||
' resources:',
|
||||
' knowledge: []',
|
||||
' skills: [one, two]',
|
||||
' learnings: []',
|
||||
' agents: []',
|
||||
'',
|
||||
].join('\n'));
|
||||
for (const ns of ['one', 'two']) {
|
||||
const dir = path.join(repoPath, 'skills', ns, 'clash');
|
||||
await fse.ensureDir(dir);
|
||||
await fse.writeFile(path.join(dir, 'SKILL.md'), '---\nname: clash\n---\n');
|
||||
}
|
||||
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
const checks = await buildChecks(ctx);
|
||||
|
||||
const resolution = checks.find((c) => c.name === 'Skills to deliver can be resolved');
|
||||
expect(resolution).toBeDefined();
|
||||
expect(await resolution!.check()).toBe(false);
|
||||
expect(resolution!.fix).toContain('clash');
|
||||
});
|
||||
|
||||
it('never writes to the tool directory it inspects', async () => {
|
||||
await deliver(CLAUDE_SKILLS, 'alpha');
|
||||
|
||||
await (await deliveryCheck()).check();
|
||||
|
||||
expect(await fse.readdir(path.join(homeDir, ...CLAUDE_SKILLS))).toEqual(['alpha']);
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,12 @@ vi.mock('../config.js', () => ({
|
||||
vi.mock('../utils/fs.js', () => ({
|
||||
pathExists: vi.fn(),
|
||||
readFileSafe: vi.fn(),
|
||||
// The delivery checks walk the team repo through resolveDesiredSkills and
|
||||
// DocsHandler. This machine has neither skills nor docs; delivery on a real
|
||||
// disk is covered by doctor-delivery.test.ts.
|
||||
listDirs: vi.fn().mockResolvedValue([]),
|
||||
listFilesRecursive: vi.fn().mockResolvedValue([]),
|
||||
expandHome: vi.fn((p: string) => p),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/logger.js', () => ({
|
||||
@@ -91,7 +97,12 @@ beforeEach(() => {
|
||||
mockedLoadLocalConfig.mockResolvedValue(mockLocalConfig);
|
||||
mockedLoadTeamConfig.mockResolvedValue(mockTeamConfig);
|
||||
mockedPathExists.mockResolvedValue(true);
|
||||
mockedReadFileSafe.mockResolvedValue(buildFullHooksContent());
|
||||
// One blob answers every read, except the role/project manifests the
|
||||
// delivery check resolves the desired skill set from: parsing hook JSON as a
|
||||
// manifest throws. Absent manifests are the shape this fixture wants anyway.
|
||||
mockedReadFileSafe.mockImplementation(async (filePath: string) => (
|
||||
filePath.includes(`${path.sep}manifest${path.sep}`) ? null : buildFullHooksContent()
|
||||
));
|
||||
});
|
||||
|
||||
// ── Tests ────────────────────────────────────────────────
|
||||
@@ -568,5 +579,120 @@ describe('buildChecks', () => {
|
||||
const check = (await buildChecks(ctx)).find((c) => c.name.includes('learnings'));
|
||||
expect(check).toBeDefined();
|
||||
expect(check?.fix).toContain('teamai pull');
|
||||
// Correct advice here, where doctor is the whole command. The pull's own
|
||||
// warning already says it, with the push error, so the post-pull pass
|
||||
// skips this one rather than repeat it — see pull-post-checks.test.ts.
|
||||
expect(check?.reportedByPull).toBe('pending-learnings');
|
||||
});
|
||||
|
||||
it('flags only the queue check as one the pull reports itself', async () => {
|
||||
mockedLoadLocalConfig.mockResolvedValue(mockLocalConfig);
|
||||
mockedLoadTeamConfig.mockResolvedValue(mockTeamConfig);
|
||||
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
|
||||
const flagged = (await buildChecks(ctx)).filter((c) => c.reportedByPull);
|
||||
expect(flagged.map((c) => c.name)).toEqual(['Contributed learnings are published']);
|
||||
});
|
||||
});
|
||||
|
||||
// A tool listed in enabledAgents is a claim by the user that they use it. Until
|
||||
// #598 the registry answered that claim with silence: buildHookChecks skipped
|
||||
// any tool whose settings directory was missing — the same silent skip #574
|
||||
// reports in pull, reproduced inside doctor.
|
||||
describe('buildChecks — a tool enabled but not installed', () => {
|
||||
const twoToolPaths = {
|
||||
claude: { settings: '.claude/settings.json', skills: '.claude/skills' },
|
||||
codex: { settings: '.codex/hooks.json', skills: '.codex/skills' },
|
||||
};
|
||||
|
||||
/** Everything exists except codex's settings directory. */
|
||||
function onlyCodexMissing(): void {
|
||||
mockedPathExists.mockImplementation(async (filePath: string) => !filePath.includes('.codex'));
|
||||
}
|
||||
|
||||
async function checksFor(localOverrides: Record<string, unknown>) {
|
||||
mockedLoadLocalConfig.mockResolvedValue({ ...mockLocalConfig, ...localOverrides });
|
||||
mockedLoadTeamConfig.mockResolvedValue({ ...mockTeamConfig, toolPaths: twoToolPaths });
|
||||
onlyCodexMissing();
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
return buildChecks(ctx);
|
||||
}
|
||||
|
||||
it('fails for a tool that carries no hook configuration at all', async () => {
|
||||
// opencode ships skills and nothing else. Hanging this check off the hook
|
||||
// registry made it invisible for exactly the tools most likely to be
|
||||
// declared and absent.
|
||||
mockedLoadLocalConfig.mockResolvedValue({
|
||||
...mockLocalConfig,
|
||||
enabledAgents: ['claude', 'opencode'],
|
||||
});
|
||||
mockedLoadTeamConfig.mockResolvedValue({
|
||||
...mockTeamConfig,
|
||||
toolPaths: {
|
||||
claude: { settings: '.claude/settings.json', skills: '.claude/skills' },
|
||||
opencode: { skills: '.opencode/skills' },
|
||||
},
|
||||
});
|
||||
mockedPathExists.mockImplementation(async (filePath: string) => !filePath.includes('.opencode'));
|
||||
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) throw new Error('expected a resolved doctor context');
|
||||
const opencode = (await buildChecks(ctx)).find((c) => c.name === 'opencode is installed');
|
||||
|
||||
expect(opencode).toBeDefined();
|
||||
expect(await opencode!.check()).toBe(false);
|
||||
});
|
||||
|
||||
it('reports an installed tool as passing rather than omitting it', async () => {
|
||||
// `doctor --json` is consumed by hooks and CI. A check that only appears
|
||||
// when it fails cannot be told apart from one that was never evaluated,
|
||||
// and no other check in the registry behaves that way.
|
||||
const checks = await checksFor({ enabledAgents: ['claude', 'codex'] });
|
||||
|
||||
const claude = checks.find((c) => c.name === 'claude is installed');
|
||||
expect(claude).toBeDefined();
|
||||
expect(await claude!.check()).toBe(true);
|
||||
});
|
||||
|
||||
it('fails a check naming the tool the user enabled', async () => {
|
||||
const checks = await checksFor({ enabledAgents: ['claude', 'codex'] });
|
||||
|
||||
const codex = checks.find((c) => c.name === 'codex is installed');
|
||||
expect(codex).toBeDefined();
|
||||
expect(await codex!.check()).toBe(false);
|
||||
expect(codex!.source).toBe('local');
|
||||
expect(codex!.fix).toContain('enabledAgents');
|
||||
});
|
||||
|
||||
it('stays silent about an uninstalled tool nobody enabled', async () => {
|
||||
const checks = await checksFor({});
|
||||
|
||||
expect(checks.map((c) => c.name)).not.toContain('codex is installed');
|
||||
// And no other check stands in for it: an unlisted tool is simply absent.
|
||||
expect(checks.map((c) => c.name)).not.toContain('teamai hooks in codex settings');
|
||||
});
|
||||
|
||||
it('reaches the JSON report with the shape every check has', async () => {
|
||||
mockedLoadLocalConfig.mockResolvedValue({
|
||||
...mockLocalConfig,
|
||||
enabledAgents: ['claude', 'codex'],
|
||||
});
|
||||
mockedLoadTeamConfig.mockResolvedValue({
|
||||
...mockTeamConfig,
|
||||
toolPaths: twoToolPaths,
|
||||
sharing: { env: { injectShellProfile: false } },
|
||||
});
|
||||
onlyCodexMissing();
|
||||
|
||||
const allPassed = await doctor({ json: true });
|
||||
|
||||
const report = JSON.parse(String(consoleSpy.mock.calls[0][0])) as DoctorReport;
|
||||
const codex = report.checks.find((c) => c.name === 'codex is installed');
|
||||
expect(codex).toMatchObject({ name: 'codex is installed', ok: false });
|
||||
expect(codex?.fix).toBeTruthy();
|
||||
expect(allPassed).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,300 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import fse from 'fs-extra';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
vi.mock('../config.js', () => ({
|
||||
detectProjectConfig: vi.fn().mockResolvedValue(null),
|
||||
loadLocalConfigForScope: vi.fn(),
|
||||
loadStateForScope: vi.fn().mockResolvedValue({ lastPull: null, lastPullRev: null }),
|
||||
loadTeamConfig: vi.fn(),
|
||||
requireInit: vi.fn(),
|
||||
saveStateForScope: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/git.js', () => ({
|
||||
getHeadRev: vi.fn().mockResolvedValue('abc1234'),
|
||||
pullRepo: vi.fn().mockResolvedValue('already up to date'),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/logger.js', () => ({
|
||||
log: {
|
||||
debug: vi.fn(), error: vi.fn(), info: vi.fn(), success: vi.fn(), warn: vi.fn(), dim: vi.fn(),
|
||||
},
|
||||
spinner: vi.fn(() => ({
|
||||
fail: vi.fn().mockReturnThis(), info: vi.fn().mockReturnThis(),
|
||||
start: vi.fn().mockReturnThis(), stop: vi.fn().mockReturnThis(),
|
||||
succeed: vi.fn().mockReturnThis(), warn: vi.fn().mockReturnThis(),
|
||||
})),
|
||||
}));
|
||||
|
||||
vi.mock('../roles.js', () => ({
|
||||
loadRolesManifest: vi.fn().mockResolvedValue({
|
||||
version: 1,
|
||||
roles: [{
|
||||
id: 'dev',
|
||||
name: 'Dev',
|
||||
description: '',
|
||||
resources: { knowledge: ['common'], skills: ['common'], learnings: ['common'], agents: [] },
|
||||
}],
|
||||
defaults: { shareTarget: 'primary-role' },
|
||||
}),
|
||||
resolveRoleResourceNamespaces: vi.fn(() => ({
|
||||
knowledge: ['common'], skills: ['common'], learnings: ['common'], agents: [],
|
||||
})),
|
||||
}));
|
||||
|
||||
// Isolation: pull() takes a real ~/.teamai/.sync-lock. Parallel vitest workers
|
||||
// sharing that path race and skip/error, so these tests mock the lock.
|
||||
vi.mock('../update.js', () => ({
|
||||
acquireLock: vi.fn().mockResolvedValue(true),
|
||||
releaseLock: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
// The registry itself is exercised in doctor.test.ts. Here the subject is the
|
||||
// wiring: which checks pull runs, and what it prints. runChecks stays real so
|
||||
// the test proves a provider check is never *invoked*, not merely not printed.
|
||||
vi.mock('../doctor.js', async (importOriginal) => ({
|
||||
...await importOriginal<typeof import('../doctor.js')>(),
|
||||
resolveDoctorContext: vi.fn(),
|
||||
buildChecks: vi.fn(),
|
||||
}));
|
||||
|
||||
import { detectProjectConfig, loadLocalConfigForScope, loadTeamConfig } from '../config.js';
|
||||
import { acquireLock } from '../update.js';
|
||||
import { buildChecks, resolveDoctorContext, type Check, type DoctorContext } from '../doctor.js';
|
||||
import { log } from '../utils/logger.js';
|
||||
import { pull } from '../pull.js';
|
||||
import type { LocalConfig, TeamaiConfig } from '../types.js';
|
||||
|
||||
/** Every line pull printed, in order, as one string. */
|
||||
function printedOutput(): string {
|
||||
const calls = [
|
||||
...vi.mocked(log.warn).mock.calls,
|
||||
...vi.mocked(log.info).mock.calls,
|
||||
...vi.mocked(log.dim).mock.calls,
|
||||
...vi.mocked(log.success).mock.calls,
|
||||
];
|
||||
return calls.map((c) => String(c[0])).join('\n');
|
||||
}
|
||||
|
||||
describe('checks at the end of an interactive pull', () => {
|
||||
let tempDir: string;
|
||||
let homeDir: string;
|
||||
let repoPath: string;
|
||||
let ctx: DoctorContext;
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-pull-checks-'));
|
||||
homeDir = path.join(tempDir, 'home');
|
||||
repoPath = path.join(tempDir, 'team-repo');
|
||||
vi.stubEnv('HOME', homeDir);
|
||||
|
||||
await fse.ensureDir(path.join(repoPath, 'skills', 'common', 'kept-skill'));
|
||||
await fse.writeFile(
|
||||
path.join(repoPath, 'skills', 'common', 'kept-skill', 'SKILL.md'),
|
||||
'---\nname: kept-skill\ndescription: kept\n---\n',
|
||||
);
|
||||
await fse.ensureDir(path.join(repoPath, 'manifest'));
|
||||
await fse.writeFile(path.join(repoPath, 'manifest', 'roles.yaml'), 'version: 1\n');
|
||||
await fse.ensureDir(path.join(homeDir, '.claude', 'skills'));
|
||||
|
||||
const localConfig: LocalConfig = {
|
||||
repo: { localPath: repoPath, remote: 'owner/repo' },
|
||||
username: 'tester',
|
||||
scope: 'user',
|
||||
primaryRole: 'dev',
|
||||
additionalRoles: [],
|
||||
};
|
||||
const teamConfig: TeamaiConfig = {
|
||||
team: 'test',
|
||||
description: '',
|
||||
repo: 'owner/repo',
|
||||
provider: 'github',
|
||||
reviewers: [],
|
||||
sharing: {
|
||||
skills: {}, rules: { enforced: [] }, docs: { localDir: '' }, env: { injectShellProfile: true },
|
||||
},
|
||||
toolPaths: { claude: { skills: '.claude/skills', rules: '.claude/rules' } },
|
||||
};
|
||||
|
||||
vi.mocked(detectProjectConfig).mockResolvedValue(null);
|
||||
vi.mocked(loadLocalConfigForScope).mockResolvedValue(localConfig);
|
||||
vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig);
|
||||
|
||||
ctx = {
|
||||
localConfig,
|
||||
teamConfig,
|
||||
toolPaths: teamConfig.toolPaths,
|
||||
baseDir: homeDir,
|
||||
};
|
||||
vi.mocked(resolveDoctorContext).mockResolvedValue(ctx);
|
||||
vi.mocked(buildChecks).mockResolvedValue([]);
|
||||
// clearAllMocks resets calls, not implementations, so a test that makes the
|
||||
// lock contended would otherwise leak into the next one.
|
||||
vi.mocked(acquireLock).mockResolvedValue(true);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.unstubAllEnvs();
|
||||
vi.clearAllMocks();
|
||||
await fse.remove(tempDir);
|
||||
});
|
||||
|
||||
it('prints each failing check with its fix', async () => {
|
||||
vi.mocked(buildChecks).mockResolvedValue([
|
||||
{ name: 'Team repo exists locally', source: 'local', check: async () => true },
|
||||
{
|
||||
name: 'teamai hooks in claude settings',
|
||||
source: 'local',
|
||||
check: async () => false,
|
||||
fix: 'Run `teamai hooks inject` to inject/update hooks',
|
||||
},
|
||||
]);
|
||||
|
||||
await pull({ force: true });
|
||||
|
||||
const output = printedOutput();
|
||||
expect(output).toContain('teamai hooks in claude settings');
|
||||
expect(output).toContain('Run `teamai hooks inject` to inject/update hooks');
|
||||
// Passing checks stay out of the way: pull is not a diagnostics report.
|
||||
expect(output).not.toContain('Team repo exists locally');
|
||||
});
|
||||
|
||||
it('prints nothing when every check passes', async () => {
|
||||
vi.mocked(buildChecks).mockResolvedValue([
|
||||
{ name: 'Team repo exists locally', source: 'local', check: async () => true },
|
||||
]);
|
||||
|
||||
await pull({ force: true });
|
||||
|
||||
expect(printedOutput()).not.toContain('Team repo exists locally');
|
||||
});
|
||||
|
||||
it('never probes the provider: those checks are not even run', async () => {
|
||||
const providerCheck = vi.fn().mockResolvedValue(false);
|
||||
vi.mocked(buildChecks).mockResolvedValue([
|
||||
{
|
||||
name: 'gh CLI is authenticated',
|
||||
source: 'provider',
|
||||
check: providerCheck,
|
||||
fix: 'Run `gh auth login` to authenticate',
|
||||
},
|
||||
]);
|
||||
|
||||
await pull({ force: true });
|
||||
|
||||
expect(providerCheck).not.toHaveBeenCalled();
|
||||
expect(printedOutput()).not.toContain('gh CLI is authenticated');
|
||||
});
|
||||
|
||||
/** The registry's queue check, as buildChecks builds it. */
|
||||
function queueCheck(check: Check['check']): Check {
|
||||
return {
|
||||
name: 'Contributed learnings are published',
|
||||
source: 'local',
|
||||
reportedByPull: 'pending-learnings',
|
||||
check,
|
||||
fix: 'Run `teamai pull` to publish them.',
|
||||
};
|
||||
}
|
||||
|
||||
it('skips a check the pull reported itself on this run', async () => {
|
||||
// A queue entry the pull will fail to publish: remote is a bare path that
|
||||
// is not a repo, so publishQueuedLearnings comes back with remaining > 0
|
||||
// and pullForScope warns, with the push error attached.
|
||||
await fse.outputFile(
|
||||
path.join(tempDir, 'pending-learnings', 'stuck.md'),
|
||||
'---\ntitle: stuck\n---\nbody\n',
|
||||
);
|
||||
const check = vi.fn().mockResolvedValue(false);
|
||||
vi.mocked(buildChecks).mockResolvedValue([queueCheck(check)]);
|
||||
|
||||
await pull({ force: true });
|
||||
|
||||
expect(printedOutput()).toContain('not published');
|
||||
// Repeating it would tell the member to run the pull they just ran, in
|
||||
// weaker words: the warning carries the push error, the fix cannot.
|
||||
expect(check).not.toHaveBeenCalled();
|
||||
expect(printedOutput()).not.toContain('Contributed learnings are published');
|
||||
});
|
||||
|
||||
it('still reports a flagged check when the pull said nothing about it', async () => {
|
||||
// Nothing queued, so pullForScope never warns. A scope that aborts before
|
||||
// the publish step lands here too. The flag must not silence the check on
|
||||
// a run where the pull has not spoken \u2014 nobody else would.
|
||||
const check = vi.fn().mockResolvedValue(false);
|
||||
vi.mocked(buildChecks).mockResolvedValue([queueCheck(check)]);
|
||||
|
||||
await pull({ force: true });
|
||||
|
||||
expect(check).toHaveBeenCalled();
|
||||
expect(printedOutput()).toContain('Contributed learnings are published');
|
||||
});
|
||||
|
||||
it('bounds building the registry, not only running it', async () => {
|
||||
// buildChecks is where the I/O is: the delivery checks stat every desired
|
||||
// skill for every tool while the registry is built. A build that never
|
||||
// settles must still end the pull, and say so rather than go quiet.
|
||||
// Real timers, because faking them stalls the pull's own filesystem work.
|
||||
// This costs one budget's wall clock, which is why there is only one.
|
||||
vi.mocked(buildChecks).mockImplementation(() => new Promise(() => {}));
|
||||
|
||||
await expect(pull({ force: true })).resolves.toBeUndefined();
|
||||
|
||||
expect(printedOutput()).toContain('Post-pull checks did not run');
|
||||
// The sync itself still happened.
|
||||
expect(await fse.pathExists(path.join(homeDir, '.claude', 'skills', 'kept-skill'))).toBe(true);
|
||||
}, 20_000);
|
||||
|
||||
it('says nothing extra when the registry cannot be built at all', async () => {
|
||||
vi.mocked(buildChecks).mockRejectedValue(new Error('registry exploded'));
|
||||
|
||||
await expect(pull({ force: true })).resolves.toBeUndefined();
|
||||
|
||||
expect(printedOutput()).toContain('Post-pull checks did not run');
|
||||
});
|
||||
|
||||
it('runs no checks when another process holds a scope lock', async () => {
|
||||
// A contended scope is dropped from every stage that reads the shared clone,
|
||||
// because the other process may have it on a transient branch. The checks
|
||||
// resolve their own context from that same clone, so running them here is
|
||||
// how a diagnostic invents a failure about someone else's work in progress.
|
||||
vi.mocked(acquireLock).mockResolvedValue(false);
|
||||
vi.mocked(buildChecks).mockResolvedValue([
|
||||
{ name: 'Team repo exists locally', source: 'local', check: async () => false },
|
||||
]);
|
||||
|
||||
await pull({ force: true });
|
||||
|
||||
expect(buildChecks).not.toHaveBeenCalled();
|
||||
expect(printedOutput()).not.toContain('Team repo exists locally');
|
||||
});
|
||||
|
||||
it('runs no checks on the silent hook path', async () => {
|
||||
await pull({ force: true, silent: true });
|
||||
|
||||
expect(resolveDoctorContext).not.toHaveBeenCalled();
|
||||
expect(buildChecks).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('runs no checks on a dry run', async () => {
|
||||
await pull({ dryRun: true });
|
||||
|
||||
expect(resolveDoctorContext).not.toHaveBeenCalled();
|
||||
expect(buildChecks).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('a check that throws does not fail the pull', async () => {
|
||||
const failing: Check = {
|
||||
name: 'explodes',
|
||||
source: 'local',
|
||||
check: async () => { throw new Error('boom'); },
|
||||
};
|
||||
vi.mocked(buildChecks).mockResolvedValue([failing]);
|
||||
|
||||
await expect(pull({ force: true })).resolves.toBeUndefined();
|
||||
// The sync itself still happened.
|
||||
expect(await fse.pathExists(path.join(homeDir, '.claude', 'skills', 'kept-skill'))).toBe(true);
|
||||
});
|
||||
});
|
||||
+271
-13
@@ -1,8 +1,9 @@
|
||||
import path from 'node:path';
|
||||
import { detectProjectConfig, loadLocalConfig, loadTeamConfig } from './config.js';
|
||||
import { pathExists, readFileSafe } from './utils/fs.js';
|
||||
import fs from 'node:fs';
|
||||
import { expandHome, listFilesRecursive, pathExists, readFileSafe } from './utils/fs.js';
|
||||
import { log, setStderrOnly } from './utils/logger.js';
|
||||
import type { GlobalOptions } from './types.js';
|
||||
import type { GlobalOptions, ResourceItem } from './types.js';
|
||||
import {
|
||||
COPILOT_TOOL_ID,
|
||||
TEAMAI_ENV_START,
|
||||
@@ -15,11 +16,34 @@ import {
|
||||
type TeamaiConfig,
|
||||
} from './types.js';
|
||||
import { isToolInstalledForConfig } from './resources/base.js';
|
||||
import { skillsReachTool } from './resources/skills.js';
|
||||
import { splitFrontmatter } from './utils/frontmatter.js';
|
||||
import { TEAMAI_HOOK_SUBCOMMANDS, isCodexTrustGatedTool, codexTrustReminder } from './hooks.js';
|
||||
import { getUserHome } from './utils/home.js';
|
||||
|
||||
/**
|
||||
* Where a check gets its answer. `provider` checks shell out to a provider CLI
|
||||
* or the network; `local` checks only read this machine. Callers that run the
|
||||
* registry outside `teamai doctor` filter on it — see the post-pull pass in
|
||||
* `pull()`, which has just used the provider successfully and must not pay for
|
||||
* an auth probe on every sync.
|
||||
*/
|
||||
export type CheckSource = 'local' | 'provider';
|
||||
|
||||
export interface Check {
|
||||
name: string;
|
||||
source: CheckSource;
|
||||
/**
|
||||
* Names something `teamai pull` says in its own words, better than a static
|
||||
* `fix` can: the queue warning carries the push error, which `doctor` cannot
|
||||
* learn without attempting a push of its own, and a read-only diagnostic must
|
||||
* not. The post-pull pass drops a check whose topic that run actually
|
||||
* reported. Not every run does: a scope whose team repo fails to refresh
|
||||
* returns before the publish step, and a publish that throws is swallowed
|
||||
* into a debug line. On those paths nobody has spoken, so the check is the
|
||||
* only voice left and must be heard.
|
||||
*/
|
||||
reportedByPull?: string;
|
||||
check: () => Promise<boolean>;
|
||||
fix?: string;
|
||||
}
|
||||
@@ -63,8 +87,67 @@ export interface DoctorReport {
|
||||
}
|
||||
|
||||
/**
|
||||
* Build hook checks only for tools whose settings parent directory already
|
||||
* exists (i.e. the tool is installed). Tools that are not installed are skipped.
|
||||
* Check that every tool the team declares and the user enabled is actually here.
|
||||
* Scope note: the loop is over `ctx.toolPaths`, already narrowed to the enabled,
|
||||
* non-excluded agents, so a name in `enabledAgents` that `teamai.yaml` declares
|
||||
* no paths for is out of scope — nothing would be written to it either way.
|
||||
*
|
||||
* That list is the user's own claim that they use the tool, and every writer —
|
||||
* skills, rules, agents, hooks — silently skips a tool whose root is missing.
|
||||
* Answering the claim with silence reproduces inside `doctor` the skip #574
|
||||
* reports in `pull`: "Synced N" while the tool receives nothing. Without
|
||||
* `enabledAgents` the team's tool list is aspirational, so an absent tool stays
|
||||
* silent, as it always has.
|
||||
*
|
||||
* The probe uses a resource path rather than the settings path: resources land
|
||||
* under `resolveToolBaseDir` (the project root in project scope), which is the
|
||||
* root a pull would have to write into.
|
||||
*/
|
||||
async function buildEnabledToolChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
const { localConfig, toolPaths } = ctx;
|
||||
if (!localConfig.enabledAgents) return [];
|
||||
|
||||
const checks: Check[] = [];
|
||||
for (const [tool, paths] of Object.entries(toolPaths)) {
|
||||
// Copilot counts itself installed as soon as enabledAgents names it
|
||||
// (isToolInstalledForConfig), so this check could never fail for it. Its
|
||||
// delivery check still reports what did not arrive.
|
||||
if (tool === COPILOT_TOOL_ID) continue;
|
||||
|
||||
const probePath = paths.skills ?? paths.rules ?? paths.agents ?? paths.settings ?? paths.hooks;
|
||||
if (!probePath) continue;
|
||||
|
||||
// A tool that receives skills is asked the way the skills write path asks:
|
||||
// OpenClaw lives at its workspace directory, not at the tool root, so the
|
||||
// generic probe passes for a `~/.openclaw` with no workspace while delivery
|
||||
// silently skips it — the same "reported success, received nothing" this
|
||||
// check exists to catch. A tool with no skills path (rules only) has no
|
||||
// such resolver, so it keeps the generic probe.
|
||||
const skillsPath = paths.skills;
|
||||
const isInstalled = skillsPath
|
||||
? (): Promise<boolean> => skillsReachTool(tool, skillsPath, localConfig)
|
||||
: (): Promise<boolean> => isToolInstalledForConfig(tool, probePath, localConfig);
|
||||
|
||||
// Pushed whether or not it passes. Every other check in the registry
|
||||
// reports both ways, and `doctor --json` is consumed by hooks and CI, where
|
||||
// a missing entry cannot be told apart from one that passed.
|
||||
checks.push({
|
||||
name: `${tool} is installed`,
|
||||
source: 'local',
|
||||
check: isInstalled,
|
||||
fix: `enabledAgents lists ${tool}, but it has no directory under `
|
||||
+ `${resolveToolBaseDir(tool, localConfig)}, so a pull delivers nothing to it. `
|
||||
+ `Install ${tool} (in project scope, opening a session there creates its root), `
|
||||
+ `or run \`teamai uninstall --agent ${tool}\` to stop syncing to it.`,
|
||||
});
|
||||
}
|
||||
|
||||
return checks;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build hook checks for tools whose settings parent directory already exists
|
||||
* (i.e. the tool is installed). Tools that are not installed are skipped.
|
||||
*/
|
||||
async function buildHookChecks(
|
||||
toolPaths: TeamaiConfig['toolPaths'],
|
||||
@@ -84,9 +167,12 @@ async function buildHookChecks(
|
||||
const installed = tool === COPILOT_TOOL_ID
|
||||
? await isToolInstalledForConfig(tool, paths.hooks ?? paths.settings ?? '', localConfig)
|
||||
: await pathExists(parentDir);
|
||||
// An uninstalled tool has no hooks to check. Whether it should be installed
|
||||
// at all is a different question — see buildEnabledToolChecks.
|
||||
if (!installed) continue;
|
||||
checks.push({
|
||||
name: `teamai hooks in ${tool} settings`,
|
||||
source: 'local',
|
||||
check: async () => {
|
||||
if (!await pathExists(settingsPath)) return false;
|
||||
const content = await readFileSafe(settingsPath);
|
||||
@@ -103,6 +189,158 @@ async function buildHookChecks(
|
||||
return checks;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a delivered skill directory is one an agent can actually discover:
|
||||
* SKILL.md present, frontmatter parses, and its `name` is the directory's own.
|
||||
* A copy that fails this landed successfully — no write-time gate can see it.
|
||||
*/
|
||||
async function skillIsDiscoverable(skillDir: string, skillName: string): Promise<boolean> {
|
||||
const content = await readFileSafe(path.join(skillDir, 'SKILL.md'));
|
||||
if (!content) return false;
|
||||
|
||||
const { data, valid } = splitFrontmatter(content);
|
||||
if (!valid) return false;
|
||||
return data.name === skillName;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `filePath` is a file something can actually read. `pathExists`
|
||||
* follows symlinks but says yes to a directory too, so on its own it cannot
|
||||
* tell a delivered document from a name occupied by something else.
|
||||
*/
|
||||
async function isReadableFile(filePath: string): Promise<boolean> {
|
||||
try {
|
||||
return (await fs.promises.stat(expandHome(filePath))).isFile();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** At most this many names in a fix string; the rest are counted. */
|
||||
const MAX_NAMED_IN_FIX = 5;
|
||||
|
||||
/** `a, b, c and 4 more` — a fix a human reads, not a wall of paths. */
|
||||
function nameList(names: string[]): string {
|
||||
if (names.length <= MAX_NAMED_IN_FIX) return names.join(', ');
|
||||
const shown = names.slice(0, MAX_NAMED_IN_FIX).join(', ');
|
||||
return `${shown} and ${names.length - MAX_NAMED_IN_FIX} more`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build one delivery check per installed tool: every skill the member should
|
||||
* have, against what is actually on disk for that tool.
|
||||
*
|
||||
* This is the only check that looks at the payload rather than the plumbing. A
|
||||
* write-time gate cannot cover it — `SkillsHandler.pullItem` skips each
|
||||
* uninstalled tool on its own, and a directory deleted by hand after a correct
|
||||
* pull leaves every gate happy (#598).
|
||||
*
|
||||
* The scan runs here rather than inside `check()` because the fix names the
|
||||
* skills that are missing, and a `Check`'s fix is read as it was built.
|
||||
*/
|
||||
async function buildDeliveryChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
const { localConfig, teamConfig, toolPaths } = ctx;
|
||||
if (!teamConfig) return [];
|
||||
|
||||
// Dynamic: pull.ts imports this module for its post-pull pass, and the desired
|
||||
// set is policy that must not be restated here.
|
||||
const { buildRolePullContext, resolveDesiredSkills } = await import('./pull.js');
|
||||
const { skillTargetForTool } = await import('./resources/skills.js');
|
||||
|
||||
let items: ResourceItem[];
|
||||
try {
|
||||
const roleContext = await buildRolePullContext(localConfig);
|
||||
({ items } = await resolveDesiredSkills(teamConfig, localConfig, roleContext));
|
||||
} catch (e) {
|
||||
// A team repo whose active namespaces collide cannot say what should be
|
||||
// delivered — `pull` aborts the scope with this same message. The command
|
||||
// whose job is explaining bad state must report it, not stack-trace on it.
|
||||
return [{
|
||||
name: 'Skills to deliver can be resolved',
|
||||
source: 'local',
|
||||
check: async () => false,
|
||||
fix: `${(e as Error).message}. Until the team repo is fixed, `
|
||||
+ 'pull cannot sync skills for this role.',
|
||||
}];
|
||||
}
|
||||
if (items.length === 0) return [];
|
||||
|
||||
const checks: Check[] = [];
|
||||
for (const [tool, paths] of Object.entries(toolPaths)) {
|
||||
const skillsPath = paths.skills;
|
||||
if (!skillsPath) continue;
|
||||
|
||||
const missing: string[] = [];
|
||||
const unreadable: string[] = [];
|
||||
let installed = true;
|
||||
for (const item of items) {
|
||||
const dest = await skillTargetForTool(tool, skillsPath, localConfig, item.name);
|
||||
if (!dest) {
|
||||
// Not installed. Nothing was promised to this tool, so nothing is owed;
|
||||
// a tool the user listed in enabledAgents is caught by its own check.
|
||||
installed = false;
|
||||
break;
|
||||
}
|
||||
if (!await pathExists(dest)) missing.push(item.name);
|
||||
else if (!await skillIsDiscoverable(dest, item.name)) unreadable.push(item.name);
|
||||
}
|
||||
if (!installed) continue;
|
||||
|
||||
const problems: string[] = [];
|
||||
if (missing.length > 0) problems.push(`not delivered: ${nameList(missing)}`);
|
||||
if (unreadable.length > 0) problems.push(`delivered but unreadable: ${nameList(unreadable)}`);
|
||||
|
||||
checks.push({
|
||||
name: `Skills delivered to ${tool}`,
|
||||
source: 'local',
|
||||
check: async () => problems.length === 0,
|
||||
fix: `In ${tool}, ${problems.join('; ')}. Run \`teamai pull --force\`: a plain pull `
|
||||
+ 'skips a scope whose team repo has not changed, so it cannot restore this. '
|
||||
+ 'If a skill stays unreadable, fix its SKILL.md in the team repo — the '
|
||||
+ 'frontmatter needs a `name` matching the directory, or the agent never '
|
||||
+ 'discovers it.',
|
||||
});
|
||||
}
|
||||
|
||||
return checks;
|
||||
}
|
||||
|
||||
/**
|
||||
* The docs bundle has one destination rather than one per tool: `DocsHandler`
|
||||
* copies the whole `docs/` tree into `sharing.docs.localDir`. So this check
|
||||
* compares the two trees, file by file, rather than asking each tool.
|
||||
*/
|
||||
async function buildDocsCheck(ctx: DoctorContext): Promise<Check[]> {
|
||||
const { localConfig, teamConfig } = ctx;
|
||||
if (!teamConfig) return [];
|
||||
|
||||
const { DocsHandler, resolveDocsDestination } = await import('./resources/docs.js');
|
||||
const handler = new DocsHandler();
|
||||
const [item] = await handler.scanTeamForPull(teamConfig, localConfig);
|
||||
if (!item) return [];
|
||||
|
||||
const dest = resolveDocsDestination(teamConfig, localConfig);
|
||||
const teamFiles = (await listFilesRecursive(item.sourcePath))
|
||||
// Same filter DocsHandler.pullItem copies with: dotfiles never travel.
|
||||
.filter((file) => file.split('/').every((segment) => !segment.startsWith('.')));
|
||||
|
||||
// isFile, not merely "something is there": a directory sitting on the
|
||||
// expected name, or a symlink with nothing behind it, would satisfy a plain
|
||||
// existence check while the doc is no more readable than a missing one.
|
||||
const missing: string[] = [];
|
||||
for (const file of teamFiles) {
|
||||
if (!await isReadableFile(path.join(dest, file))) missing.push(file);
|
||||
}
|
||||
|
||||
return [{
|
||||
name: 'Team docs delivered',
|
||||
source: 'local',
|
||||
check: async () => missing.length === 0,
|
||||
fix: `Missing from ${dest}: ${nameList(missing)}. Run \`teamai pull --force\`: a plain `
|
||||
+ 'pull skips a scope whose team repo has not changed, so it cannot restore these.',
|
||||
}];
|
||||
}
|
||||
|
||||
/**
|
||||
* True if a trust-gated Codex tool (the public `codex`) already has teamai hooks
|
||||
* installed on disk (settings file exists and contains the hook-dispatch
|
||||
@@ -162,11 +400,13 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
checks.push(
|
||||
{
|
||||
name: 'gf CLI is installed',
|
||||
source: 'provider',
|
||||
check: async () => isGfInstalled(),
|
||||
fix: 'Run `teamai init` to install gf CLI automatically',
|
||||
},
|
||||
{
|
||||
name: 'gf CLI is authenticated',
|
||||
source: 'provider',
|
||||
check: async () => gfIsAuthenticated(),
|
||||
fix: 'Run `teamai init` to authenticate via gf auth login',
|
||||
},
|
||||
@@ -177,11 +417,13 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
checks.push(
|
||||
{
|
||||
name: 'gh CLI is installed',
|
||||
source: 'provider',
|
||||
check: async () => isGhInstalled(),
|
||||
fix: 'Install from https://cli.github.com/ or run `brew install gh`',
|
||||
},
|
||||
{
|
||||
name: 'gh CLI is authenticated',
|
||||
source: 'provider',
|
||||
check: async () => ghIsAuthenticated(),
|
||||
fix: 'Run `gh auth login` to authenticate',
|
||||
},
|
||||
@@ -191,6 +433,7 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
const { gitlabIsAuthenticated } = await import('./providers/gitlab/index.js');
|
||||
checks.push({
|
||||
name: 'GitLab token is configured',
|
||||
source: 'provider',
|
||||
check: async () => gitlabIsAuthenticated(),
|
||||
fix: 'Export GITLAB_TOKEN (a Personal Access Token with `api` scope). '
|
||||
+ 'GITLAB_PRIVATE_TOKEN and GITLAB_PAT are accepted as aliases.',
|
||||
@@ -200,6 +443,7 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
const { gitcodeIsAuthenticated } = await import('./providers/gitcode/index.js');
|
||||
checks.push({
|
||||
name: 'GitCode token is configured',
|
||||
source: 'provider',
|
||||
check: async () => gitcodeIsAuthenticated(),
|
||||
fix: 'Export GITCODE_TOKEN (a GitCode Personal Access Token), or run `teamai init` '
|
||||
+ 'to paste one interactively. GC_TOKEN is accepted as an alias.',
|
||||
@@ -209,11 +453,13 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
checks.push(
|
||||
{
|
||||
name: 'Team repo exists locally',
|
||||
source: 'local',
|
||||
check: async () => pathExists(localConfig.repo.localPath),
|
||||
fix: 'Run `teamai init` to clone the team repo',
|
||||
},
|
||||
{
|
||||
name: 'Team config (teamai.yaml) is valid',
|
||||
source: 'local',
|
||||
check: async () => {
|
||||
const config = await loadTeamConfig(localConfig.repo.localPath);
|
||||
return config !== null;
|
||||
@@ -225,6 +471,10 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
// this check a member whose pushes are rejected queues notes forever and
|
||||
// is told each time that the next pull will retry.
|
||||
name: 'Contributed learnings are published',
|
||||
source: 'local',
|
||||
// pullForScope warns about the queue on its own, with the push error
|
||||
// attached; this check is the standing version of it for `teamai doctor`.
|
||||
reportedByPull: 'pending-learnings',
|
||||
check: async () => {
|
||||
const { listPendingLearnings } = await import('./utils/pending-learnings.js');
|
||||
return (await listPendingLearnings(localConfig)).length === 0;
|
||||
@@ -232,9 +482,13 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
fix: 'Run `teamai pull` to publish them. If they stay queued, check that you '
|
||||
+ 'can push to the team repo (run with --verbose to see the push error).',
|
||||
},
|
||||
...await buildEnabledToolChecks(ctx),
|
||||
...await buildHookChecks(toolPaths, baseDir, localConfig),
|
||||
...await buildDeliveryChecks(ctx),
|
||||
...await buildDocsCheck(ctx),
|
||||
{
|
||||
name: 'Env variables injected in shell profile',
|
||||
source: 'local',
|
||||
check: async () => {
|
||||
if (teamConfig?.sharing?.env?.injectShellProfile === false) return true;
|
||||
|
||||
@@ -272,7 +526,7 @@ export async function buildChecks(ctx: DoctorContext): Promise<Check[]> {
|
||||
* lands, so the human rendering keeps streaming while a slow check (a provider
|
||||
* CLI auth probe) is still running.
|
||||
*/
|
||||
async function runChecks(
|
||||
export async function runChecks(
|
||||
checks: Check[],
|
||||
onResult?: (result: CheckResult) => void,
|
||||
): Promise<CheckResult[]> {
|
||||
@@ -291,14 +545,18 @@ function emitReport(report: DoctorReport): void {
|
||||
console.log(JSON.stringify(report, null, 2));
|
||||
}
|
||||
|
||||
/** The human rendering of one finished check. */
|
||||
function renderResult({ name, ok, fix }: CheckResult): void {
|
||||
if (ok) {
|
||||
console.log(` ✔ ${name}`);
|
||||
return;
|
||||
}
|
||||
console.log(` ✖ ${name}`);
|
||||
if (fix) console.log(` → ${fix}`);
|
||||
/**
|
||||
* The human rendering of one finished check, as lines. Exported because `pull`
|
||||
* prints the same shape through the logger rather than stdout — one definition
|
||||
* of the glyphs and the indent, two sinks.
|
||||
*/
|
||||
export function formatCheckResult({ name, ok, fix }: CheckResult): string[] {
|
||||
if (ok) return [` ✔ ${name}`];
|
||||
return fix ? [` ✖ ${name}`, ` → ${fix}`] : [` ✖ ${name}`];
|
||||
}
|
||||
|
||||
function renderResult(result: CheckResult): void {
|
||||
for (const line of formatCheckResult(result)) console.log(line);
|
||||
}
|
||||
|
||||
export async function doctor(options: DoctorOptions): Promise<boolean> {
|
||||
|
||||
+174
-38
@@ -48,7 +48,7 @@ import { withTimeout } from './utils/async.js';
|
||||
let pendingUsageReport: Promise<void> | undefined;
|
||||
const FILE_NOT_FOUND_ERROR_CODE = 'ENOENT';
|
||||
|
||||
interface RolePullContext {
|
||||
export interface RolePullContext {
|
||||
activeNamespaces: ResourceNamespaces;
|
||||
activeSkillNames: Set<string>;
|
||||
inactiveSkillNames: Set<string>;
|
||||
@@ -314,6 +314,74 @@ export async function scanRoleAwareSkills(localConfig: LocalConfig, namespaces:
|
||||
return [...items.values()];
|
||||
}
|
||||
|
||||
/** What a member should have on disk, and what the team repo holds. */
|
||||
export interface DesiredSkills {
|
||||
/** The skills this member should have: role namespaces ∪ subscribed tags − exclusions. */
|
||||
items: ResourceItem[];
|
||||
/** Every skill in the team repo — the set cleanup is allowed to prune from. */
|
||||
teamItems: ResourceItem[];
|
||||
/** How many skills the tag channel left out, for the sync line. */
|
||||
skippedByTags: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the skills this member should have. Read-only: `pull` calls it to
|
||||
* decide what to install, and `doctor` calls it to check what landed (#598).
|
||||
* Keeping it in one place is the point — re-deriving the union inside the check
|
||||
* would put role namespaces, tag subscriptions and exclusions in a second place
|
||||
* that drifts on its own.
|
||||
*
|
||||
* `roleContext` is explicit rather than resolved here: `pullForScope` already
|
||||
* holds one (it also drives rules, agents and cleanup), and null means "no roles
|
||||
* configured", not "not looked up yet".
|
||||
*/
|
||||
export async function resolveDesiredSkills(
|
||||
teamConfig: TeamaiConfig,
|
||||
localConfig: LocalConfig,
|
||||
roleContext: RolePullContext | null,
|
||||
): Promise<DesiredSkills> {
|
||||
const handler = getHandler('skills');
|
||||
const tagsConfig = await loadTagsConfig(localConfig.repo.localPath);
|
||||
const subscribedTags = localConfig.subscribedTags;
|
||||
const excludedSkills = new Set(localConfig.excludedSkills ?? []);
|
||||
|
||||
const directoryItems = roleContext
|
||||
? await scanRoleAwareSkills(localConfig, roleContext.activeNamespaces)
|
||||
: await handler.scanTeamForPull(teamConfig, localConfig);
|
||||
|
||||
const teamItems = await handler.scanTeamForPull(teamConfig, localConfig);
|
||||
|
||||
// Tag channel: only augment when subscriptions are actually active
|
||||
const hasActiveTagSubscriptions = tagsConfig != null
|
||||
&& subscribedTags != null
|
||||
&& subscribedTags.length > 0;
|
||||
|
||||
let tagIncluded: ResourceItem[] = [];
|
||||
let skippedByTags = 0;
|
||||
if (hasActiveTagSubscriptions) {
|
||||
const tagResult = filterByTags(teamItems, tagsConfig, subscribedTags, 'skills');
|
||||
const subscribedTagSet = new Set(subscribedTags);
|
||||
tagIncluded = tagResult.included.filter((item) => {
|
||||
const itemTags = tagsConfig.skills[item.name];
|
||||
return itemTags?.some((tag) => subscribedTagSet.has(tag));
|
||||
});
|
||||
skippedByTags = tagResult.skipped.length;
|
||||
}
|
||||
|
||||
// Union: merge directory items with tag-matched items
|
||||
const merged = new Map<string, ResourceItem>();
|
||||
for (const item of directoryItems) merged.set(item.name, item);
|
||||
for (const item of tagIncluded) {
|
||||
if (!merged.has(item.name)) merged.set(item.name, item);
|
||||
}
|
||||
|
||||
const items = excludedSkills.size > 0
|
||||
? [...merged.values()].filter((item) => !excludedSkills.has(item.name))
|
||||
: [...merged.values()];
|
||||
|
||||
return { items, teamItems, skippedByTags };
|
||||
}
|
||||
|
||||
// Deployment adds a CONTRIBUTORS file that the team source may not have; ignore it
|
||||
// when checking whether a deployed skill still matches its source (same file as
|
||||
// resources/skills.ts and pre-push-sync.ts use for modification detection).
|
||||
@@ -563,6 +631,13 @@ async function cleanupTombstonedResources(
|
||||
async function pullForScope(
|
||||
localConfig: LocalConfig,
|
||||
options: GlobalOptions,
|
||||
/**
|
||||
* Collects what this scope tells the member in its own words, so the
|
||||
* post-pull pass does not repeat it. Required rather than optional on
|
||||
* `policy`: a call site that forgot it would silently stop recording, which
|
||||
* is the failure this mechanism exists to avoid. See `Check.reportedByPull`.
|
||||
*/
|
||||
reported: Set<string>,
|
||||
policy: {
|
||||
resourceTypes?: readonly ResourceType[];
|
||||
revisionField?: 'lastPullRev' | 'lastInheritedPullRev';
|
||||
@@ -612,6 +687,7 @@ async function pullForScope(
|
||||
if (queue.remaining > 0) {
|
||||
// Say it out loud. A member whose pushes are rejected would otherwise
|
||||
// queue notes forever and never hear about it.
|
||||
reported.add('pending-learnings');
|
||||
log.warn(
|
||||
`${queue.remaining} learning(s) are written locally but not published`
|
||||
+ `${queue.lastError ? `: ${queue.lastError}` : ''}. `
|
||||
@@ -728,41 +804,12 @@ async function pullForScope(
|
||||
let items: ResourceItem[];
|
||||
let skippedByTags = 0;
|
||||
if (type === 'skills') {
|
||||
const directoryItems = roleContext
|
||||
? await scanRoleAwareSkills(localConfig, roleContext.activeNamespaces)
|
||||
: await handler.scanTeamForPull(freshConfig, localConfig);
|
||||
|
||||
const allTeamSkills = await handler.scanTeamForPull(freshConfig, localConfig);
|
||||
|
||||
// Tag channel: only augment when subscriptions are actually active
|
||||
const hasActiveTagSubscriptions = tagsConfig != null
|
||||
&& subscribedTags != null
|
||||
&& subscribedTags.length > 0;
|
||||
|
||||
let tagIncluded: ResourceItem[] = [];
|
||||
if (hasActiveTagSubscriptions) {
|
||||
const tagResult = filterByTags(allTeamSkills, tagsConfig, subscribedTags, 'skills');
|
||||
const subscribedTagSet = new Set(subscribedTags);
|
||||
tagIncluded = tagResult.included.filter((item) => {
|
||||
const itemTags = tagsConfig.skills[item.name];
|
||||
return itemTags?.some((tag) => subscribedTagSet.has(tag));
|
||||
});
|
||||
skippedByTags = tagResult.skipped.length;
|
||||
}
|
||||
|
||||
// Union: merge directory items with tag-matched items
|
||||
const merged = new Map<string, ResourceItem>();
|
||||
for (const item of directoryItems) merged.set(item.name, item);
|
||||
for (const item of tagIncluded) {
|
||||
if (!merged.has(item.name)) merged.set(item.name, item);
|
||||
}
|
||||
items = [...merged.values()];
|
||||
if (excludedSkills.size > 0) {
|
||||
items = items.filter((item) => !excludedSkills.has(item.name));
|
||||
}
|
||||
const desired = await resolveDesiredSkills(freshConfig, localConfig, roleContext);
|
||||
items = desired.items;
|
||||
skippedByTags = desired.skippedByTags;
|
||||
desiredSkillNames = new Set(items.map((i) => i.name));
|
||||
knownRepoSkillNames = new Set(allTeamSkills.map((i) => i.name));
|
||||
knownRepoSkillSources = new Map(allTeamSkills.map((i) => [i.name, i.sourcePath]));
|
||||
knownRepoSkillNames = new Set(desired.teamItems.map((i) => i.name));
|
||||
knownRepoSkillSources = new Map(desired.teamItems.map((i) => [i.name, i.sourcePath]));
|
||||
} else if (type === 'agents') {
|
||||
// Role/project namespace filter (root = everyone), same as rules. Throws
|
||||
// on a stem collision; the caller's try/catch logs it and aborts the scope.
|
||||
@@ -1564,6 +1611,11 @@ async function reinjectLegacyHooks(localConfig: LocalConfig): Promise<void> {
|
||||
* source skills are pulled only for the active project scope.
|
||||
*/
|
||||
export async function pull(options: GlobalOptions): Promise<void> {
|
||||
// What the scopes below say in their own words, so the post-pull pass does
|
||||
// not repeat it. Owned here rather than at module scope so nothing survives
|
||||
// into another call.
|
||||
const reported = new Set<string>();
|
||||
|
||||
// Whether HOME's settings.json still has the pre-dispatch hook format. Read now
|
||||
// (HOME-only, no shared clone), but the actual reinject runs later under the
|
||||
// scope lock so it never consumes a concurrent push's transient branch config.
|
||||
@@ -1629,7 +1681,7 @@ export async function pull(options: GlobalOptions): Promise<void> {
|
||||
inheritedUserConfig = loadedUserConfig;
|
||||
log.info('project scope detected, inheriting user-scope resources and knowledge');
|
||||
if (await lockScope(inheritedUserConfig)) {
|
||||
await pullForScope(inheritedUserConfig, options, {
|
||||
await pullForScope(inheritedUserConfig, options, reported, {
|
||||
resourceTypes: ['skills', 'rules', 'docs', 'agents'],
|
||||
revisionField: 'lastInheritedPullRev',
|
||||
});
|
||||
@@ -1637,7 +1689,7 @@ export async function pull(options: GlobalOptions): Promise<void> {
|
||||
} else {
|
||||
activeUserConfig = loadedUserConfig;
|
||||
if (await lockScope(activeUserConfig)) {
|
||||
await pullForScope(activeUserConfig, options);
|
||||
await pullForScope(activeUserConfig, options, reported);
|
||||
}
|
||||
}
|
||||
} else if (inheritUserScope) {
|
||||
@@ -1654,7 +1706,7 @@ export async function pull(options: GlobalOptions): Promise<void> {
|
||||
if (projectConfig) {
|
||||
try {
|
||||
if (await lockScope(projectConfig)) {
|
||||
await pullForScope(projectConfig, options);
|
||||
await pullForScope(projectConfig, options, reported);
|
||||
}
|
||||
} catch (e) {
|
||||
log.warn(`Project-scope pull error: ${(e as Error).message}`);
|
||||
@@ -1787,6 +1839,15 @@ export async function pull(options: GlobalOptions): Promise<void> {
|
||||
log.debug(`Source pull skipped: ${(e as Error).message}`);
|
||||
}
|
||||
}
|
||||
|
||||
// 6. Post-conditions. Everything above reported what it *did*; these report
|
||||
// what is actually on disk (issue #598). Only after an explicit pull: the
|
||||
// SessionStart hook runs pull({ silent: true }) and must stay free.
|
||||
// Skipped when any scope was contended: those are dropped from every
|
||||
// clone-consuming stage above for the same reason the checks would need
|
||||
// the clone, and reading it while the other process holds it on a
|
||||
// transient branch is how a diagnostic invents a failure.
|
||||
await reportPostPullChecks(options, reported, contended.size > 0);
|
||||
} finally {
|
||||
const releaseSyncLocks = async () => {
|
||||
for (const lock of heldLocks.values()) await releaseLock(lock);
|
||||
@@ -1804,6 +1865,81 @@ export async function pull(options: GlobalOptions): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
/** Post-pull diagnostics are a courtesy, not the job. Do not wait forever. */
|
||||
const POST_PULL_CHECKS_TIMEOUT_MS = 5000;
|
||||
|
||||
/**
|
||||
* Re-run the `teamai doctor` registry after an explicit pull and print only what
|
||||
* failed. Every line above this one reports what the pull *did*; these report
|
||||
* what is actually on disk — the gap behind #574, #525, #342 and friends, where
|
||||
* the command says "Synced N" and the tool receives nothing.
|
||||
*
|
||||
* Two kinds are left out. Provider checks: this pull just used the provider
|
||||
* successfully, so re-probing `gh auth status` would add a subprocess to every
|
||||
* sync and prove nothing new. And a check whose `reportedByPull` topic this run
|
||||
* actually reported — repeating it would say the same thing twice and, since
|
||||
* its `fix` is written for `doctor`, tell the member to run the pull they just
|
||||
* ran. A topic the pull stayed silent about is NOT suppressed: the scope may
|
||||
* have aborted before reaching it. `teamai doctor` still runs everything.
|
||||
*/
|
||||
async function reportPostPullChecks(
|
||||
options: GlobalOptions,
|
||||
reported: ReadonlySet<string>,
|
||||
/**
|
||||
* True when another process held a scope's sync lock this run. The checks
|
||||
* resolve their own context from the shared clone, which that process may
|
||||
* have on a transient branch, so their answers would be about its work in
|
||||
* progress rather than about this machine.
|
||||
*/
|
||||
contended: boolean,
|
||||
): Promise<void> {
|
||||
if (options.silent || options.dryRun) return;
|
||||
if (contended) {
|
||||
// The pull already said the scope was skipped. Saying nothing more is the
|
||||
// honest outcome; `teamai doctor` runs them once the other process is done.
|
||||
log.debug('Post-pull checks skipped: another pull/push holds a scope lock');
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const { resolveDoctorContext, buildChecks, runChecks, formatCheckResult } = await import('./doctor.js');
|
||||
const ctx = await resolveDoctorContext();
|
||||
if (!ctx) return;
|
||||
|
||||
// The budget covers building the registry as well as running it: the
|
||||
// delivery checks stat every desired skill for every tool while the
|
||||
// registry is built, which is where the I/O actually is.
|
||||
const results = await withTimeout(
|
||||
(async () => {
|
||||
const local = (await buildChecks(ctx))
|
||||
.filter((c) => c.source === 'local')
|
||||
.filter((c) => !c.reportedByPull || !reported.has(c.reportedByPull));
|
||||
return runChecks(local);
|
||||
})(),
|
||||
POST_PULL_CHECKS_TIMEOUT_MS,
|
||||
`Post-pull checks are still running after ${POST_PULL_CHECKS_TIMEOUT_MS}ms`,
|
||||
);
|
||||
|
||||
const failures = results.filter((r) => !r.ok);
|
||||
if (failures.length === 0) return;
|
||||
|
||||
log.warn(`Pull finished, but ${failures.length} check(s) failed:`);
|
||||
for (const failure of failures) {
|
||||
const [headline, ...detail] = formatCheckResult(failure);
|
||||
log.warn(headline);
|
||||
for (const line of detail) log.dim(line);
|
||||
}
|
||||
log.dim(' Run `teamai doctor` for the full report.');
|
||||
} catch (e) {
|
||||
// The sync already succeeded. A diagnostic that breaks must not undo that,
|
||||
// so this never rethrows. It does say one line though: staying silent after
|
||||
// the whole budget is the same "reported success, nothing happened" shape
|
||||
// these checks exist to catch. The reason stays on the debug channel
|
||||
// because it is about teamai, not about the member's repo.
|
||||
log.debug(`Post-pull checks skipped: ${(e as Error).message}`);
|
||||
log.dim(' Post-pull checks did not run. Run `teamai doctor` for the full report.');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reconcile built-in (A) + team (B) hooks across all active scopes. Bypasses the
|
||||
* rev fast-path so team hook changes and newly shipped built-in hooks apply even
|
||||
|
||||
+14
-12
@@ -2,10 +2,22 @@ import path from 'node:path';
|
||||
import fse from 'fs-extra';
|
||||
import { ResourceHandler } from './base.js';
|
||||
import type { ResourceItem, TeamaiConfig, LocalConfig } from '../types.js';
|
||||
import { resolveBaseDir } from '../types.js';
|
||||
import { expandHome, listFilesRecursive } from '../utils/fs.js';
|
||||
import { log } from '../utils/logger.js';
|
||||
|
||||
/**
|
||||
* The single directory the team docs bundle is copied into. In project scope a
|
||||
* `~/`-prefixed `sharing.docs.localDir` is relative to the project root, not to
|
||||
* HOME. `pull` writes here and `doctor` checks here (#598).
|
||||
*/
|
||||
export function resolveDocsDestination(teamConfig: TeamaiConfig, localConfig: LocalConfig): string {
|
||||
const localDir = teamConfig.sharing.docs.localDir;
|
||||
if (localConfig.scope === 'project' && localConfig.projectRoot && localDir.startsWith('~/')) {
|
||||
return path.join(localConfig.projectRoot, localDir.substring(2));
|
||||
}
|
||||
return expandHome(localDir);
|
||||
}
|
||||
|
||||
export class DocsHandler extends ResourceHandler {
|
||||
readonly type = 'docs' as const;
|
||||
|
||||
@@ -40,17 +52,7 @@ export class DocsHandler extends ResourceHandler {
|
||||
* Sync docs from team repo to local docs directory.
|
||||
*/
|
||||
async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<void> {
|
||||
// For project scope, resolve docs dir relative to projectRoot
|
||||
const docsLocalDir = teamConfig.sharing.docs.localDir;
|
||||
let localDocsDir: string;
|
||||
if (localConfig.scope === 'project' && localConfig.projectRoot) {
|
||||
// Replace ~ with projectRoot
|
||||
localDocsDir = docsLocalDir.startsWith('~/')
|
||||
? path.join(localConfig.projectRoot, docsLocalDir.substring(2))
|
||||
: expandHome(docsLocalDir);
|
||||
} else {
|
||||
localDocsDir = expandHome(docsLocalDir);
|
||||
}
|
||||
const localDocsDir = resolveDocsDestination(teamConfig, localConfig);
|
||||
try {
|
||||
const src = expandHome(item.sourcePath);
|
||||
await fse.copy(src, localDocsDir, {
|
||||
|
||||
+83
-27
@@ -30,8 +30,14 @@ export async function resolveSkillDestination(
|
||||
if (tool === CODEX_TOOL) {
|
||||
const sharedDestination = path.join(baseDir, SHARED_AGENT_SKILLS_PATH, skillName);
|
||||
if (await pathExists(sharedDestination)) {
|
||||
// No source to compare against: the caller only wants to know where the
|
||||
// skill lives. Reconciling needs the team copy to prove the two are the
|
||||
// same, so without it there is nothing to decide and nothing to report —
|
||||
// `doctor` and the post-pull pass would otherwise warn about a conflict
|
||||
// on every skill, for copies the write path treats as identical.
|
||||
if (!sourcePath) return sharedDestination;
|
||||
if (await pathExists(configuredDestination)) {
|
||||
if (sourcePath && await dirContentEqual(sharedDestination, configuredDestination) && await dirContentEqual(configuredDestination, sourcePath)) {
|
||||
if (await dirContentEqual(sharedDestination, configuredDestination) && await dirContentEqual(configuredDestination, sourcePath)) {
|
||||
await remove(configuredDestination);
|
||||
log.debug(`Removed identical TeamAI skill ${skillName} from ${configuredSkillsPath}`);
|
||||
} else {
|
||||
@@ -45,6 +51,80 @@ export async function resolveSkillDestination(
|
||||
return configuredDestination;
|
||||
}
|
||||
|
||||
/**
|
||||
* Name used only to ask the resolver a yes/no question. It shapes the path that
|
||||
* comes back, never the installed gate, so no skill by this name need exist.
|
||||
*/
|
||||
const INSTALL_PROBE_SKILL = '__teamai_probe__';
|
||||
|
||||
/**
|
||||
* Whether skills reach `tool` at all on this machine.
|
||||
*
|
||||
* Asks `skillTargetForTool`, which is the gate the write path itself runs:
|
||||
* OpenClaw resolves through its workspace directory, Hermes through its home,
|
||||
* Copilot counts itself installed once `enabledAgents` names it, and everything
|
||||
* else falls back to the tool root. A probe that answered any of those
|
||||
* differently is exactly how "Synced N skills" ends up true while a tool
|
||||
* receives nothing (#598), which is the failure `doctor` exists to catch.
|
||||
*/
|
||||
export async function skillsReachTool(
|
||||
tool: string,
|
||||
configuredSkillsPath: string,
|
||||
localConfig: LocalConfig,
|
||||
): Promise<boolean> {
|
||||
return await skillTargetForTool(tool, configuredSkillsPath, localConfig, INSTALL_PROBE_SKILL) !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Where `skillName` lands for `tool` on this machine, or null when the tool
|
||||
* cannot receive it: no skills path configured, or the tool is not installed.
|
||||
*
|
||||
* One place answers that question, so `pull` writes and `doctor` checks the very
|
||||
* same paths (#598). A second copy of these gates is how "Synced 12 skills"
|
||||
* ends up true for one tool and silently false for another.
|
||||
*
|
||||
* `sourcePath` belongs to the write path: it lets the Codex shared-directory
|
||||
* reconciliation delete a duplicate it can prove is identical. Omit it to
|
||||
* resolve a destination without that side effect.
|
||||
*/
|
||||
export async function skillTargetForTool(
|
||||
tool: string,
|
||||
configuredSkillsPath: string | undefined,
|
||||
localConfig: LocalConfig,
|
||||
skillName: string,
|
||||
sourcePath?: string,
|
||||
): Promise<string | null> {
|
||||
if (!configuredSkillsPath) return null;
|
||||
|
||||
if (tool === 'openclaw') {
|
||||
const wsDir = await resolveOpenclawWorkspaceDir();
|
||||
if (!wsDir) {
|
||||
log.debug('Skipping skill sync for openclaw: workspace dir not found');
|
||||
return null;
|
||||
}
|
||||
return path.join(wsDir, 'skills', skillName);
|
||||
}
|
||||
|
||||
if (tool === 'hermes') {
|
||||
// Like every other tool, skip when not installed: getHermesHome() always
|
||||
// resolves (HERMES_HOME or ~/.hermes), so without this check every pull
|
||||
// creates a hermes home the user never asked for.
|
||||
if (!await pathExists(getHermesHome())) {
|
||||
log.debug(`Skipping skill sync for ${tool}: tool not installed`);
|
||||
return null;
|
||||
}
|
||||
return path.join(getHermesHome(), 'skills', skillName);
|
||||
}
|
||||
|
||||
if (!await isToolInstalledForConfig(tool, configuredSkillsPath, localConfig)) {
|
||||
log.debug(`Skipping skill sync for ${tool}: tool not installed`);
|
||||
return null;
|
||||
}
|
||||
|
||||
const baseDir = resolveToolBaseDir(tool, localConfig);
|
||||
return resolveSkillDestination(tool, configuredSkillsPath, baseDir, skillName, sourcePath);
|
||||
}
|
||||
|
||||
/** Add fields immediately before the closing delimiter without reformatting existing YAML. */
|
||||
function appendFrontmatterFields(raw: string, fields: Record<string, string>): string {
|
||||
const eol = raw.includes('\r\n') ? '\r\n' : '\n';
|
||||
@@ -480,33 +560,9 @@ export class SkillsHandler extends ResourceHandler {
|
||||
async pullItem(item: ResourceItem, teamConfig: TeamaiConfig, localConfig: LocalConfig): Promise<void> {
|
||||
for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) {
|
||||
if (isAgentExcluded(localConfig, tool)) continue;
|
||||
if (!toolPath.skills) continue;
|
||||
|
||||
let dest: string;
|
||||
if (tool === 'openclaw') {
|
||||
const wsDir = await resolveOpenclawWorkspaceDir();
|
||||
if (!wsDir) {
|
||||
log.debug(`Skipping skill sync for openclaw: workspace dir not found`);
|
||||
continue;
|
||||
}
|
||||
dest = path.join(wsDir, 'skills', item.name);
|
||||
} else if (tool === 'hermes') {
|
||||
// Like every other tool, skip when not installed: getHermesHome()
|
||||
// always resolves (HERMES_HOME or ~/.hermes), so without this check
|
||||
// every pull creates a hermes home the user never asked for.
|
||||
if (!await pathExists(getHermesHome())) {
|
||||
log.debug(`Skipping skill sync for ${tool}: tool not installed`);
|
||||
continue;
|
||||
}
|
||||
dest = path.join(getHermesHome(), 'skills', item.name);
|
||||
} else {
|
||||
if (!await isToolInstalledForConfig(tool, toolPath.skills, localConfig)) {
|
||||
log.debug(`Skipping skill sync for ${tool}: tool not installed`);
|
||||
continue;
|
||||
}
|
||||
const baseDir = resolveToolBaseDir(tool, localConfig);
|
||||
dest = await resolveSkillDestination(tool, toolPath.skills, baseDir, item.name, item.sourcePath);
|
||||
}
|
||||
const dest = await skillTargetForTool(tool, toolPath.skills, localConfig, item.name, item.sourcePath);
|
||||
if (!dest) continue;
|
||||
|
||||
try {
|
||||
await copyDir(item.sourcePath, dest);
|
||||
|
||||
Reference in New Issue
Block a user