feat(cli): add git warp doctor structural diagnostics#28
Conversation
7 diagnostic checks (repo-accessible, refs-consistent, coverage-complete, checkpoint-fresh, audit-consistent, clock-skew, hooks-installed) with budget enforcement, sorted findings, deterministic health derivation, and machine-readable JSON output. Includes --strict flag (exit 4 on warnings), text presenter with colored status icons, doctorSchema, golden-JSON unit tests, and BATS E2E tests.
- Place TODO(ts-cleanup) tags on the same line as wildcard casts - Fix circular JsonValue typedef for TypeScript compatibility - Add type annotations to doctor.test.js mock variables
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new read-only CLI command Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Handler
participant Ctx as DoctorContext
participant Reg as ALL_CHECKS Registry
participant Check as Individual Check
participant Pers as Persistence/Services
participant Presenter as Presenter
CLI->>Ctx: build context (persistence, graph, writer heads, policy)
CLI->>Reg: run ALL_CHECKS with deadline/budget
loop per check until deadline
Reg->>Check: execute check(ctx)
Check->>Pers: query refs, coverage, checkpoints, audit, hooks, clock
Pers-->>Check: return data / evidence
Check-->>Reg: return DoctorFinding(s)
end
Reg-->>CLI: aggregated findings
CLI->>CLI: assemble payload (health, summary, duration, exit code)
CLI->>Presenter: render payload (json or text)
Presenter-->>CLI: formatted output
CLI-->>CLI: exit with computed code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Release Preflight
If you tag this commit as |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@bin/cli/commands/doctor/checks.js`:
- Around line 291-301: collectWriterDates currently calls
ctx.persistence.getNodeInfo(head.sha) without guarding head.sha, which will
throw if head.sha is null; update the function (collectWriterDates) to skip or
filter out ctx.writerHeads entries with falsy head.sha before calling
ctx.persistence.getNodeInfo (e.g., continue/omit entries where !head.sha) so you
only call getNodeInfo with a valid SHA and avoid the CHECK_INTERNAL_ERROR during
the clock-skew check.
In `@bin/cli/commands/doctor/index.js`:
- Around line 142-152: The loop invoking check.fn(ctx) lacks an error boundary
so a thrown exception aborts the doctor run; wrap the call to check.fn(ctx) in a
try/catch around the code that measures checkStart/checkDuration and processes
resultArray (the block using checkStart, checkDuration, normalizeResult,
findings, and checksRun), and on catch push a finding built using the existing
CHECK_INTERNAL_ERROR code from codes.js (include context like
check.name/identifier and the caught error message), increment checksRun, set
durationMs appropriately, and continue to the next check so one failing check
doesn't crash the whole command.
In `@CHANGELOG.md`:
- Line 21: The changelog entry for "BATS E2E tests" incorrectly enumerates 4
scenarios; update the parenthetical in the line that starts with "**BATS E2E
tests**" to list all five scenarios (healthy JSON, human output, broken ref,
missing checkpoint, strict mode) so the text matches the actual tests.
In `@test/unit/cli/doctor.test.js`:
- Around line 72-81: The JSDoc for CLI_OPTIONS incorrectly claims it's a
DoctorPayload; update the annotation for CLI_OPTIONS to reflect the actual
CLI/options type (or a generic type) instead of DoctorPayload—locate the correct
type name in the doctor command's types file (or remove the specific type and
use /** `@type` {*} */) and replace DoctorPayload in the JSDoc so the symbol
CLI_OPTIONS is typed appropriately and no longer misleading.
🧹 Nitpick comments (8)
test/bats/cli-doctor.bats (1)
43-60: Unnecessarycdround-trips whengit -Cis already used.Lines 45-47 do
cd "${PROJECT_ROOT}", thengit -C "${TEST_REPO}" update-ref ..., thencd "${TEST_REPO}". Sincegit -CtargetsTEST_REPOdirectly, bothcdcommands are unnecessary. Same pattern appears in Test 4 (lines 64-66) and Test 5 (lines 82-84).That said, this follows the same pattern used in other BATS tests in this repo, so this may be intentional for consistency.
♻️ Simplified version (applicable to all three tests)
- cd "${PROJECT_ROOT}" || return 1 git -C "${TEST_REPO}" update-ref refs/warp/demo/writers/ghost deadbeefdeadbeefdeadbeefdeadbeefdeadbeef - cd "${TEST_REPO}" || return 1bin/cli/commands/doctor/index.js (1)
73-94: Three linear scans overfindingswhere one suffices.Lines 74-76 iterate
findingsthree times to count ok/warn/fail. With a single pass this is trivial and more efficient if the findings array ever grows.♻️ Single-pass counting
- const ok = findings.filter((f) => f.status === 'ok').length; - const warn = findings.filter((f) => f.status === 'warn').length; - const fail = findings.filter((f) => f.status === 'fail').length; + let ok = 0, warn = 0, fail = 0; + for (const f of findings) { + if (f.status === 'ok') ok++; + else if (f.status === 'warn') warn++; + else if (f.status === 'fail') fail++; + }test/unit/cli/doctor.test.js (2)
197-210: Sort-order test silently passes when preconditions aren't met.The
if (lastFailIdx >= 0 && firstOkIdx >= 0)guard on line 207 means the test body is skipped entirely if the mock setup doesn't actually produce bothfailandokfindings. A test that conditionally asserts nothing is fragile and can mask regressions.Consider asserting the preconditions unconditionally so the test fails loudly if the setup is wrong.
Proposed fix
const statuses = result.payload.findings.map((/** `@type` {*} */ f) => f.status); - // fail should come before warn, which comes before ok - const firstOkIdx = statuses.indexOf('ok'); - const lastFailIdx = statuses.lastIndexOf('fail'); - if (lastFailIdx >= 0 && firstOkIdx >= 0) { - expect(lastFailIdx).toBeLessThan(firstOkIdx); - } + // Precondition: the mock must produce both fail and ok findings + expect(statuses).toContain('fail'); + expect(statuses).toContain('ok'); + + const firstOkIdx = statuses.indexOf('ok'); + const lastFailIdx = statuses.lastIndexOf('fail'); + expect(lastFailIdx).toBeLessThan(firstOkIdx);
89-107: Dynamic import insidebeforeEachis redundant after the first run.ESM modules are cached after the first
await import(...). Subsequent calls in laterbeforeEachinvocations return the same cached module, sohandleDoctoris always the same reference. The dynamic import is only needed once. This isn't a bug (it works correctly), but it's misleading — it reads as though the module is re-evaluated per test to "pick up mocks," which isn't what happens. The comment on line 104 reinforces that misconception.If the intent is just to load the module after
vi.mockhoisting takes effect, a single top-levelawait import(...)(like the one on line 28 forshared.js) would be clearer.bin/cli/commands/doctor/checks.js (4)
43-62:checkRepoAccessibleinstantiatesHealthCheckServiceandClockAdapterdirectly, bypassing any injected/mocked versions.The test file mocks both
HealthCheckServiceandClockAdapterat the module level, so this works for testing. However, this check creates its ownHealthCheckServiceusingctx.persistencerather than receiving it via the context — unlike the other checks that only usectxmembers. IfHealthCheckServiceorClockAdapteris ever replaced or configured differently at the CLI layer, this check won't pick up those changes.This is acceptable for now given the mocking strategy, but worth noting for future maintainability.
191-213: Checkpoint with unparseable date is reported asok.When
ageHoursisnull(i.e., the date couldn't be parsed),buildCheckpointFindingreturns anokstatus with message "Checkpoint exists (age unknown)". An unparseable date could indicate data corruption or an unexpected format. Returningokhere could mask a real issue — awarnwith a code likeCHECKPOINT_OK(or a dedicated code) might be more appropriate for "age unknown."This is a design-level observation; up to you whether to address it now.
348-356:checkHooksInstalleddeclared as sync but the JSDoc andALL_CHECKStype expectPromise.The function manually wraps return values in
Promise.resolve()to match the async contract. This works but is inconsistent with the other six checks, which are allasync. Marking itasyncwould remove the need for manual wrapping and align the style.Proposed fix
-export function checkHooksInstalled(ctx) { +export async function checkHooksInstalled(ctx) { try { const installer = createHookInstaller(); const s = installer.getHookStatus(ctx.repoPath); - return Promise.resolve(buildHookFinding(s)); + return buildHookFinding(s); } catch (/** `@type` {*} */ err) { // TODO(ts-cleanup): narrow error type - return Promise.resolve(internalError('hooks-installed', err)); + return internalError('hooks-installed', err); } }
240-254:auditIds.includes(id)is an O(n·m) lookup — fine for small sets butSetwould be more idiomatic.
writerIdsis already aSet, but the comparison targetauditIdsis an array, making the.filter(id => !auditIds.includes(id))check linear per writer. For consistency and clarity, convertingauditIdsto aSetwould match the existing pattern.Proposed fix
const writerIds = new Set(ctx.writerHeads.map((h) => h.writerId)); - const auditIds = auditRefs.map((r) => r.slice(auditPrefix.length)).filter((id) => id.length > 0); - const missing = [...writerIds].filter((id) => !auditIds.includes(id)); + const auditIdSet = new Set(auditRefs.map((r) => r.slice(auditPrefix.length)).filter((id) => id.length > 0)); + const missing = [...writerIds].filter((id) => !auditIdSet.has(id));
- Single-pass counting in assemblePayload instead of 3 filter scans - Error boundary in runChecks so one failing check doesn't crash doctor - Guard falsy head.sha in collectWriterDates before calling getNodeInfo - Use Set for auditId lookups instead of Array.includes - Remove unnecessary cd round-trips in BATS tests (git -C suffices) - Assert preconditions unconditionally in sort-order test - Fix CLI_OPTIONS JSDoc type (CliOptions, not DoctorPayload) - Clarify ESM cache comment in beforeEach - Fix CHANGELOG BATS scenario enumeration (list all 5)
Release Preflight
If you tag this commit as |
- Add seed-doctor-graph.js that materializes (creates checkpoint + coverage refs) so the "healthy graph" tests pass - Install hooks in healthy-graph tests so hooks-installed check passes - Write dangling ref directly to .git/refs/ instead of git update-ref (which rejects nonexistent objects in some git versions)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bin/cli/commands/doctor/checks.js`:
- Around line 119-125: In checkCoverageComplete, avoid calling
ctx.persistence.isAncestor with a falsy head.sha; add a null/falsy guard inside
the loop over ctx.writerHeads (e.g., if (!head.sha) continue) before invoking
ctx.persistence.isAncestor(head.sha, coverageSha) so heads without a sha are
skipped (consistent with checkRefsConsistent and collectWriterDates) and will
not cause a CHECK_INTERNAL_ERROR.
🧹 Nitpick comments (2)
bin/cli/commands/doctor/checks.js (1)
351-359:checkHooksInstalledis notasyncbut declared to returnPromise<DoctorFinding>.Every other check function is
async, naturally returning a Promise. This one manually wraps results inPromise.resolve. Making itasyncwould be more consistent and slightly simpler.Suggested refactor
-export function checkHooksInstalled(ctx) { +export async function checkHooksInstalled(ctx) { try { const installer = createHookInstaller(); const s = installer.getHookStatus(ctx.repoPath); - return Promise.resolve(buildHookFinding(s)); + return buildHookFinding(s); } catch (/** `@type` {*} */ err) { // TODO(ts-cleanup): narrow error type - return Promise.resolve(internalError('hooks-installed', err)); + return internalError('hooks-installed', err); } }test/unit/cli/doctor.test.js (1)
197-211: Sort-order test:nodeExists(false)affects multiple checks.Setting
nodeExiststo always returnfalsewill cause failures in bothcheckRefsConsistentandprobeAuditRefs(audit dangling detection). This is fine for testing sort order since the test only asserts thatfailfindings precedeokfindings, but be aware that if the sort logic or finding shapes change, this broad mock could make the test fragile.
Release Preflight
If you tag this commit as |
Release Preflight
If you tag this commit as |
- Add _run_json helper that captures stdout only (BATS 1.8+ merges stderr into $output, corrupting JSON when git emits diagnostics) - Remove install-hooks from healthy-graph tests (template path doesn't resolve inside Docker container) - Relax healthy-graph assertion: check fail==0 in JSON instead of exit code 0 (hooks-installed warning is expected without template) - Human output test checks content only, not exit code
Release Preflight
If you tag this commit as |
createHookInstaller() resolved templateDir to <root>/hooks/ which does not exist. The actual template lives at src/hooks/post-merge.sh. Fix the path to resolve to <root>/src/hooks/. Restore BATS tests to properly install hooks and assert health == ok. Keep _run_json helper for stderr isolation in JSON-parsing tests.
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/bats/helpers/seed-doctor-graph.js`:
- Around line 1-5: The file-level doc comment in seed-doctor-graph.js
incorrectly states "and installs hooks" even though the script does not install
hooks; update the header comment (the top block comment in seed-doctor-graph.js)
to remove or reword the "installs hooks" clause so it accurately reflects
behavior (e.g., "Creates patches, materializes (checkpoint + coverage)." or note
that hooks are installed by the BATS test) so the comment matches the actual
implementation.
- checkCoverageComplete: null-sha writer heads reported as missing (not silently skipped) with regression test - checkClockSkew: regression test for null-sha guard in collectWriterDates - checkHooksInstalled: made async (await Promise.resolve pattern) - Sort-order test: exercises all three status tiers with targeted mocks; asserts full three-key comparator (status > impact > id) - seed-doctor-graph.js: removed stale "installs hooks" doc comment - CHANGELOG: added Fixed section for v10.13.0
Add Record<string,number> annotations for STATUS_ORDER/IMPACT_ORDER maps and cast findings to any for indexing. Cast checkCoverageComplete return to any for evidence.missingWriters access.
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bin/cli/commands/doctor/checks.js`:
- Around line 67-101: The OK message in checkRefsConsistent wrongly uses
allRefs.length even when some writer heads have null sha; update the logic to
count only refs with a non-null sha (e.g., compute checkedCount by filtering
allRefs for entries with sha or build allRefsFiltered =
ctx.writerHeads.map(...).filter(r => r.sha)) and use that checkedCount in the
success message and any related text so the message accurately reflects how many
refs were actually verified; leave the dangling-ref detection loop and findings
unchanged.
🧹 Nitpick comments (3)
bin/cli/commands/doctor/checks.js (2)
355-363: Unnecessaryawait Promise.resolve(…)wrapping.
buildHookFindingandinternalErrorare synchronous. In anasyncfunction, simply returning a value already wraps it in a resolved promise. Theawait Promise.resolve(…)pattern adds noise without benefit.Proposed simplification
export async function checkHooksInstalled(ctx) { try { const installer = createHookInstaller(); const s = installer.getHookStatus(ctx.repoPath); - return await Promise.resolve(buildHookFinding(s)); + return buildHookFinding(s); } catch (/** `@type` {*} */ err) { // TODO(ts-cleanup): narrow error type - return await Promise.resolve(internalError('hooks-installed', err)); + return internalError('hooks-installed', err); } }
43-62:ClockAdapter.global()andnew HealthCheckService(…)instantiated on every invocation.This is fine for a diagnostic command that runs once, but worth noting:
checkRepoAccessiblecreates a freshHealthCheckServiceand clock adapter each call. If the check registry ever supports re-runs or batching, consider injecting these viactx.test/unit/cli/doctor.test.js (1)
110-156: Healthy-graph test doesn't verifyCLOCK_SYNCEDorAUDIT_OKcodes.Lines 150-155 assert the presence of
REPO_OK,REFS_OK,COVERAGE_OK,CHECKPOINT_OK, andHOOKS_OKbut omitCLOCK_SYNCEDandAUDIT_OK. Since the mock has only one writer,checkClockSkewreturnsCLOCK_SYNCED(skipped path), andcheckAuditConsistentreturnsAUDIT_OK— both would pass if asserted. Consider adding them for completeness.expect(codes).toContain(CODES.CHECKPOINT_OK); expect(codes).toContain(CODES.HOOKS_OK); + expect(codes).toContain(CODES.CLOCK_SYNCED); + expect(codes).toContain(CODES.AUDIT_OK);
checkRefsConsistent now filters writer heads with null sha into a separate `checkable` array so the OK message accurately reports how many refs were actually verified. Adds regression test confirming the count excludes null-sha entries.
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
Release Preflight
If you tag this commit as |
- Remove redundant `await Promise.resolve()` wrapping in checkHooksInstalled - Add CLOCK_SYNCED and AUDIT_OK assertions to healthy-graph test - Update CHANGELOG to reflect simplified async pattern
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
Release Preflight
If you tag this commit as |
Root causes: - collectWriterHeads crashed when readRef encountered a ref pointing to a missing object (git show-ref exits 128, not 1, for dangling refs) - seed-doctor-graph.js called materialize() but not createCheckpoint() or syncCoverage(), so doctor always found missing checkpoint/coverage - _run_json BATS helper had status capture bug: `|| true` made $? always 0, masking real exit codes Fixes: - collectWriterHeads now catches readRef failures and includes the writer head with sha=null for downstream checks to report - checkRefsConsistent reports null-sha heads as REFS_DANGLING_OBJECT - seed-doctor-graph.js calls createCheckpoint() + syncCoverage() - _run_json properly captures exit code via local variable
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
Release Preflight
If you tag this commit as |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bin/cli/commands/doctor/index.js`:
- Around line 151-174: The duration timer checkStart is declared inside the try
so the catch path reports 0; move hoisting of the start time by declaring and
assigning checkStart = Date.now() before the try that calls check.fn(ctx), then
inside both the try (after await) and the catch compute checkDuration =
Date.now() - checkStart (keeping the existing checkDuration variable) so that
findings pushed in both the success path (via normalizeResult loop) and the
failure push include the actual elapsed time; ensure checksRun++ behavior is
preserved.
🧹 Nitpick comments (2)
bin/cli/commands/doctor/checks.js (2)
204-211: Considerwarninstead ofokwhen checkpoint age is unknown.When
ageHoursis null (date missing or unparseable), the finding is reported asokwith "age unknown." Since the freshness policy can't actually be verified,warnwithCHECKPOINT_STALE(or a dedicated code) would be more defensive — a checkpoint with an unparseable date is unusual and worth surfacing.
239-253: Silent skip when audit ref resolves to null sha.If
readRefreturnsnullfor a listed audit ref (Line 241–242), the ref is silently skipped with no finding. Unlike writer heads incheckRefsConsistent(which reportREFS_DANGLING_OBJECTfor null shas), an unresolvable audit ref produces no diagnostic output. Consider emitting anAUDIT_DANGLINGfinding here for consistency.Proposed fix
const sha = await ctx.persistence.readRef(ref); if (!sha) { - continue; + findings.push({ + id: 'audit-consistent', status: 'warn', code: CODES.AUDIT_DANGLING, + impact: 'data_integrity', + message: `Audit ref ${ref} points to a missing or unreadable object`, + evidence: { ref }, + }); + continue; }
| let checkDuration; | ||
| try { | ||
| const checkStart = Date.now(); | ||
| const result = await check.fn(ctx); | ||
| checkDuration = Date.now() - checkStart; | ||
| checksRun++; | ||
|
|
||
| const resultArray = normalizeResult(result); | ||
| for (const f of resultArray) { | ||
| f.durationMs = checkDuration; | ||
| findings.push(f); | ||
| } | ||
| } catch (/** @type {*} TODO(ts-cleanup): narrow error type */ err) { | ||
| checkDuration = checkDuration ?? 0; | ||
| checksRun++; | ||
| findings.push({ | ||
| id: check.id, | ||
| status: 'fail', | ||
| code: CODES.CHECK_INTERNAL_ERROR, | ||
| impact: 'data_integrity', | ||
| message: `Internal error in ${check.id}: ${err?.message || String(err)}`, | ||
| durationMs: checkDuration, | ||
| }); | ||
| } |
There was a problem hiding this comment.
checkStart is block-scoped to try, so errored checks always report durationMs: 0.
const checkStart at Line 153 is inside the try block and inaccessible in catch. When check.fn(ctx) throws, checkDuration falls through to ?? 0 at Line 164 instead of reflecting actual elapsed time.
Proposed fix — hoist checkStart above try
let checkDuration;
+ const checkStart = Date.now();
try {
- const checkStart = Date.now();
const result = await check.fn(ctx);
checkDuration = Date.now() - checkStart;
checksRun++;
@@ ..
} catch (/** `@type` {*} TODO(ts-cleanup): narrow error type */ err) {
- checkDuration = checkDuration ?? 0;
+ checkDuration = Date.now() - checkStart;
checksRun++;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let checkDuration; | |
| try { | |
| const checkStart = Date.now(); | |
| const result = await check.fn(ctx); | |
| checkDuration = Date.now() - checkStart; | |
| checksRun++; | |
| const resultArray = normalizeResult(result); | |
| for (const f of resultArray) { | |
| f.durationMs = checkDuration; | |
| findings.push(f); | |
| } | |
| } catch (/** @type {*} TODO(ts-cleanup): narrow error type */ err) { | |
| checkDuration = checkDuration ?? 0; | |
| checksRun++; | |
| findings.push({ | |
| id: check.id, | |
| status: 'fail', | |
| code: CODES.CHECK_INTERNAL_ERROR, | |
| impact: 'data_integrity', | |
| message: `Internal error in ${check.id}: ${err?.message || String(err)}`, | |
| durationMs: checkDuration, | |
| }); | |
| } | |
| let checkDuration; | |
| const checkStart = Date.now(); | |
| try { | |
| const result = await check.fn(ctx); | |
| checkDuration = Date.now() - checkStart; | |
| checksRun++; | |
| const resultArray = normalizeResult(result); | |
| for (const f of resultArray) { | |
| f.durationMs = checkDuration; | |
| findings.push(f); | |
| } | |
| } catch (/** `@type` {*} TODO(ts-cleanup): narrow error type */ err) { | |
| checkDuration = Date.now() - checkStart; | |
| checksRun++; | |
| findings.push({ | |
| id: check.id, | |
| status: 'fail', | |
| code: CODES.CHECK_INTERNAL_ERROR, | |
| impact: 'data_integrity', | |
| message: `Internal error in ${check.id}: ${err?.message || String(err)}`, | |
| durationMs: checkDuration, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@bin/cli/commands/doctor/index.js` around lines 151 - 174, The duration timer
checkStart is declared inside the try so the catch path reports 0; move hoisting
of the start time by declaring and assigning checkStart = Date.now() before the
try that calls check.fn(ctx), then inside both the try (after await) and the
catch compute checkDuration = Date.now() - checkStart (keeping the existing
checkDuration variable) so that findings pushed in both the success path (via
normalizeResult loop) and the failure push include the actual elapsed time;
ensure checksRun++ behavior is preserved.
1 participant
Summary
git warp doctorcommand with 7 diagnostic checks: repo-accessible, refs-consistent, coverage-complete, checkpoint-fresh, audit-consistent, clock-skew, hooks-installed--jsonoutput with versionedDoctorPayload(v1), policy echo, sorted findings, deterministic health derivation--strictflag treats warnings as failures (exit 4 vs exit 3)Test plan
doctorSchemaunit tests (3 tests in schemas.test.js)Summary by CodeRabbit
New Features
Documentation
Tests
Chores