Repository navigation
Add doctor command - #22
Merged
Merged
Conversation
Summary by CodeRabbit
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Agent
participant CLI as CLI Handler
participant Doctor as Doctor Engine
participant Config as Config Parser
participant FS as File System
participant Git as Git Subprocess
User->>CLI: method doctor [--json]
CLI->>Doctor: runDoctor(root)
Doctor->>Config: Read & parse .method.json
alt config parse ok
Config-->>Doctor: config + schema ok
else parse failed
Config-->>Doctor: emit config-parse-failed
end
Doctor->>FS: Verify required repo structure
FS-->>Doctor: missing-directory / type mismatches
Doctor->>FS: Traverse packet/backlog files, validate YAML frontmatter
FS-->>Doctor: missing/unterminated/invalid frontmatter
Doctor->>Git: Inspect core.hooksPath & hook files
Git-->>Doctor: git-hooks-not-configured / missing hooks / unavailable
Doctor->>FS: Scan backlog lanes for orphaned items
FS-->>Doctor: orphaned-backlog-item warnings
Doctor->>Doctor: Aggregate checks & issues → DoctorReport
alt --json flag
Doctor-->>CLI: DoctorReport (JSON)
CLI-->>User: JSON.stringify(report)
else
Doctor-->>CLI: DoctorReport
CLI->>Doctor: renderDoctorText(report)
Doctor-->>CLI: Formatted text
CLI-->>User: Human-readable output
end
CLI-->>User: exit 1 if any error, else 0 (ok/warn)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 094c0464bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli-args.ts`:
- Line 180: The unknown-option throw in src/cli-args.ts should include doctor
usage guidance: update the throw at the Unknown option branch so the MethodError
includes usage('doctor') (either by passing usage('doctor') as additional
context/argument to MethodError or appending its output to the error message)
where the current throw new MethodError(`Unknown option: ${value}`) occurs;
reference the MethodError constructor and the usage function to add the doctor
usage text.
In `@src/doctor.ts`:
- Around line 82-131: The JSON parse-failure branch in inspectConfig currently
returns DEFAULT_PATHS which causes downstream bogus missing-file/directory
issues for repos with custom paths; change the catch block to return a
non-default empty/neutral paths value (e.g., an empty array or null-equivalent
your code expects) instead of DEFAULT_PATHS so you don't invent structure
errors, leaving the single createIssue('config-parse-failed', ...) intact;
update references to DEFAULT_PATHS in the catch return and ensure the rest of
the code that consumes InspectConfig (callers of inspectConfig) correctly
handles the empty/neutral paths value to avoid further false positives.
- Around line 203-228: The frontmatter detection currently only accepts
LF-delimited blocks and fails on CRLF or EOF-terminated closing delimiters;
update the logic that checks raw, end, and yamlBlock to (1) accept either
'---\n' or '---\r\n' as the opening delimiter (compute startOffset accordingly
instead of hardcoding 4), and (2) search for a closing delimiter using a
tolerant pattern like /\r?\n---(?:\r?\n|$)/ (or equivalent string search) so it
matches CRLF or LF and also a terminating '---' at EOF; then slice the YAML with
the computed startOffset and the found closing index to populate yamlBlock.
Reference: raw, end, yamlBlock, createIssue.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6586aec5-995f-47ee-bdd6-44c48ccfc676
📒 Files selected for processing (16)
ARCHITECTURE.mdREADME.mddocs/CLI.mddocs/MCP.mddocs/design/0038-doctor-command/doctor-command.mddocs/method/backlog/inbox/PROCESS_doctor-command.mddocs/method/retro/0038-doctor-command/doctor-command.mddocs/method/retro/0038-doctor-command/witness/verification.mdsrc/cli-args.tssrc/cli.tssrc/doctor.tssrc/domain.tssrc/mcp.tstests/cli.test.tstests/doctor.test.tstests/mcp.test.ts
💤 Files with no reviewable changes (1)
- docs/method/backlog/inbox/PROCESS_doctor-command.md
Owner
Author
|
Resolved the current PR #22 review round in two commits:
Validation after the fixes:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/doctor.ts`:
- Around line 448-465: The helper hasInstalledHook currently assumes the
provided path is a directory and will throw when given a regular file; update
hasInstalledHook(directory: string) to first return false (or throw a clear
error) if !isDirectoryPath(directory) so it does not call readdirSync on
non-directories, and then keep the existing file-check logic
(readdirSync/resolve/statSync/mode test) for actual directories; also update the
caller inspectGitHooks() to detect isDirectoryPath(directory) === false and emit
a specific "hooks path is not a directory" diagnostic instead of letting the
broad catch map the error to git-hooks-unavailable, ensuring you reference the
hasInstalledHook and inspectGitHooks functions when making these changes.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a106e8b5-b3ec-4b5e-b615-bb2719f9aa57
📒 Files selected for processing (4)
src/cli-args.tssrc/doctor.tstests/cli.test.tstests/doctor.test.ts
Owner
Author
|
Additional fix for the final hook-path review thread:
Validation after the fix:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/doctor.ts`:
- Around line 24-39: runDoctor currently builds issues only for checks that ran
(structure/frontmatter/backlog) but still calls summarizeCheck for every id in
DOCTOR_CHECKS, producing "ok" for checks that were skipped when
configInspection.paths === null; update runDoctor to detect which checks
actually ran (use configInspection.paths to determine that) and call
summarizeCheck with either a filtered list of only-run check ids or pass an
explicit flag/metadata to summarizeCheck indicating a skipped check so it can
return a skipped status/message; reference runDoctor, DOCTOR_CHECKS,
summarizeCheck, and configInspection.paths and ensure the final checks array
reflects skipped checks explicitly instead of reporting them as healthy.
- Around line 113-120: The current logic feeds {} into PathsSchema.safeParse
when the top-level JSON is non-object (arrays/primitives), causing DEFAULT_PATHS
to be applied and spawning bogus findings; change the branch so you only call
PathsSchema.safeParse when rawConfig is a plain object and actually has a
'paths' property (i.e., typeof rawConfig === 'object' && rawConfig !== null &&
!Array.isArray(rawConfig) && 'paths' in rawConfig), otherwise set paths: null;
keep using parsedPaths.success ? parsedPaths.data : null when you do parse so
that fallback DEFAULT_PATHS is only produced from a real config object (refer to
rawConfig, parsedPaths, and PathsSchema to locate the code).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c68a212-c036-41fe-a6cc-bc5140c75228
📒 Files selected for processing (2)
src/doctor.tstests/doctor.test.ts
Comment on lines
+24
to
+39
| export function runDoctor(root: string): DoctorReport { | ||
| const configInspection = inspectConfig(root); | ||
| const structureIssues = configInspection.paths === null ? [] : inspectStructure(root, configInspection.paths); | ||
| const frontmatterIssues = configInspection.paths === null ? [] : inspectFrontmatter(root, configInspection.paths); | ||
| const gitHookIssues = inspectGitHooks(root); | ||
| const backlogIssues = configInspection.paths === null ? [] : inspectBacklog(root, configInspection.paths); | ||
|
|
||
| const issues = [ | ||
| ...configInspection.issues, | ||
| ...structureIssues, | ||
| ...frontmatterIssues, | ||
| ...gitHookIssues, | ||
| ...backlogIssues, | ||
| ]; | ||
|
|
||
| const checks = DOCTOR_CHECKS.map((id) => summarizeCheck(id, issues)); |
There was a problem hiding this comment.
Don't report skipped path checks as healthy.
When paths is null, Lines 26-29 correctly skip structure, frontmatter, and backlog, but Line 39 still summarizes those checks as ok / No issues found. That is a false green report for checks that never ran. Surface them as explicitly skipped, or at least with a non-ok status/message, instead of folding them into the healthy path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/doctor.ts` around lines 24 - 39, runDoctor currently builds issues only
for checks that ran (structure/frontmatter/backlog) but still calls
summarizeCheck for every id in DOCTOR_CHECKS, producing "ok" for checks that
were skipped when configInspection.paths === null; update runDoctor to detect
which checks actually ran (use configInspection.paths to determine that) and
call summarizeCheck with either a filtered list of only-run check ids or pass an
explicit flag/metadata to summarizeCheck indicating a skipped check so it can
return a skipped status/message; reference runDoctor, DOCTOR_CHECKS,
summarizeCheck, and configInspection.paths and ensure the final checks array
reflects skipped checks explicitly instead of reporting them as healthy.
Comment on lines
+113
to
+120
| const parsedPaths = PathsSchema.safeParse( | ||
| rawConfig !== null && typeof rawConfig === 'object' | ||
| ? ('paths' in rawConfig ? (rawConfig as { paths?: unknown }).paths : {}) | ||
| : {}, | ||
| ); | ||
|
|
||
| return { | ||
| paths: parsedPaths.success ? parsedPaths.data : null, |
There was a problem hiding this comment.
Non-object .method.json still invents default-path findings.
If the file parses as JSON but the top level is not an object ([], "x", 42), this branch feeds {} into PathsSchema.safeParse() and recovers DEFAULT_PATHS. On a repo that actually uses custom paths, that recreates the bogus missing-directory / missing-file noise you already avoided for JSON parse failures. Only recover fallback paths from a real config object; otherwise keep paths: null and skip the path-based checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/doctor.ts` around lines 113 - 120, The current logic feeds {} into
PathsSchema.safeParse when the top-level JSON is non-object (arrays/primitives),
causing DEFAULT_PATHS to be applied and spawning bogus findings; change the
branch so you only call PathsSchema.safeParse when rawConfig is a plain object
and actually has a 'paths' property (i.e., typeof rawConfig === 'object' &&
rawConfig !== null && !Array.isArray(rawConfig) && 'paths' in rawConfig),
otherwise set paths: null; keep using parsedPaths.success ? parsedPaths.data :
null when you do parse so that fallback DEFAULT_PATHS is only produced from a
real config object (refer to rawConfig, parsedPaths, and PathsSchema to locate
the code).
4 of 6 tasks
This was referenced Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
doctorsurface for METHOD in both CLI and MCPWorkspaceinstance0038-doctor-commandwith retro and witness evidenceNotes
method doctornow treats missing empty backlog lane directories as acceptable instead of structural errorscore.hooksPathnow falls back to default git hook inspection instead of misreporting git metadata as unavailablemethod doctorcurrently reportswarnwith onlygit-hooks-not-configuredValidation
npm testnpm run buildgit diff --check./node_modules/.bin/tsx src/cli.ts doctor