feat(doctor): publish actionable JSON remediation - #4611
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThe doctor command now emits a schema-validated JSON report with stable check metadata, remediation prompts, deterministic summaries, and CI-compatible exit codes. Documentation, generated agent guidance, changelog entries, and contract tests describe and verify the new format. ChangesDoctor JSON contract
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RailsCommand
participant Doctor
participant DoctorSchema
participant Stdout
participant Stderr
RailsCommand->>Doctor: run_json_diagnosis
Doctor->>Doctor: build_json_report
Doctor->>DoctorSchema: validate!(report)
DoctorSchema-->>Doctor: validated report
Doctor->>Stdout: print JSON report
Doctor->>Stderr: print incidental output
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)react_on_rails/lib/react_on_rails/doctor.rbast-grep timed out on this file react_on_rails/spec/lib/react_on_rails/doctor_spec.rbast-grep timed out on this file 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 |
|
+ci-status |
CI StatusHead SHA: Only the required gate is active unless hosted CI is requested. |
|
+ci-run-hosted |
github-actions
Bot
added
the
ready-for-hosted-ci
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
Greptile SummaryThis PR adds a stable JSON contract for the React on Rails doctor command. The main changes are:
Confidence Score: 5/5This looks safe to merge after confirming the new schema file is included in packaged releases.
react_on_rails/lib/react_on_rails/doctor.rb Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Doctor FORMAT=json] --> B[Collect check results]
B --> C[Build report]
C --> D[Validate with DoctorSchema]
D --> E[Emit JSON]
E --> F[Exit from schema exit codes]
C --> G[Add severity, docs, remediation, details]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Doctor FORMAT=json] --> B[Collect check results]
B --> C[Build report]
C --> D[Validate with DoctorSchema]
D --> E[Emit JSON]
E --> F[Exit from schema exit codes]
C --> G[Add severity, docs, remediation, details]
Reviews (1): Last reviewed commit: "Allow doctor docs URL before deployment" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.github.com/in/867647?s=60&v=4){kind=small}
![claude[bot]](https://avatars.github.com/in/1236702?s=60&v=4){kind=small}
ReviewOverviewThis PR turns Correctness issue
Code quality
Security
Test coverage
Nice overall structure — the main thing to resolve before merge is reconciling the |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
react_on_rails/spec/lib/react_on_rails/doctor_spec.rb (1)
192-192: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winReplace
Marshal.load(Marshal.dump(...))with the JSON round-trip already used elsewhere in this file.Static analysis flags
Marshal.load/Marshal.dumpas a deserialization risk. Here it's cloning locally-constructed, trusted data (not attacker input), so the risk is minimal, but this file already has a safer idiom in use (e.g. line 168) that avoids the lint noise entirely.♻️ Proposed fix
- mutated_report = Marshal.load(Marshal.dump(report)) + mutated_report = JSON.parse(JSON.generate(report), symbolize_names: true)Apply the same change at line 212 (
mutated_report[:checks].first[:details].firstcase).Also applies to: 212-212
🤖 Prompt for 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. In `@react_on_rails/spec/lib/react_on_rails/doctor_spec.rb` at line 192, Replace the Marshal.load/M棁arshal.dump cloning expressions assigned to mutated_report in both affected cases with the JSON round-trip idiom already used elsewhere in doctor_spec.rb, including the mutated_report[:checks].first[:details].first case. Preserve the existing cloned data and subsequent mutations.Source: Linters/SAST tools
react_on_rails/lib/react_on_rails/doctor_schema.rb (1)
19-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
CHECK_METADATAanddoctor.rb'sCHECK_SECTIONSmust stay in sync manually — no enforcement.
metadata(check_id)usesCHECK_METADATA.fetch(check_id), anddoctor.rb#build_check_entrycalls this beforeDoctorSchema.validate!runs. If a futureCHECK_SECTIONSentry is added indoctor.rbwithout a matchingCHECK_METADATAentry here, doctor will crash with an unhandledKeyErrorinstead of the friendlyArgumentErrorthatvalidate_check!'sassert(CHECK_METADATA.key?(check[:id]), ...)is meant to provide (that assertion never gets a chance to run, sincebuild_check_entryblows up first).Consider adding a load-time or spec-level guard that the two id sets match, e.g. in
doctor.rbafterCHECK_SECTIONS_BY_IDis defined:missing_metadata = CHECK_SECTIONS.map { |section| section[:id] } - DoctorSchema::CHECK_METADATA.keys raise "Missing DoctorSchema::CHECK_METADATA for: #{missing_metadata.join(', ')}" if missing_metadata.any?🤖 Prompt for 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. In `@react_on_rails/lib/react_on_rails/doctor_schema.rb` around lines 19 - 78, Add an explicit consistency guard between DoctorSchema::CHECK_METADATA and doctor.rb’s CHECK_SECTIONS/CHECK_SECTIONS_BY_ID identifiers, preferably immediately after the section index is defined, so missing or extra IDs fail at load or spec time before build_check_entry calls metadata. Keep validate_check! responsible for its existing friendly ArgumentError while ensuring both ID sets remain synchronized.
🤖 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 `@CHANGELOG.md`:
- Around line 87-91: Update the “Agent-legible doctor contract” changelog entry
to append the standard `[PR ...] by [author]` attribution, preserving the
existing issue link and entry content.
---
Nitpick comments:
In `@react_on_rails/lib/react_on_rails/doctor_schema.rb`:
- Around line 19-78: Add an explicit consistency guard between
DoctorSchema::CHECK_METADATA and doctor.rb’s CHECK_SECTIONS/CHECK_SECTIONS_BY_ID
identifiers, preferably immediately after the section index is defined, so
missing or extra IDs fail at load or spec time before build_check_entry calls
metadata. Keep validate_check! responsible for its existing friendly
ArgumentError while ensuring both ID sets remain synchronized.
In `@react_on_rails/spec/lib/react_on_rails/doctor_spec.rb`:
- Line 192: Replace the Marshal.load/M棁arshal.dump cloning expressions assigned
to mutated_report in both affected cases with the JSON round-trip idiom already
used elsewhere in doctor_spec.rb, including the
mutated_report[:checks].first[:details].first case. Preserve the existing cloned
data and subsequent mutations.
🪄 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: CHILL
Plan: Pro
Run ID: 1c4fa399-c49d-43cf-ba30-3f54e4fc5eb2
📒 Files selected for processing (10)
.lychee.tomlCHANGELOG.mddocs/oss/api-reference/doctor.mddocs/sidebars.tsllms-full.txtreact_on_rails/lib/generators/react_on_rails/templates/agent_files/AGENTS.mdreact_on_rails/lib/react_on_rails/doctor.rbreact_on_rails/lib/react_on_rails/doctor_schema.rbreact_on_rails/spec/lib/react_on_rails/doctor_spec.rbreact_on_rails/spec/react_on_rails/generators/install_generator_spec.rb
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/oss/api-reference/doctor.md (1)
68-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReference the correct remediation field path.
filesis documented as nested underremediation, but the workflow refers to it as though it were a top-level check field. This can cause agents to miss the relevant file paths.Suggested wording
-2. Review `message`, `files`, and any `fix_command`. +2. Review `message`, `remediation.files`, and any `fix_command`.🤖 Prompt for 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. In `@docs/oss/api-reference/doctor.md` around lines 68 - 74, Update the broken-configuration workflow to reference file paths through the check’s remediation field, alongside remediation.prompt, while preserving the existing array-order handling and id, severity, message, and fix_command guidance.
🤖 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.
Outside diff comments:
In `@docs/oss/api-reference/doctor.md`:
- Around line 68-74: Update the broken-configuration workflow to reference file
paths through the check’s remediation field, alongside remediation.prompt, while
preserving the existing array-order handling and id, severity, message, and
fix_command guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8f4381e-65ad-4c30-8be7-d30bd8d9efc1
📒 Files selected for processing (4)
CHANGELOG.mddocs/oss/api-reference/doctor.mdllms-full.txtreact_on_rails/lib/react_on_rails/doctor_schema.rb
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- llms-full.txt
- react_on_rails/lib/react_on_rails/doctor_schema.rb
Review:
|
Address-review summaryScan scope: full PR history (no prior address-review summary cutoff). Mattered
Optional
Skipped
Non-cutoff status only. The next review pass must use |
Review:
|
1 participant
Summary
react_on_rails:doctor FORMAT=jsonnow gives agents a stable, validated remediation contract instead of requiring them to parse human-oriented prose. Every check keeps its existing stable ID and deterministic order while adding severity, check-specific documentation, nullable safe commands, and structured remediation context; existing pass/warn/fail exit behavior remains unchanged.The runtime validates the published v1 invariants before emitting JSON, including status/severity consistency, unique IDs, nested detail shapes, exact summary counts, and overall status. Generated agent guidance points consumers at the contract, and the new API page documents fields, exit codes, safe command handling, and scoped runs. The docs URL is narrowly excluded from online link checks only until the new page deploys, matching the repository's existing planned-deployment policy.
Fixes #4602
Validation
llms-full.txt, and online Markdown links: clean.Codex Decision Log
Confidence note: high for the JSON contract and compatibility surface; focused automated and manual coverage passed. Hosted CI remains the final repository-wide confirmation.
Summary by CodeRabbit
react_on_rails:doctorwith machine-readableFORMAT=json, including a versioned schema, stable check IDs, deterministic check ordering, structured remediation guidance, docs links, and CI-friendly exit codes (0 for informational/warnings, 1 for failures).