Skip to content

feat(doctor): export the check registry and add --json - #599

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
SaulMoro:feat/doctor-checks-json-598
Sep 17, 2026
Merged

jeff-r2026 merged 1 commit into
Tencent:mainfrom
SaulMoro:feat/doctor-checks-json-598

Conversation

@SaulMoro

Copy link
Copy Markdown
Contributor

Summary

teamai doctor holds the only checks that catch this repo's most common defect shape — the command reports success and nothing lands on disk (#574, #525, #342, #335, #331, #508, #436, #585). #569 made that result actionable for a human: doctor() returns allPassed and the command exits 1. Two things still stopped anything else from using it. The registry was a local array inside doctor(), so no other caller could run it, and there was no --json, so a hook, a CI job or an agent could read the exit code but never which check failed or how to fix it.

This is phase 1 of #598: make the registry callable and the result parseable. Nothing else changes — same checks, same human output, same exit codes.

 teamai doctor
-  doctor()
-    resolve config, build the registry, run it and render — all inline
+  doctor(options)
+    resolveDoctorContext()                  exported; null when not initialized
+    buildChecks(ctx)                        exported; renders nothing
+    runChecks(checks, onResult?)            one loop for both modes
+    emitReport(report) | renderResult(...)  --json, or the rendering that was there

The two modes differ only in where the bytes go:

teamai doctor                     teamai doctor --json
  stdout  the lines you know        stdout  one JSON object, nothing else
  stderr  —                         stderr  every log line (setStderrOnly)
  exit    0 | 1                     exit    0 | 1   unchanged, from #569

--json emits the { name, ok, fix } shape the checks already carried, so the flag adds a channel rather than a second source of truth:

{
  "ok": false,
  "scope": "user",
  "checks": [
    { "name": "Team repo exists locally", "ok": true },
    {
      "name": "teamai hooks in claude settings",
      "ok": false,
      "fix": "Run `teamai hooks inject` to inject/update hooks"
    }
  ]
}

packages appears only when the team repo declares packages, and carries pkgDoctorReport's rendered lines as what they are — human text, not checks. notes appears only when there is an advisory; today that is the Codex trust-gate reminder, which would otherwise be lost in JSON mode.

This does not touch src/pull.ts, so it does not collide with #597.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

Branched from origin/main @ c674ffe.

  • npx tsc --noEmit passes
  • npx vitest run passes (233 files, 3278 tests)
  • Added/updated tests for the change

Unit, src/__tests__/doctor.test.ts — the 13 existing tests are untouched and still green, which is what proves the extraction did not move the human rendering. Five added:

✓ emits a single JSON object carrying every check
✓ carries the fix string of a failing check
✓ emits the same envelope before initialization
✓ buildChecks runs outside doctor and yields one hook check per enabled agent
✓ resolveDoctorContext returns a null context before initialization

The buildChecks test asserts consoleSpy was never called: building the registry has to render nothing, or phases 2 and 3 cannot reuse it. The JSON helper insists on exactly one console.log, so every JSON test also proves stdout purity rather than tolerating a leak.

E2E, src/__tests__/e2e/doctor-cli.test.ts, spawning the built CLI with a sandboxed HOME — 5 tests, two added, both parsing result.stdout whole:

✓ --json puts the report on stdout and nothing else
✓ --json keeps its envelope before initialization

End-to-end by hand against dist/index.js, with throwaway HOMEs. --json 2>/dev/null had to parse whole in every case, since that is the contract a CI job depends on:

scenario exit --json stdout
initialized, hooks present 0 ok: true, 4 checks, 373 bytes, parses
hooks missing from settings 1 ok: false, failing check carries its fix, 440 bytes, parses
not initialized 1 ok: false, scope: null, one check, 224 bytes, parses

Agent matrix, since toolPaths drives one hook check per tool:

claude      {"ok":true,"hooks":["teamai hooks in claude settings"]}
codex       {"ok":true,"hooks":["teamai hooks in codex settings"]}
codebuddy   {"ok":true,"hooks":["teamai hooks in codebuddy settings"]}
opencode    {"ok":true,"hooks":["teamai hooks in opencode settings"]}

Provider matrix, since the provider branch decides which checks exist:

git       ["Team repo exists locally","Team config (teamai.yaml) is valid","teamai hooks in claude settings","Env variables injected in shell profile"]
gitlab    ["GitLab token is configured", …]
github    ["gh CLI is installed","gh CLI is authenticated", …]

Output is English only; the sole non-ASCII characters are the ✔ ✖ → ℹ ⚠ glyphs already in use, and the JSON is pure ASCII.

Related Issues

Part of #598 — phase 1 of three. It does not close the issue: phase 2 (running the checks at the end of an interactive pull) and phase 3 (checking that the skills pull reported actually reached the tool directories) still stand.

Notes for Reviewers

What this unlocks is the point, not the flag. Phase 2 needs a registry it can build without rendering; phase 3 needs somewhere to put a delivery check. Both are now one call, and neither has to re-derive the toolPaths narrowing (#569) or the resolveHookScope baseDir (#264).

The issue names the export buildChecks(config, scope); this uses buildChecks(ctx). A context object is the honest version: the registry also needs toolPaths and baseDir, and a (config, scope) signature would force every caller to re-derive both — the second place that drifts on its own, which #598 rejects elsewhere. I will correct the issue text to match, including resolveDoctorContext, which is now part of the phase-2 contract.

Three things deliberately not done.

pkgDoctorReport still returns pre-rendered lines, so JSON carries them under packages instead of being refactored into checks. Promoting it would drag src/pkg/ and its tests into a PR about doctor. It is the obvious follow-up if a consumer needs those structured.

runChecks and the loop stay private. Phase 2 will want them, but exporting for a caller that does not exist yet is generality on credit; CheckResult and DoctorReport are exported, because those are the wire contract.

Check.check keeps its name, although a thunk named as a noun reads oddly now that the interface is public. Renaming it to run touches all eleven registry entries, and their diff currently reads "moved verbatim", which is what makes the extraction reviewable. Happy to do it in a follow-up.

The JSON keys off display names. Rewording a check's name is a breaking change for a script that greps it. A stable id per check would fix that and can land later without breaking { name, ok, fix }; it seemed premature before anyone consumes this.

setStderrOnly(true) is a process global that is never reset, matching hook-dispatch-cli.ts:187 and contribute-check.ts:702. It is reachable only through doctor(), and phase 2 will call buildChecks directly, so no in-process caller inherits it. Worth knowing before someone calls doctor({ json: true }) from inside another command.

The ja / ko / th README rows are mine and I do not speak those languages. They mirror the structure of the neighbouring row and only add the flag, but a native reader should confirm the phrasing. Dropping them and keeping only README.md / README.zh-CN.md would leave three of five tables stale, which seemed worse.

The checks that catch "reported success, nothing on disk" lived inside
doctor() as a local array, so nothing else could run them, and the only
machine-readable result was the exit code Tencent#569 added.

Extract resolveDoctorContext() and buildChecks(), which render nothing,
and add --json: one object on stdout, every log line on stderr, exit code
unchanged. Human output is byte-for-byte what it was.

@jeff-r2026 jeff-r2026 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM — approving. Clean phase-1 extraction: human output is behavior-equivalent (13 untouched tests prove it) and --json keeps stdout pure — checked every console.log is !jsonMode-guarded (setStderrOnly only redirects log.*), and resolveHookScope path is preserved.

@jeff-r2026
jeff-r2026 merged commit 0982976 into Tencent:main Sep 17, 2026
7 checks passed
jeff-r2026 added a commit that referenced this pull request Sep 17, 2026
…606)

src/doctor.ts imported LocalConfig twice — once in the top type-only
import (line 5) and once in the grouped import block from './types.js'.
TypeScript rejected this with TS2300 (Duplicate identifier 'LocalConfig'),
so `tsc --noEmit` failed and the main CI has been red since #599.

The two imports landed cleanly as a merge (no textual conflict) but
collided at the type level, so each PR's own branch build was green.

Drop the LocalConfig binding from line 5 and keep it in the grouped
block alongside TeamaiConfig, matching the surrounding style.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants