[feat] Make doctor's checks reusable, and add a check that skills actually landed

[SaulMoro]

Problem

Several bugs in this repo share one shape. The command reports success and nothing lands on disk: #574, #525, #342, #335, #331, #508, #436, and #585, which is still open.

src/doctor.ts already holds checks that catch part of that shape. Three things stop them from being useful.

doctor runs only after a person already suspects a problem. A member runs teamai pull, reads Synced N rule(s), and finds out days later that the hooks never reached the tool settings.

The result is still not reusable. #569 landed after this issue was filed and fixed half of it: doctor() now returns allPassed (src/doctor.ts:234) and the command exits 1 when a check fails (src/index.ts:222), so teamai doctor && deploy is safe now. The registry is still inline at src/doctor.ts:102-199, so no other caller can run it, and there is still no --json, so a hook, a CI job, or an agent can only read the exit code, never which check failed or how to fix it.

Nothing checks the payload. Every check verifies plumbing: provider CLI, config, clone, hook injection, env block. None of them verifies that the skills pull reported are on disk and readable by the agent — which is what the bugs above are actually about.

Proposed solution

No new command. Extract the check registry, then read it in three places.

 teamai doctor
   buildChecks()
   render()
-  process.exitCode = anyFailed ? 1 : 0     landed in #569
+  --json emits [{ name, ok, fix }]

 teamai pull
   sync()
   report("Synced N rule(s)")
+  if (!silent) buildChecks(), then print the failures

 hook session-start
   pull({ silent: true })      unchanged, runs no checks

 buildChecks()
+  + delivery check: every desired skill resolves to a directory
+    on disk with a parseable SKILL.md

Phase 1 exports the result. It is useful on its own. Move the check registry out of doctor() into an exported buildChecks(ctx): Promise<Check[]>, fed by an exported resolveDoctorContext(): Promise<DoctorContext | null>. The registry is inline today at src/doctor.ts:102-199. A context object rather than (config, scope): the registry also needs the toolPaths narrowing (src/doctor.ts:89-94) and the resolveHookScope baseDir (src/doctor.ts:100), and passing the two loose inputs would make every caller re-derive both — the second place that drifts on its own, which this issue rejects for inline post-conditions. resolveDoctorContext is therefore part of the phase-2 contract too. doctor() then calls both, renders, and returns allPassed as it does now. Add --json, which emits the { name, ok, fix } shape the checks already carry. codebase --json and codebase review --json set the precedent, and #580 restored the same flag on the cache commands.

Phase 2 runs the checks at the end of an interactive pull. Call resolveDoctorContext() and buildChecks() after the sync and print each failure with the fix string it already has. Gate the call on the existing silent flag. The session-start hook calls pull({ silent: true }) at src/hook-handlers.ts:98, and the host awaits that handler, so the gate is what keeps session startup free: the hook path runs no checks, and only an explicit teamai pull does.

One blind spot is worth closing here. buildHookChecks skips a tool whose settings directory does not exist (src/doctor.ts:33). That is the same silent skip #574 reports in pull, reproduced inside doctor. After #569, toolPaths is already narrowed to the enabled, non-excluded tools (src/doctor.ts:89-94), so "declared in enabledAgents, no directory on disk" is a one-line change from continue to a failing check.

Phase 3 adds the check that looks at the payload. The delivery check is a set comparison:

flowchart LR
  D["desired set<br>role namespaces ∪ subscribed tags − excluded<br>src/pull.ts:704-741"]
  R{"resolveSkillDestination + stat<br>per enabled tool<br>src/resources/skills.ts:22"}
  OK["ok — release<br>.claude/skills/release/SKILL.md"]
  BAD["landed, invisible — incident<br>frontmatter name does not match the directory<br>the agent never discovers it"]
  MISS["missing — security-scan<br>never written to this tool"]
  D --> R
  R -->|"directory exists, SKILL.md parses, name matches"| OK
  R -->|"directory exists, frontmatter broken"| BAD
  R -->|"no directory"| MISS

Loading

On failure the check lists the missing ones per tool, with teamai pull as the fix.

The desired set is already computed and already discarded. pullForScope builds it at src/pull.ts:739 as desiredSkillNames, uses it at src/pull.ts:853 to decide what not to delete, and drops it when the run ends. Phase 3 extracts that block into an exported, write-free resolveDesiredSkills(teamConfig, localConfig), calls it from pullForScope where it stands today, and calls it again from the check. Re-deriving the policy inside doctor would put role namespaces, tag subscriptions and excludes in a second place, which is the drift this issue already rejects for inline post-conditions.

ResourceHandler.diff() computes this direction already — removed at src/resources/base.ts:132 is "in the team repo, not local" — and has no production callers — only two assertions in src/__tests__/remove.test.ts exercise it. It cannot be reused as is: scanLocalForPush skips a local copy whose content equals the team repo's (src/resources/skills.ts:365), which is the success case, so every correctly delivered skill would report as missing. The check needs presence, not push candidacy.

This does not overlap #597, which now gates rules, skills and agents at write time. The two verify the same property at different moments:

	#597	phase 3
runs	inside pull, at write time	inside buildChecks, any time after
scope	rules, skills and agents, through installedToolsFor	skills first
catches	the write never happened	that, plus drift after a correct pull: directory deleted by hand, tool reinstalled, role changed, #590's over-deletion
blind to	anything after the run ends	a failure inside a run that nobody follows with a check

Both are worth having.

Skills first, because the destination resolver already exists. Rules and agents are the same check with a different toolPath field, and docs is the same check against a single directory. mcp and the CLAUDE.md injections — culture at src/pull.ts:1017, shared instructions at src/pull.ts:1032-1043 — are a different shape: they land as an entry or a marked section inside a file, so their check is closer to the hook check that already exists.

What this covers, and what it does not

buildChecks verifies these today:

provider CLI installed, or provider token configured   (gf, gh, GitLab, GitCode)
team repo cloned
teamai.yaml parses
teamai hooks present in <tool> settings                (one check per installed tool)
env block present in the shell profile

Phase 2 makes those run at the end of an interactive pull, which covers part of #574 and #342.

Phase 3 covers what a write-time gate cannot see. #597 has since generalized its own: installedToolsFor(teamConfig, localConfig, field) now backs rules, skills and agents, and docs is left out with a reason — DocsHandler.pullItem copies into one fixed directory that fse.copy creates, so there is no per-tool skip to gate. Three things survive that:

• Per-tool granularity. The gate passes when at least one tool directory exists, while SkillsHandler.pullItem skips each uninstalled tool silently. With claude installed and codex not, Synced 12 skills still prints and Codex receives none — [bug] init 不做初始同步,且 pull 在工具目录不存在时静默跳过却报成功，以及codex必须在启动后进行一次会话才会执行teamai pull #574 one level down.
• Landed but invisible. SKILL.md missing, frontmatter unparseable, name not matching the directory ([bug] 换行符不一致导致skill有两份名字跟描述，正则只匹配LF #372's class). The write succeeded, so no write-time gate has anything to report.
• Drift after a correct pull. A directory deleted by hand, a tool reinstalled, a role changed, [bug] teamai remove deletes rules and skills from tools excluded by enabledAgents #590's over-deletion. The pull that was right yesterday says nothing about the disk today.

env is also ungated: it counts its source variables at src/pull.ts:764. Hooks and mcp never reach this loop at all — resourceTypes is ['skills', 'rules', 'docs', 'env', 'agents'] (src/pull.ts:667) and both are reconciled separately.

Alternatives considered

A teamai verify command. Rejected, because it is doctor under a second name, and the repo's rule is to extend an existing command first.

Post-condition assertions written inline in each writer. Rejected, because the same assertion then lives in several places and each copy drifts on its own.

For phase 3, having pull write an installed manifest that doctor reads instead of resolving destinations from state. Cheaper to compute, but it goes stale exactly when it matters: the manifest still says a skill was installed after someone deletes the directory, which is the case the check exists to catch.

Folding this into #585. #585 covers init --agent seeding and the specific miscount when a tool directory is missing, and #597 implements it. The two do not overlap, and #597 should land first.

Additional context

One decision before I open a PR, and it only affects phase 3. Extracting resolveDesiredSkills out of pullForScope touches the pull path, which is the hottest code in the repo right now (#597 is open against it). The alternative is to land phases 1 and 2 first and keep phase 3 behind #597. I would extract it, because the check is the only part of this issue that addresses the bugs it opens with, but the sequencing is your call.

Phase 1 is implemented in #599, against c674ffe. It extracts the two functions above, adds --json, and touches no other command; src/pull.ts is untouched, so it does not collide with #597. Phases 2 and 3 are still open.

Line numbers refer to c674ffe.

Related: #597, #585, #574, #569, #525, #342, #335, #331, #508, #436.

[SaulMoro]

Updated the body against c674ffe. Three changes, no review had started yet:

• fix: make doctor diagnostics actionable #569 landed the exit code after this was filed (doctor() returns allPassed, src/index.ts:222 sets process.exitCode). Dropped it from phase 1, which is now the registry extraction plus --json. The open question I ended with — exit 1 by default or behind --strict — is answered by fix: make doctor diagnostics actionable #569, so it is gone.
• Added phase 3, the delivery check: compare the desired skill set against what is on disk per enabled tool. This is the residue the previous body flagged as covered by nobody. pullForScope already computes the desired set at src/pull.ts:739 and discards it, and ResourceHandler.diff() (src/resources/base.ts:121) computes the same direction with no production callers, so the phase is mostly wiring rather than new policy.
• Rebased all line numbers to c674ffe and noted that after fix: make doctor diagnostics actionable #569 the buildHookChecks skip at src/doctor.ts:33 is a one-line fix, since toolPaths is already narrowed to the enabled tools.

The remaining decision is sequencing: phase 3 needs resolveDesiredSkills extracted from pullForScope, and #597 is open against that same file.

[SaulMoro]

Phase 1 is up as #599.

One correction to the body while implementing it: the export is buildChecks(ctx) fed by resolveDoctorContext(), not buildChecks(config, scope). The registry also needs the toolPaths narrowing (src/doctor.ts:89-94) and the resolveHookScope baseDir (src/doctor.ts:100), so the two loose arguments would have made every caller re-derive both — the second place that drifts on its own, which this issue argues against elsewhere. resolveDoctorContext is therefore part of what phase 2 consumes, and the body now says so.

Nothing else moved: src/pull.ts is untouched, so #599 does not collide with #597, and phases 2 and 3 stay as written.

[jeff-r2026]

Thanks for the thorough write-up — and for keeping #599 self-contained; it's merged, so Phase 1's done. 🙏

Let's do it all — Phase 2 and Phase 3 together. The delivery check is the part that really fixes these bugs, so no need to defer it.

Just one thing: since #597 is still open against src/pull.ts, could you land it first and base the resolveDesiredSkills extraction on top? That keeps the two PRs from rebasing on each other.

Ping me per PR, and thanks again!

[SaulMoro]

@jeff-r2026 Agreed on scope — phases 2 and 3 together, delivery check included.

One correction on the sequencing, though: #597 is not mine. It is @Morrowga's PR — I have only been reviewing it. So I cannot land it first: it is currently conflicting with main and the rebase has to come from its author.

Rather than block on someone else's branch, I will open phases 2 + 3 as one PR against current main and rebase it onto #597 after that merges. The two touch the same files (src/pull.ts, src/doctor.ts), so a conflict is expected — but it is mine to resolve. #597 keeps priority and nothing I push will ask @Morrowga to rebase.

Two notes while I am here:

• The line numbers in the issue body are against c674ffe and have drifted since (desiredSkillNames is src/pull.ts:687/756 today). I will rebase the references in the PR description.
• Happy to leave a formal review on fix(pull): report actual sync outcome instead of team-repo item count #597 if that helps it move — it is the only thing standing between phase 3 and a clean base.

Will ping you when the PR is up.

[SaulMoro]

@jeff-r2026 Phases 2 and 3 are up as #625, against current main for the reason above.

Three corrections to the body, all found while implementing or while driving the real CLI:

• Phase 2 does not run the whole registry. Check gained source: 'local' | 'provider', and 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 everything.
• The buildHookChecks skip was not a one-line fix. Turning that continue into a failing check 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 over the narrowed toolPaths, probing a resource path rather than the settings path.
• docs is in, on top of skills. It is one fixed directory, so it is a tree compare that costs nothing extra.

rules, agents and mcp are not in. The body predicted they would be "the same check with a different toolPath field"; that turned out to be wrong. Rules change filename and content per tool (.mdc with derived frontmatter, Copilot instructions), and an agent carries spec.targets per item across three render formats, so its desired set is a relation rather than a product — and checking it means parsing every agent YAML on every pull. I opened #624 with the shape I would use (a deliveryTargets seam on ResourceHandler) and the one decision worth your call: whether rules and agents run in the post-pull pass at all, or only in teamai doctor.

Test plan and the real-CLI report are in the PR: unit + e2e green, and a manual sandbox across git/gitlab/github and Claude/Codex/CodeBuddy/OpenCode.

One thing that report surfaced, unrelated to this change: a resource deleted by hand is not restored by the next teamai pull, because the rev fast-path prints "Already synced, skipping". --force heals it. The new check is what makes it visible; I left the healing alone. Say the word and I will file it.