Skip to content

feat(doctor): add typed diagnostics domain#1225

Open
decode2 wants to merge 2 commits into
Gentleman-Programming:mainfrom
decode2:feat/1197-doctor-01-domain
Open

feat(doctor): add typed diagnostics domain#1225
decode2 wants to merge 2 commits into
Gentleman-Programming:mainfrom
decode2:feat/1197-doctor-01-domain

Conversation

@decode2

@decode2 decode2 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1197

PR Type

  • type:feature - New feature
  • type:bug - Bug fix
  • type:docs - Documentation only
  • type:refactor - Code refactoring
  • type:chore - Maintenance/tooling
  • type:breaking-change - Breaking change

Summary

  • Introduces presentation-independent typed doctor check IDs, statuses, evidence, and conservatively classified remedy metadata.
  • Adds deterministic read-only execution with one PATH snapshot per run and per-check panic isolation.
  • Preserves the exact existing CLI output and starts the sequential main-targeted delivery for issue feat(tui): add doctor diagnostics with safe managed repairs #1197.

Changes

File Change
internal/doctor/doctor.go Adds typed diagnostic/remedy contracts, eligibility and platform metadata, and the ordered panic-isolating runner.
internal/doctor/doctor_test.go Covers IDs, order, continuation, redacted panic evidence, remedy classification, and empty reports.
internal/cli/doctor.go Adapts existing probes/rendering to the shared model and snapshots PATH once per run.
internal/cli/doctor_test.go Preserves probe coverage and proves byte-for-byte rendering compatibility.

Test Plan

go test ./internal/doctor -count=20
go test ./internal/cli -run '^(TestCheckOneTool_|TestCheckStateJSON_|TestCheckEngramReachable_|TestCheckDiskSpace_|TestRunDoctor_|TestRenderDoctorReport)' -count=20
go test ./internal/cli -run '^TestRunDoctor_IntegrationAllMocked$' -count=20
go build ./...
go vet ./...
go test ./...
  • Domain tests pass 20 consecutive runs.
  • Focused CLI doctor tests pass 20 consecutive runs.
  • Exact CLI output regression passes 20 consecutive runs.
  • go build ./... passes.
  • go vet ./... passes.
  • Full go test ./... passes locally. Existing unrelated Windows platform/config failures remain; the affected doctor package passes.
  • git diff --check upstream/main passes.
  • Fresh private reliability review completed with no findings.

Contributor Checklist

  • Linked approved issue feat(tui): add doctor diagnostics with safe managed repairs #1197 with Closes #1197.
  • Kept the PR within 400 changed lines.
  • Requested type:feature as the sole type:* label.
  • Kept tests with the behavior they verify.
  • Kept this slice read-only with no TUI, executor, or mutation behavior.
  • Used conventional commits with no attribution trailers.

Chain Context

Field Value
Strategy Stacked-to-main, Slice 1/5
Base main at e4a085327ad2d2a8912ebf5cce0148ba0035324e
Depends on None
Follow-ups Executor primitives, managed sync repair, TUI Health/recheck, then remaining managed capabilities and docs
Review budget 378 / 400 changed lines (312+, 66-)
End boundary 838da6ec11ac163c0e1d54af3d8b2521d1cd2b39
main
  -> Slice 1/5: typed doctor domain and read-only runner (this PR)
    -> Slice 2/5: allowlisted executor primitives
      -> Slice 3/5: managed sync repair
        -> Slice 4/5: TUI Health and recheck flow
          -> Slice 5/5: remaining capabilities, integration, and docs

Scope

  • Includes typed doctor domain, stable IDs, classified remedy metadata, deterministic read-only execution, CLI adaptation, and focused tests.
  • Excludes executor behavior, TUI work, mutations, managed repairs, and convergence handling.

Boundaries

  • Start: upstream/main at e4a0853.
  • End: 838da6e, changing only the four files listed above.
  • Rollback: revert 838da6e and 39e09ab.
  • Dependencies: none; this slice can merge independently.

@decode2

decode2 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer action requested: please apply the sole type:feature label required by CI.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c11e1c8f-0724-402d-91df-cc72a7278935

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds a shared typed doctor diagnostics package, migrates CLI checks to its runner and result model, and updates CLI and package tests for structured remedies, stable IDs, panic-safe ordering, and exact report output.

Changes

Doctor diagnostics model

Layer / File(s) Summary
Shared diagnostic contracts and runner
internal/doctor/doctor.go, internal/doctor/doctor_test.go
Defines typed check IDs, statuses, classified remedies, results, reports, and ordered panic-safe execution with unit coverage.
CLI doctor integration
internal/cli/doctor.go
Aliases shared types, executes ordered doctor.Check values through doctor.Runner, returns standardized IDs and structured remedies, and renders optional remedy descriptions.
CLI doctor regression coverage
internal/cli/doctor_test.go
Updates tool, Engram, disk, and integration assertions for nil remedies, exact report output, PATH snapshot behavior, and omitted remedy metadata.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: alan-thegentleman

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR delivers the shared typed doctor model and read-only CLI integration for this initial slice, with later repair and TUI work deferred by design.
Out of Scope Changes check ✅ Passed The changes stay focused on doctor diagnostics types, CLI adaptation, and tests, with no unrelated functionality added.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: introducing a typed doctor diagnostics domain.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/doctor/doctor.go`:
- Around line 71-79: Update Runner.Run to recover panics independently around
each check.Run(ctx) invocation, allowing subsequent checks to execute and the
report to be returned. Convert a recovered panic into a consistent failed Result
for that check, preserving its check.ID and including the panic details via fmt
as indicated; keep normal check results unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 00245c4a-98ed-41d3-b758-72ad0b689e91

📥 Commits

Reviewing files that changed from the base of the PR and between e4a0853 and 39e09ab.

📒 Files selected for processing (4)
  • internal/cli/doctor.go
  • internal/cli/doctor_test.go
  • internal/doctor/doctor.go
  • internal/doctor/doctor_test.go

Comment thread internal/doctor/doctor.go
@decode2

decode2 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Implemented the Slice 1 follow-up in 838da6e:

  • classified remedies by category, action mode, eligibility, and supported platforms, with unsafe/unbounded remedies manual-only;
  • snapshots PATH once per doctor run;
  • isolates each check panic, continues in order, and emits redacted generic evidence;
  • documents Runner.Run and keeps rendering read-only and byte-for-byte compatible.

Verified domain, focused CLI, and exact-output tests with -count=20; go build ./..., go vet ./..., and git diff --check upstream/main pass. Final diff: 312+, 66- (378 lines).

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

@decode2 Thanks for the detailed update on the Slice 1 follow-up work — the remedy classification, PATH snapshotting, panic isolation, and byte-for-byte compatible rendering all sound like solid improvements. I'll review the new commit now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

feat(tui): add doctor diagnostics with safe managed repairs

1 participant