Skip to content

doctor: verify OpenCode asset content drift#20

Open
ivke995 wants to merge 1 commit intomainfrom
feat/doctor-content-hash
Open

doctor: verify OpenCode asset content drift#20
ivke995 wants to merge 1 commit intomainfrom
feat/doctor-content-hash

Conversation

@ivke995
Copy link
Copy Markdown
Contributor

@ivke995 ivke995 commented Apr 3, 2026

Summary by CodeRabbit

  • New Features

    • Embedded assets now include SHA-256 checksums for integrity verification
    • Enhanced health diagnostics to validate embedded asset content during setup
  • Improvements

    • Better detection and reporting of asset content mismatches and read failures
    • Clearer error messages when embedded assets don't match expected values

Co-authored-by: SCE <sce@crocoder.dev>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 3, 2026

📝 Walkthrough

Walkthrough

The PR adds SHA-256 content hashing to embedded assets. The build script computes digests for each asset file and embeds them in generated code. The EmbeddedAsset struct gains a sha256 field. The doctor service switches from presence-only validation to content comparison using these digests.

Changes

Cohort / File(s) Summary
Build-time Hashing Infrastructure
cli/Cargo.toml, cli/build.rs
Added sha2 as a build-time dependency. Build script now computes SHA-256 digests for each embedded asset file and includes them as literal array values in generated EmbeddedAsset entries.
Data Structure Extension
cli/src/services/setup.rs
Extended EmbeddedAsset struct with new sha256: [u8; 32] field to store content digest.
Integration Health Validation
cli/src/services/doctor.rs
Replaced presence-only checks with content-based validation. Added IntegrationContentState enum (Match, Missing, Mismatch, ReadFailed) to replace boolean present field. Doctor now computes on-disk file SHA-256 digests and compares against embedded values. Added new ProblemKind variants for content mismatch and read failures. Updated integration group health logic and report rendering.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • #19: Modifies doctor.rs to implement integration group-based health checks and asset inspection — foundational work for the content comparison approach introduced here.
  • #15: Adds OpenCode plugin registry and asset presence checks to doctor.rs — extended by this PR to include SHA-256 content validation.

Suggested reviewers

  • davidabram
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'doctor: verify OpenCode asset content drift' accurately describes the main change, which is implementing content-based verification of OpenCode embedded assets in the doctor service to detect drift from expected state.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/doctor-content-hash

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cli/build.rs (1)

1-9: 🧹 Nitpick | 🔵 Trivial

Reorder imports: third-party crates should follow standard library.

Per coding guidelines, group imports as: standard library first, then third-party crates, then crate::....

♻️ Suggested reorder
-use sha2::{Digest, Sha256};
 use std::{
     env,
     fmt::Write,
     fs,
     io::{self, Write as IoWrite},
     path::{Path, PathBuf},
     process::Command,
 };
+
+use sha2::{Digest, Sha256};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cli/build.rs` around lines 1 - 9, The import block in this file mixes
third-party and standard-library imports; reorder them so all std::... imports
come first and third-party crate imports (sha2::{Digest, Sha256}) follow,
preserving existing nested items and aliases (e.g., fmt::Write, io::Write as
IoWrite, path::{Path, PathBuf}, process::Command) and keeping the overall import
semantics intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@cli/build.rs`:
- Around line 1-9: The import block in this file mixes third-party and
standard-library imports; reorder them so all std::... imports come first and
third-party crate imports (sha2::{Digest, Sha256}) follow, preserving existing
nested items and aliases (e.g., fmt::Write, io::Write as IoWrite, path::{Path,
PathBuf}, process::Command) and keeping the overall import semantics intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 388fee43-f4cb-4d09-990d-b60695b9ff59

📥 Commits

Reviewing files that changed from the base of the PR and between f0cb443 and 2aa7ea6.

⛔ Files ignored due to path filters (3)
  • context/overview.md is excluded by !context/**/*.md
  • context/plans/doctor-opencode-content-hash.md is excluded by !context/**/*.md
  • context/sce/agent-trace-hook-doctor.md is excluded by !context/**/*.md
📒 Files selected for processing (4)
  • cli/Cargo.toml
  • cli/build.rs
  • cli/src/services/doctor.rs
  • cli/src/services/setup.rs

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.

1 participant