Skip to content

feat: /doctor self-diagnostics - #43

Draft
BabyKoan wants to merge 10 commits into
DenizOkcu:mainfrom
BabyKoan:koan/implement-26
Draft

BabyKoan wants to merge 10 commits into
DenizOkcu:mainfrom
BabyKoan:koan/implement-26

Conversation

@BabyKoan

@BabyKoan BabyKoan commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds /doctor self-diagnostics. /doctor runs read-only checks for settings, providers, models, Node, ripgrep, LSP/MCP servers, skills, and context files, grouping results by severity. /doctor --full includes a soft provider reachability check. Startup sanity/log rotation/session pruning were removed per reviewer feedback; this PR now ships the command only.

Closes #26

Changes

  • New src/cli/doctor/types.ts and checks.ts modules.
  • New src/cli/commands/doctorCommand.ts command handler and formatter.
  • Registered /doctor and /doctor --full in commands.ts and commandHelp.ts.
  • Exported commandExists from lspSettings.ts for reuse.
  • Added tests for checks, formatter, and command registration.
  • Updated README command list.

Test plan

  • npm run typecheck passes.
  • npm run lint passes.
  • npm test passes.

@DenizOkcu

Copy link
Copy Markdown
Owner

This is a genuinely nice feature — a /doctor self-diagnostics command is a great fit for the tool, and the severity grouping is well thought out. The breadth of checks (settings, providers, Node, ripgrep, LSP/MCP, skills, context files) is impressive. 👏

Deferring rather than merging, for reasons specific to scope and one important safety concern:

  • Startup side effects: the PR adds runStartupSanity() to chat.tsx, which silently prunes session files older than 30 days and rotates oversized logs on every launch. That is automatic, irreversible data deletion baked into a "simple" CLI. We are not comfortable shipping silent file deletion as startup behavior. We would want to either drop the startup pruning entirely or move it behind an explicit opt-in.
  • Size and new surface: at ~540 lines it is the largest open PR and introduces a whole new user-facing command plus a new src/cli/doctor/ module. That is more scope than we want to absorb in this round, which was focused on small, low-risk fixes.

The diagnostics command itself is something we would love to have. If you are up for a follow-up, a slimmer version that ships only the /doctor command (no startup sanity, no session pruning) would be a much easier yes. Thanks for the substantial work!

@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-26 was rebased onto main and review feedback was applied.

Changes applied

  • Summary:
  • Removed startup sanity call from src/cli/commands/chat.tsx. No automatic log rotation or session pruning on launch. Reviewer objected to silent, irreversible data deletion as startup behavior.
  • Deleted src/cli/doctor/sanity.ts and tests/cli/doctor/sanity.test.ts. These only implemented the startup cleanup behavior, so they are gone.
  • Kept /doctor and /doctor --full command, diagnostics checks, formatter, command registration, README listing, and existing tests. This matches the requested slimmer scope (command-only, no startup side effects).

Stats

11 files changed, 415 insertions(+), 31 deletions(-)
Actions performed
  • Already-solved check: skipped (Claude call failed)
  • Rebased koan/implement-26 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-26 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@BabyKoan
BabyKoan force-pushed the koan/implement-26 branch from f3f1442 to d21c13f Compare June 27, 2026 09:07
BabyKoan added a commit to BabyKoan/haze that referenced this pull request Jun 27, 2026
@atoomic

atoomic commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan review

@BabyKoan

Copy link
Copy Markdown
Contributor Author

@DenizOkcu: This is a genuinely nice feature — a /doctor self-diagnostics command is a great fit for the tool,...

Startup sanity and session pruning were removed in the rebase. chat.tsx no longer calls any startup cleanup, and src/cli/doctor/sanity.ts is gone. The branch now contains only the /doctor command, diagnostics checks, formatter, registration, tests, and README update. Scope matches the slimmer command-only version requested.

@BabyKoan

BabyKoan commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Previous review — superseded by a newer review below.

@atoomic

atoomic commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan rebase

@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase completed; review feedback not applied

Branch koan/implement-26 was rebased onto main, but review feedback could not be applied automatically.

Warning

Review feedback was NOT applied — the feedback step errored. The reviewer comments above still need to be addressed: re-run /rebase or apply them manually.

Stats

11 files changed, 415 insertions(+), 31 deletions(-)
Actions performed
  • Already-solved check: skipped (Claude call failed)

  • Rebased koan/implement-26 onto upstream/main

  • Review feedback step failed: Exit code 1: OCGO model mapping enabled for claude (/home/baby/.config/ocgo/model-mapping.json)
    haiku -> deepseek-v4-flash
    opus -> glm-5.2
    sonnet -> kimi-k2.7-code
    Error: exit status 1
    Usage:

  • Review feedback failed (continuing with rebase)

  • Review feedback step errored (Exit code 1: OCGO model mapping enabled for claude (/home/baby/.config/ocgo/model-mapping.json)
    haiku -> deepseek-v4-flash
    opus -> glm-5.2
    sonnet -> kimi-k2.7-code
    Error: exit status 1
    Usage:
    ocgo launch claude [-- claude args...] [flags]

Flags:
-h, --help help for claude
--model string OpenCode Go model ID
--yes Allow Claude Code to skip permission prompts); pushing rebase without feedback changes

  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-26 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@atoomic

atoomic commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@BabyKoan rebase

@BabyKoan BabyKoan changed the title feat: /doctor self-diagnostics and startup sanity feat: /doctor self-diagnostics Jun 30, 2026
@BabyKoan

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/implement-26 was rebased onto main and review feedback was applied.

Changes applied

  • hen .haze/ does not exist; avoids write side effects.
  • Fixed checkProviderReachable resource cleanup. Moved clearTimeout(timer) into finally so the abort timer is cancelled on fetch errors.
  • Report invalid raw MCP entries. Added mcpServerValidationError in mcpSettings.ts; checkMcpServers now validates raw settings.mcpServers before normalization and reports each invalid server by name with a specific error.
  • Isolated doctor tests from host filesystem and PATH. Rewrote tests/cli/commands/doctor.test.ts and tests/cli/doctor/checks.test.ts with temp HAZE_DIR and mocked commandExists, readContextFiles, and loadSkillRegistry. Added edge cases for malformed settings.json, missing .haze/, invalid MCP, missing LSP, and reachability timer cleanup.
  • Mocked doctorCommand in tests/cli/commands.test.ts so /doctor registration tests do not touch real ~/.haze.
  • Updated PR title/description to the slimmer command-only scope (startup sanity already removed).

Stats

12 files changed, 634 insertions(+), 2 deletions(-)
Actions performed
  • Already-solved check: skipped (Claude call failed)
  • Rebased koan/implement-26 onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/implement-26 to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@BabyKoan
BabyKoan force-pushed the koan/implement-26 branch from d21c13f to f56a318 Compare June 30, 2026 03:55
@BabyKoan

Copy link
Copy Markdown
Contributor Author

PR Review — feat: /doctor self-diagnostics

Solid command-only implementation that addresses the prior review feedback, but two user-facing/contract issues remain before merge.

What's done well:

  • Prior review findings were applied: tests use temp HAZE_DIR and mocks, checkSettingsValid no longer calls ensureDir, checkHazeDirWritable uses fs.access instead of a write probe, the abort timer is cleared in finally, and raw MCP entries are validated via mcpServerValidationError.
  • Severity-grouped report formatter is concise and scannable.
  • Command registration and /help integration follow the existing slash-command pattern.
  • No package-lock.json noise remains in the diff.

What still needs attention:

  • checkSkillsValid still creates ~/.haze/skills via loadSkillRegistry, undermining the read-only framing.
  • The .haze/ writable hint tells users to configure HAZE_DIR, which is not actually configurable.
  • The fixable field on CheckResult is unused.
  • Extra arguments to /doctor are silently ignored.

🟡 Important

1. checkSkillsValid still creates ~/.haze/skills
src/cli/doctor/checks.ts:159-161

loadSkillRegistry() calls fs.ensureDir(GLOBAL_SKILLS_DIR) before reading, so /doctor will create ~/.haze/skills on first run. This contradicts the PR's "read-only checks" framing and the prior review feedback that called out write side effects in diagnostics.

To keep /doctor read-only, guard the call: if GLOBAL_SKILLS_DIR does not exist, return info that no skills are configured instead of ensuring the directory. Only call loadSkillRegistry() when the directory already exists.

export async function checkSkillsValid(): Promise<CheckResult> {
  try {
    const registry = await loadSkillRegistry();
    return result('skills parse & validate', 'ok', `${registry.skills.size} skill(s) loaded successfully.`);
  } catch (error) {
2. Hint suggests configurable HAZE_DIR that does not exist
src/cli/doctor/checks.ts:84

The hint tells users to "set a writable HAZE_DIR", but src/config/paths.ts hardcodes HAZE_DIR to path.join(os.homedir(), '.haze') with no env-var override. The remediation advice is therefore impossible to follow and will confuse users who hit a permission error.

Change the hint to match reality, e.g. "Check permissions on ~/.haze or run Haze as a user with write access to the home directory."

'Check permissions on ~/.haze or set a writable HAZE_DIR.',

🟢 Suggestions

1. fixable field is declared but never used
src/cli/doctor/types.ts:8

CheckResult.fixable is only assigned once (in checkSettingsValid) and never consumed by the formatter or command. Either wire it into formatDoctorReport (e.g. append "(fixable)" to hints) or remove it from the type to avoid half-implemented surface area.

export interface CheckResult {
  name: string;
  severity: CheckSeverity;
  message: string;
  hint?: string;
  fixable?: boolean;
}
2. Unknown /doctor arguments are silently ignored
src/cli/commands/doctorCommand.ts:77

/doctor --foo or /doctor --full extra sets full = false and runs the default report without telling the user the argument was not recognized. Add a small validation so users get feedback when they mistype the only supported flag.

export async function handleDoctorCommand(args: string, ctx: CommandContext): Promise<CommandResult> {
  const full = args.trim() === '--full';
  const results = await runDoctorChecks(ctx.settings, {full});
  ctx.addSystemMessage(formatDoctorReport(results));
  return 'handled';
}

Checklist

  • No hardcoded secrets or credentials
  • Input validation at CLI boundaries
  • Error handling and resource cleanup
  • Test isolation and filesystem mocking
  • No unrelated changes or lockfile noise
  • Backward compatibility preserved
  • Read-only diagnostic contract respected — warning #1
  • No YAGNI or half-implemented surface area — suggestion #1, suggestion #2

To rebase specific severity levels, mention me: @BabyKoan rebase critical (fixes 🔴 only), @BabyKoan rebase important (fixes 🔴 + 🟡), or just @BabyKoan rebase for all.


Silent Failure Analysis

🟠 **HIGH** — unhandled promise rejection / fire-and-forget composition
src/cli/commands/doctorCommand.ts:60-72

Risk: Promise.all rejects as soon as any single check throws, so one unexpected failure in a check like checkProvidersConfigured aborts the entire diagnostic report instead of surfacing that check as critical.

export async function runDoctorChecks(
  settings: CommandContext['settings'],
  options: {full?: boolean} = {},
): Promise<CheckResult[]> {
  return await Promise.all([
    checkSettingsValid(),
    checkProvidersConfigured(settings),
    ...
  ]);
}

Fix: Wrap each check promise with .catch(e => result(name, 'critical', ...)) so the report always completes and individual failures are visible.

🟡 **MEDIUM** — silent argument ignore
src/cli/commands/doctorCommand.ts:75-77

Risk: Any argument other than exactly '--full' (e.g. '/doctor --full --verbose' or a typo) silently falls back to the default non-full run without warning the user.

export async function handleDoctorCommand(args: string, ctx: CommandContext): Promise<CommandResult> {
  const full = args.trim() === '--full';
  const results = await runDoctorChecks(ctx.settings, {full});

Fix: Validate args and return an error or warning when unrecognized flags are provided.


Automated review by Kōan (Claude) HEAD=f56a318 25 min 37s

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.

🩺 /doctor: self-diagnostics + startup sanity (Complexity 5/10 · Value 9/10)

3 participants