Conversation
Every line a pull prints reports what it did; none reported what is on disk. That gap is the shape of Tencent#574, Tencent#525, Tencent#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 Tencent#598.
buildHookChecks skipped any tool whose settings directory was missing — the same silent skip Tencent#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 Tencent#598.
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 Tencent#598.
Every other check verifies plumbing — provider CLI, clone, config, hooks, env. None verified the payload, which is what Tencent#574, Tencent#525, Tencent#342 and Tencent#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 Tencent#598.
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 (Tencent#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 Tencent#598.
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 Tencent#598.
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 Tencent#598.
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 Tencent#598.
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 Tencent#598.
This was referenced Sep 17, 2026
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 Tencent#598.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phases 2 and 3 of #598. An interactive
teamai pullnow ends by running thedoctorchecks and printing what failed, and the registry gained the checks that look at the payload rather than the plumbing: the skills and docs a pull reported syncing are verified to be on disk and readable.teamai pull sync() report("Synced N skills") + buildChecks() → print each failure with its fix (interactive only) hook session-start pull({ silent: true }) unchanged, runs no checks buildChecks() + + <tool> is installed enabledAgents names it, nothing is here + + Skills delivered to <tool> per tool, per skill, SKILL.md readable + + Team docs delivered the bundle against sharing.docs.localDirThe delivery check separates three outcomes, because they need different fixes:
flowchart LR D["desired set<br>role namespaces ∪ subscribed tags − excluded"] --> R{"per enabled tool:<br>resolve destination, stat, read SKILL.md"} R -->|"directory exists, frontmatter name matches"| OK["ok"] R -->|"no directory"| MISS["not delivered<br>→ teamai pull"] R -->|"frontmatter broken or name mismatched"| BAD["delivered but unreadable<br>→ fix SKILL.md in the team repo"]Type of Change
Decisions worth a reviewer's attention
Post-pull runs the local checks only.
Checkgainedsource: 'local' | 'provider', and the post-pull pass filters out the provider ones. The pull just used the provider successfully, so re-probinggh auth statuswould add a subprocess to every sync and prove nothing new.teamai doctorstill runs the full registry — verified below ongitlabandgithub.The "enabled but not installed" check is not the one-liner the issue described. #598 proposed turning the
continueinbuildHookChecksinto a failing check. Driving the real CLI showed that never fires for OpenCode or CodeBuddy: they ship skills and no hook configuration, so the function returns before the question is asked — exactly the tools most likely to be declared and absent. It is now its own builder overctx.toolPaths, probing a resource path (resources land underresolveToolBaseDir, the project root in project scope).One resolver per destination, shared with the write path.
skillTargetForTool(src/resources/skills.ts) andresolveDocsDestination(src/resources/docs.ts) are extracted frompullItemand used by both. A second copy of those gates is how "Synced 12 skills" ends up true for one tool and silently false for another.docsis included,rules/agents/mcpare not. Docs are one fixed directory, so the check is a tree compare and costs nothing extra. Rules need a per-tool destination resolver (.mdcwith derived frontmatter, Copilot instructions), and agents carryspec.targetsper item plus three render formats — so their desired set is a relation, not a product, and checking them means parsing every agent YAML on every pull. That deserves its own design and its own review: #624.Known limits, stated rather than hidden. A delivery check's
check()closes over a scan done when the registry was built, so thefixcan name the missing items; it is consistent within a run, not re-entrant. And per-tool granularity for an uninstalled tool depends onenabledAgentsbeing set: without it, the team's tool list is aspirational and an absent tool stays silent, as it always has.Test Plan
npx tsc --noEmitpassesnpx vitest runpasses — 247 files, 3387 testsnpm run test:e2epasses — 33 files, 153 tests (3 files / 26 tests skipped, network-gated)pull-post-checks.test.ts(6),doctor-delivery.test.ts(16),desired-skills.test.ts(6), plus 4 cases indoctor.test.tsReal CLI end-to-end
Throwaway HOME, local git remote, project scope, four agents declared (
claude,codex,codebuddy,opencode), run against providersgit,gitlabandgithub.A hand-deleted skill is reported, with the tool and the name
A skill that landed but is invisible is reported differently, at the end of the pull
An enabled tool that is not installed
The docs bundle
All four agents, once their roots exist
The hook path stays silent,
--dry-runruns nothing, and the provider is never probedRelated Issues
For #598 (phases 2 and 3; phase 1 landed in #599). Follow-up for the remaining primitives: #624.
Notes for Reviewers
This is based on
origin/main, not on #597. That PR is @Morrowga's and is currently conflicting withmain, so I did not block on it — as discussed in the issue. It touchessrc/pull.tsandsrc/doctor.tstoo, so a conflict is expected and it is mine to resolve: #597 keeps priority and I rebase this branch after it merges.Docs are updated in both guides and both languages (
docs/usage-guide.md,docs/usage-guide.zh-CN.md) plus a CHANGELOG entry. No README change: theteamai doctorrow still describes the command, and touching it would cost five synchronized translations.