ci: align workflows, fix CI detection, merge dependency updates#12
ci: align workflows, fix CI detection, merge dependency updates#12EffortlessSteven merged 9 commits intomainfrom
Conversation
Bumps the actions group with 6 updates: | Package | From | To | | --- | --- | --- | | [actions/upload-artifact](https://github.com/actions/upload-artifact) | `4` | `7` | | [actions/checkout](https://github.com/actions/checkout) | `4` | `6` | | [codecov/codecov-action](https://github.com/codecov/codecov-action) | `4` | `6` | | [actions/cache](https://github.com/actions/cache) | `4` | `5` | | [actions/download-artifact](https://github.com/actions/download-artifact) | `4` | `8` | | [actions/github-script](https://github.com/actions/github-script) | `7` | `8` | Updates `actions/upload-artifact` from 4 to 7 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v7) Updates `actions/checkout` from 4 to 6 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v6) Updates `codecov/codecov-action` from 4 to 6 - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v4...v6) Updates `actions/cache` from 4 to 5 - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v4...v5) Updates `actions/download-artifact` from 4 to 8 - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v4...v8) Updates `actions/github-script` from 7 to 8 - [Release notes](https://github.com/actions/github-script/releases) - [Commits](actions/github-script@v7...v8) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: codecov/codecov-action dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/cache dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/download-artifact dependency-version: '8' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/github-script dependency-version: '8' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps the dependencies group with 11 updates: | Package | From | To | | --- | --- | --- | | [criterion](https://github.com/criterion-rs/criterion.rs) | `0.5.1` | `0.8.2` | | [thiserror](https://github.com/dtolnay/thiserror) | `1.0.69` | `2.0.18` | | [clap](https://github.com/clap-rs/clap) | `4.5.56` | `4.5.60` | | [toml](https://github.com/toml-rs/toml) | `0.8.23` | `1.1.0+spec-1.1.0` | | [sha2](https://github.com/RustCrypto/hashes) | `0.10.9` | `0.11.0` | | [uselesskey](https://github.com/EffortlessMetrics/uselesskey) | `0.2.0` | `0.5.1` | | [tempfile](https://github.com/Stebalien/tempfile) | `3.26.0` | `3.27.0` | | [proptest](https://github.com/proptest-rs/proptest) | `1.9.0` | `1.11.0` | | [jsonschema](https://github.com/Stranger6667/jsonschema) | `0.28.3` | `0.45.0` | | [tokio](https://github.com/tokio-rs/tokio) | `1.49.0` | `1.50.0` | | [regex](https://github.com/rust-lang/regex) | `1.12.2` | `1.12.3` | Updates `criterion` from 0.5.1 to 0.8.2 - [Release notes](https://github.com/criterion-rs/criterion.rs/releases) - [Changelog](https://github.com/criterion-rs/criterion.rs/blob/master/CHANGELOG.md) - [Commits](criterion-rs/criterion.rs@0.5.1...criterion-v0.8.2) Updates `thiserror` from 1.0.69 to 2.0.18 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@1.0.69...2.0.18) Updates `clap` from 4.5.56 to 4.5.60 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.5.56...clap_complete-v4.5.60) Updates `toml` from 0.8.23 to 1.1.0+spec-1.1.0 - [Commits](toml-rs/toml@toml-v0.8.23...toml-v1.1.0) Updates `sha2` from 0.10.9 to 0.11.0 - [Commits](RustCrypto/hashes@sha2-v0.10.9...sha2-v0.11.0) Updates `uselesskey` from 0.2.0 to 0.5.1 - [Release notes](https://github.com/EffortlessMetrics/uselesskey/releases) - [Changelog](https://github.com/EffortlessMetrics/uselesskey/blob/main/CHANGELOG.md) - [Commits](EffortlessMetrics/uselesskey@v0.2.0...v0.5.1) Updates `tempfile` from 3.26.0 to 3.27.0 - [Changelog](https://github.com/Stebalien/tempfile/blob/master/CHANGELOG.md) - [Commits](Stebalien/tempfile@v3.26.0...v3.27.0) Updates `proptest` from 1.9.0 to 1.11.0 - [Release notes](https://github.com/proptest-rs/proptest/releases) - [Changelog](https://github.com/proptest-rs/proptest/blob/main/CHANGELOG.md) - [Commits](proptest-rs/proptest@v1.9.0...v1.11.0) Updates `jsonschema` from 0.28.3 to 0.45.0 - [Release notes](https://github.com/Stranger6667/jsonschema/releases) - [Changelog](https://github.com/Stranger6667/jsonschema/blob/master/CHANGELOG.md) - [Commits](Stranger6667/jsonschema@rust-v0.28.3...ruby-v0.45.0) Updates `tokio` from 1.49.0 to 1.50.0 - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](tokio-rs/tokio@tokio-1.49.0...tokio-1.50.0) Updates `regex` from 1.12.2 to 1.12.3 - [Release notes](https://github.com/rust-lang/regex/releases) - [Changelog](https://github.com/rust-lang/regex/blob/master/CHANGELOG.md) - [Commits](rust-lang/regex@1.12.2...1.12.3) --- updated-dependencies: - dependency-name: criterion dependency-version: 0.8.2 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: dependencies - dependency-name: thiserror dependency-version: 2.0.18 dependency-type: direct:production update-type: version-update:semver-major dependency-group: dependencies - dependency-name: clap dependency-version: 4.5.60 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: dependencies - dependency-name: toml dependency-version: 1.1.0+spec-1.1.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: dependencies - dependency-name: sha2 dependency-version: 0.11.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: dependencies - dependency-name: uselesskey dependency-version: 0.5.1 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: dependencies - dependency-name: tempfile dependency-version: 3.27.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: dependencies - dependency-name: proptest dependency-version: 1.11.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: dependencies - dependency-name: jsonschema dependency-version: 0.45.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: dependencies - dependency-name: tokio dependency-version: 1.50.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: dependencies - dependency-name: regex dependency-version: 1.12.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: dependencies ... Signed-off-by: dependabot[bot] <support@github.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 17 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughUpdated GitHub Actions to newer major action versions, bumped several workspace dependency versions, applied widespread Rust formatting and minor refactors across many crates, added conditional crate-detection/skip logic in the deprecation workflow, and changed two public enum/serde-related shapes in the explain-builder and report-builder crates. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/lintdiff-code-policy/src/lib.rs (1)
444-457:⚠️ Potential issue | 🟠 MajorNormalize
pathbefore filtering and storing in results.
evaluate_filecurrently uses and stores rawpath, so semantically identical inputs (e.g../src/main.rsvssrc\main.rs) can yield different matching/output. This breaks canonical path and deterministic-output requirements.🔧 Proposed fix
impl PolicyEvaluator { + fn normalize_repo_path(path: &str) -> String { + let mut p = path.replace('\\', "/"); + while let Some(stripped) = p.strip_prefix("./") { + p = stripped.to_string(); + } + p + } + /// Evaluate code against rules for a specific file. #[must_use] pub fn evaluate_file(&self, path: &str, code: &str) -> Vec<PolicyResult> { + let normalized_path = Self::normalize_repo_path(path); let mut results = Vec::new(); for rule in &self.rules { // Check if rule applies to this file - if !rule.applies_to_file(path).unwrap_or(true) { + if !rule.applies_to_file(&normalized_path).unwrap_or(true) { continue; } let matches = self.match_pattern(rule, code); if !matches.is_empty() { results.push( - PolicyResult::new(rule.clone(), matches, rule.policy).with_file_path(path), + PolicyResult::new(rule.clone(), matches, rule.policy) + .with_file_path(normalized_path.clone()), ); } }As per coding guidelines, "Ensure path canonicalization is repo-relative with forward slashes and no leading
./" and "Maintain deterministic outputs: same inputs must produce byte-identical outputs with stable ordering and reproducible truncation".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/lintdiff-code-policy/src/lib.rs` around lines 444 - 457, Normalize the incoming path at the start of evaluate_file and use that normalized value for both filtering and storing in results: compute a repo-relative canonical form with forward slashes and no leading "./" (e.g., normalize_backslashes -> strip_leading_dot_slash -> collapse_redundant_separators) and assign it to a variable (use that variable when calling rule.applies_to_file, match_pattern, and when calling PolicyResult::with_file_path) so identical semantic paths produce identical outputs; update references to evaluate_file, rule.applies_to_file, match_pattern, and PolicyResult::with_file_path to use the normalized path.crates/lintdiff-report-builder/src/lib.rs (1)
593-619:⚠️ Potential issue | 🟠 MajorEnforce canonical repo-relative file paths during validation.
validate()currently rejects only empty paths. It should also reject leading./, absolute paths, and backslashes to keep path handling canonical and cross-platform.💡 Proposed fix
pub enum ReportBuilderError { @@ #[error("File path cannot be empty")] EmptyFilePath, + #[error("Invalid file path: '{0}'")] + InvalidFilePath(String), @@ for path in self.file_results.keys() { if path.is_empty() { return Err(ReportBuilderError::EmptyFilePath); } + if path.starts_with("./") || path.starts_with('/') || path.contains('\\') { + return Err(ReportBuilderError::InvalidFilePath(path.clone())); + } }As per coding guidelines,
Ensure path canonicalization is repo-relative with forward slashes and no leading \./``.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/lintdiff-report-builder/src/lib.rs` around lines 593 - 619, In validate(), extend the file path checks (iterate over self.file_results.keys()) to reject non-canonical repo-relative paths: return an error when a path starts with "./", is absolute (starts with '/' or matches Windows drive-letter like r"^[A-Za-z]:\" or UNC paths starting with "\\"), or contains backslashes '\' (use path.contains('\\')); include the offending path in the error by returning a new or existing variant such as ReportBuilderError::InvalidFilePath(path.clone()) (add that enum variant if missing) so callers can see which path failed; keep the existing EmptyFilePath check and do not attempt to auto-normalize here.crates/lintdiff-render-markdown/src/lib.rs (1)
333-338:⚠️ Potential issue | 🔴 CriticalFix potential UTF-8 boundary panic during message truncation.
Line 337 uses byte-offset slicing (
&escaped[..max_length.saturating_sub(3)]) which can panic if the slice position lands within a multibyte UTF-8 character. This violates the coding guideline requiring "reproducible truncation" and "deterministic outputs." While existing tests don't trigger this (they use ASCII-only messages or keep content below the 120-byte default limit), any message with multibyte characters (e.g., emoji, CJK) at certain positions will cause a runtime panic.Proposed fix
fn truncate_and_escape(s: &str, max_length: usize) -> String { let escaped = s.replace('|', "\\|").replace('\r', "").replace('\n', " "); - if escaped.len() > max_length { - format!("{}...", &escaped[..max_length.saturating_sub(3)]) + if escaped.chars().count() > max_length { + let keep = max_length.saturating_sub(3); + let truncated: String = escaped.chars().take(keep).collect(); + format!("{truncated}...") } else { escaped } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/lintdiff-render-markdown/src/lib.rs` around lines 333 - 338, The current truncate_and_escape function uses a byte-slice (&escaped[..max_length.saturating_sub(3)]) which can cut inside a multibyte UTF-8 char and panic; change it to compute a safe cut index on a char boundary: compute byte_limit = max_length.saturating_sub(3), iterate escaped.char_indices() to find the largest byte index <= byte_limit (e.g., use char_indices().take_while(|(i,_)| *i < byte_limit).last() to derive the cut position), then slice escaped up to that cut and append "..." so slicing is always on valid UTF-8 boundaries and avoids panics in truncate_and_escape.crates/lintdiff-explain-summary/src/lib.rs (1)
720-748:⚠️ Potential issue | 🟠 Major
filter_by_dispositionreturns inconsistent severity totals.
result.total_findingsis filtered by disposition, butresult.by_severityis copied from the unfiltered summary, and each filtered file summary also copies full per-file severity counts. This yields internally inconsistent summaries and incorrect reporting for filtered views.Proposed direction
pub fn filter_by_disposition(summary: &ExplainSummary, disposition: &str) -> ExplainSummary { @@ - // Copy severity counts proportionally - result.by_severity.clone_from(&summary.by_severity); + // Cannot derive exact filtered severities from current FileSummary schema. + // Keep empty (or introduce per-finding/per-disposition severity tracking upstream). + result.by_severity.clear();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/lintdiff-explain-summary/src/lib.rs` around lines 720 - 748, filter_by_disposition is producing inconsistent severity totals because result.total_findings and per-file finding_count are filtered by disposition while result.by_severity and each new_file_summary.by_severity are copied from the unfiltered summary/by_file; fix by recalculating severity counts only for findings that match the given disposition: in filter_by_disposition iterate the original summary.by_file findings (or a helper that can inspect individual findings) to compute per-file by_severity and aggregate into result.by_severity and result.total_findings (instead of cloning summary.by_severity), update new_file_summary.by_severity using these filtered counts, and ensure disposition_count calls reflect the same filtered data so result, result.by_severity, result.by_disposition, and result.files_affected remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deprecation.yml:
- Around line 21-35: Centralize the deprecated crate inventory by extracting the
repeated crate names (lintdiff-domain, lintdiff-core, lintdiff-ingest) into a
single shared list variable (e.g., DEPRECATED_CRATES or deprecated_crates_list)
and replace each duplicated block that checks directory existence and appends to
deprecated_crates with a loop that iterates over that single list; update the
build/check/warning steps to source or reference that shared variable so the
existence checks and echo messages are performed from one place rather than
duplicating the same if [ -d "${{ github.workspace }}/crates/<name>" ] ...
deprecated_crates+=("<name>") logic in multiple locations.
In @.github/workflows/mutation.yml:
- Around line 55-63: Replace mutable major-version tags with immutable full
commit SHAs for all GitHub Action uses to prevent unexpected workflow changes:
update uses: actions/checkout@v6, uses: dtolnay/rust-toolchain@stable, uses:
actions/cache@v5 and the other action uses referenced in this file to their
corresponding full-length commit SHAs (e.g., uses:
actions/checkout@<full-commit-sha> and add an inline comment with the
human-readable tag like # v6.1.0) so each action reference is pinned immutably
while preserving the original tag in a comment for clarity.
---
Outside diff comments:
In `@crates/lintdiff-code-policy/src/lib.rs`:
- Around line 444-457: Normalize the incoming path at the start of evaluate_file
and use that normalized value for both filtering and storing in results: compute
a repo-relative canonical form with forward slashes and no leading "./" (e.g.,
normalize_backslashes -> strip_leading_dot_slash ->
collapse_redundant_separators) and assign it to a variable (use that variable
when calling rule.applies_to_file, match_pattern, and when calling
PolicyResult::with_file_path) so identical semantic paths produce identical
outputs; update references to evaluate_file, rule.applies_to_file,
match_pattern, and PolicyResult::with_file_path to use the normalized path.
In `@crates/lintdiff-explain-summary/src/lib.rs`:
- Around line 720-748: filter_by_disposition is producing inconsistent severity
totals because result.total_findings and per-file finding_count are filtered by
disposition while result.by_severity and each new_file_summary.by_severity are
copied from the unfiltered summary/by_file; fix by recalculating severity counts
only for findings that match the given disposition: in filter_by_disposition
iterate the original summary.by_file findings (or a helper that can inspect
individual findings) to compute per-file by_severity and aggregate into
result.by_severity and result.total_findings (instead of cloning
summary.by_severity), update new_file_summary.by_severity using these filtered
counts, and ensure disposition_count calls reflect the same filtered data so
result, result.by_severity, result.by_disposition, and result.files_affected
remain consistent.
In `@crates/lintdiff-render-markdown/src/lib.rs`:
- Around line 333-338: The current truncate_and_escape function uses a
byte-slice (&escaped[..max_length.saturating_sub(3)]) which can cut inside a
multibyte UTF-8 char and panic; change it to compute a safe cut index on a char
boundary: compute byte_limit = max_length.saturating_sub(3), iterate
escaped.char_indices() to find the largest byte index <= byte_limit (e.g., use
char_indices().take_while(|(i,_)| *i < byte_limit).last() to derive the cut
position), then slice escaped up to that cut and append "..." so slicing is
always on valid UTF-8 boundaries and avoids panics in truncate_and_escape.
In `@crates/lintdiff-report-builder/src/lib.rs`:
- Around line 593-619: In validate(), extend the file path checks (iterate over
self.file_results.keys()) to reject non-canonical repo-relative paths: return an
error when a path starts with "./", is absolute (starts with '/' or matches
Windows drive-letter like r"^[A-Za-z]:\" or UNC paths starting with "\\"), or
contains backslashes '\' (use path.contains('\\')); include the offending path
in the error by returning a new or existing variant such as
ReportBuilderError::InvalidFilePath(path.clone()) (add that enum variant if
missing) so callers can see which path failed; keep the existing EmptyFilePath
check and do not attempt to auto-normalize here.
🪄 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: 119667d6-1592-416d-a1de-80ea9f168ab7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (101)
.github/workflows/bench.yml.github/workflows/ci.yml.github/workflows/coverage.yml.github/workflows/deny.yml.github/workflows/deprecation.yml.github/workflows/fuzz.yml.github/workflows/mutation.yml.github/workflows/release.yml.github/workflows/semver.ymlCargo.tomlaction.ymlcrates/lintdiff-annotation-format/src/lib.rscrates/lintdiff-annotation-format/tests/annotation_format_tests.rscrates/lintdiff-app/src/lib.rscrates/lintdiff-bdd-harness/src/lib.rscrates/lintdiff-budget/tests/budget_tests.rscrates/lintdiff-ci-env/src/lib.rscrates/lintdiff-ci-env/tests/ci_env_tests.rscrates/lintdiff-cli/tests/bdd.rscrates/lintdiff-code-norm/src/lib.rscrates/lintdiff-code-policy/src/lib.rscrates/lintdiff-code-policy/tests/code_policy_tests.rscrates/lintdiff-code-url/src/lib.rscrates/lintdiff-code-url/tests/code_url_tests.rscrates/lintdiff-config-types/tests/config_types_tests.rscrates/lintdiff-config/tests/config_tests.rscrates/lintdiff-counts/src/lib.rscrates/lintdiff-counts/tests/counts_tests.rscrates/lintdiff-diagnostic-level/src/lib.rscrates/lintdiff-diagnostic-level/tests/diagnostic_level_tests.rscrates/lintdiff-diff-paths/src/lib.rscrates/lintdiff-diff-paths/tests/diff_paths_tests.rscrates/lintdiff-diff-stats/src/lib.rscrates/lintdiff-diff-stats/tests/diff_stats_tests.rscrates/lintdiff-disposition/tests/disposition_tests.rscrates/lintdiff-escape/tests/escape_tests.rscrates/lintdiff-exit/tests/exit_tests.rscrates/lintdiff-explain-builder/src/lib.rscrates/lintdiff-explain-builder/tests/explain_builder_tests.rscrates/lintdiff-explain-summary/src/lib.rscrates/lintdiff-explain/src/lib.rscrates/lintdiff-explain/tests/explain_tests.rscrates/lintdiff-finding-builder/src/lib.rscrates/lintdiff-finding-builder/tests/finding_builder_tests.rscrates/lintdiff-finding/src/lib.rscrates/lintdiff-finding/tests/finding_tests.rscrates/lintdiff-git-info/src/lib.rscrates/lintdiff-git-info/tests/git_info_tests.rscrates/lintdiff-glob/src/lib.rscrates/lintdiff-glob/tests/glob_tests.rscrates/lintdiff-host-info/tests/host_info_tests.rscrates/lintdiff-hunk-header/src/lib.rscrates/lintdiff-hunk-header/tests/hunk_header_tests.rscrates/lintdiff-line-merge/tests/line_merge_tests.rscrates/lintdiff-locale-detect/src/lib.rscrates/lintdiff-locale-detect/tests/locale_tests.rscrates/lintdiff-location/src/lib.rscrates/lintdiff-location/tests/location_tests.rscrates/lintdiff-message-norm/src/lib.rscrates/lintdiff-message-norm/tests/message_norm_tests.rscrates/lintdiff-message-truncate/src/lib.rscrates/lintdiff-message-truncate/tests/message_truncate_tests.rscrates/lintdiff-path-norm/src/lib.rscrates/lintdiff-path-norm/tests/path_norm_tests.rscrates/lintdiff-range-merge/src/lib.rscrates/lintdiff-range-merge/tests/range_merge_tests.rscrates/lintdiff-render-annotations/src/lib.rscrates/lintdiff-render-annotations/tests/annotations_tests.rscrates/lintdiff-render-markdown/src/lib.rscrates/lintdiff-render-markdown/tests/markdown_tests.rscrates/lintdiff-render-utils/src/lib.rscrates/lintdiff-render-utils/tests/render_utils_tests.rscrates/lintdiff-report-builder/src/lib.rscrates/lintdiff-report-builder/tests/report_builder_tests.rscrates/lintdiff-report-schema/src/lib.rscrates/lintdiff-report-schema/tests/report_schema_tests.rscrates/lintdiff-run-info/src/lib.rscrates/lintdiff-run-info/tests/run_info_tests.rscrates/lintdiff-schema-diff/src/lib.rscrates/lintdiff-schema-diff/tests/schema_diff_tests.rscrates/lintdiff-severity-map/src/lib.rscrates/lintdiff-severity-map/tests/severity_map_tests.rscrates/lintdiff-severity/src/lib.rscrates/lintdiff-severity/tests/severity_tests.rscrates/lintdiff-slugify/src/lib.rscrates/lintdiff-slugify/tests/slugify_tests.rscrates/lintdiff-sort/src/lib.rscrates/lintdiff-sort/tests/sort_tests.rscrates/lintdiff-span-intersect/src/lib.rscrates/lintdiff-span-intersect/tests/span_intersect_tests.rscrates/lintdiff-span/src/lib.rscrates/lintdiff-stats/src/lib.rscrates/lintdiff-stats/tests/stats_tests.rscrates/lintdiff-timestamp/src/lib.rscrates/lintdiff-timestamp/tests/timestamp_tests.rscrates/lintdiff-truncate/src/lib.rscrates/lintdiff-types/Cargo.tomlcrates/lintdiff-verdict-reason/src/lib.rscrates/lintdiff-verdict-reason/tests/verdict_reason_tests.rscrates/lintdiff-verdict/src/lib.rscrates/lintdiff-verdict/tests/verdict_tests.rs
| deprecated_crates=() | ||
| if [ -d "${{ github.workspace }}/crates/lintdiff-domain" ]; then | ||
| deprecated_crates+=("lintdiff-domain") | ||
| else | ||
| echo "✅ Skipping lintdiff-domain (already removed)" | ||
| fi | ||
| if [ -d "${{ github.workspace }}/crates/lintdiff-core" ]; then | ||
| deprecated_crates+=("lintdiff-core") | ||
| else | ||
| echo "✅ Skipping lintdiff-core (already removed)" | ||
| fi | ||
| if [ -d "${{ github.workspace }}/crates/lintdiff-ingest" ]; then | ||
| deprecated_crates+=("lintdiff-ingest") | ||
| else | ||
| echo "✅ Skipping lintdiff-ingest (already removed)" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Centralize the deprecated crate inventory.
The same crate names and existence checks are now duplicated across build, check, and warning steps. The next rename or removal will require touching several places again, so this workflow is still prone to drift. Consider defining the manifest-path list once and iterating over it.
Also applies to: 48-62, 82-85, 117-120, 151-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deprecation.yml around lines 21 - 35, Centralize the
deprecated crate inventory by extracting the repeated crate names
(lintdiff-domain, lintdiff-core, lintdiff-ingest) into a single shared list
variable (e.g., DEPRECATED_CRATES or deprecated_crates_list) and replace each
duplicated block that checks directory existence and appends to
deprecated_crates with a loop that iterates over that single list; update the
build/check/warning steps to source or reference that shared variable so the
existence checks and echo messages are performed from one place rather than
duplicating the same if [ -d "${{ github.workspace }}/crates/<name>" ] ...
deprecated_crates+=("<name>") logic in multiple locations.
| uses: actions/checkout@v6 | ||
|
|
||
| - name: Install Rust stable | ||
| uses: dtolnay/rust-toolchain@stable | ||
|
|
||
| # Cache cargo registry and build artifacts for faster runs | ||
| - name: Cache cargo registry | ||
| uses: actions/cache@v4 | ||
| uses: actions/cache@v5 | ||
| with: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the workflow file exists and get its size
wc -l .github/workflows/mutation.yml 2>/dev/null || echo "File not found"Repository: EffortlessMetrics/lintdiff
Length of output: 104
🏁 Script executed:
# Read the specific lines mentioned in the review
echo "=== Lines 55-63 ==="
sed -n '55,63p' .github/workflows/mutation.yml 2>/dev/null || echo "File not found"
echo -e "\n=== Lines 263-272 ==="
sed -n '263,272p' .github/workflows/mutation.yml 2>/dev/null || echo "File not found"Repository: EffortlessMetrics/lintdiff
Length of output: 803
🏁 Script executed:
# Search for all 'uses:' entries in the workflow to see the pattern
echo "=== All action references in mutation.yml ==="
grep -n "uses:" .github/workflows/mutation.yml 2>/dev/null || echo "File not found"Repository: EffortlessMetrics/lintdiff
Length of output: 321
🌐 Web query:
GitHub Actions security best practices pin commit SHA immutable
💡 Result:
GitHub Actions security best practices strongly recommend pinning actions to a full-length commit SHA for immutability and protection against supply chain attacks. This prevents malicious updates to tags or branches. Key practices: - Pin to full SHA: uses: actions/checkout@692973e (Verify SHA from official repo, not forks). - Add comment for readability and Dependabot: # v4.1.0 - Use for third-party actions, especially high-risk ones handling secrets. - Avoid @main, @latest, or unpinned tags; tags can be moved. - Enforce via org/repo policies (new feature): require SHA pinning, block risky actions with !prefix. - Automate updates with Dependabot version updates or tools like pin-github-action. - Audit action source code; prefer first-party (actions/*). - Combine with: least-privilege GITHUB_TOKEN, OIDC for cloud auth, secret masking, runtime protection (e.g., Harden-Runner). Example workflow step: - uses: actions/checkout@34e1148 # v4.3.1 Benefits: Mitigates backdoors (SHA collision hard), reproducibility, counters dependency confusion.
Citations:
- 1: https://www.codewrecks.com/post/github/github-sha-pinning/
- 2: https://www.stepsecurity.io/blog/pinning-github-actions-for-enhanced-security-a-complete-guide
- 3: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 4: https://www.nearform.com/insights/why-you-should-pin-your-github-actions-by-commit-hash
- 5: https://cloudnativeengineer.substack.com/p/github-actions-reproducibility-security
- 6: fix: pin GitHub Actions to immutable SHA hashes to prevent supply chain attacks github/awesome-copilot#1088
- 7: https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-githubs-security-features-to-secure-your-use-of-github-actions
- 8: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions
- 9: https://docs.github.com/en/actions/security-for-github-actions
- 10: https://docs.github.com/en/actions/concepts/security/compromised-runners
- 11: https://dev.to/ciberconscientes/github-actions-why-you-should-pin-your-actions-to-a-specific-version-1513
- 12: https://stackoverflow.com/questions/78903499/how-do-i-pin-an-action-to-a-specific-sha
Pin these action upgrades to immutable commit SHAs.
These uses: entries use mutable major version tags (@v6, @v5, @v7, @v8), allowing tag reassignment outside this repository to change workflow behavior unexpectedly. GitHub's security guidance recommends pinning to full-length commit SHAs, which are immutable and prevent supply chain attacks. Use the format: uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v6.1.0
Applies to lines 55, 62, 263, and 272.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/mutation.yml around lines 55 - 63, Replace mutable
major-version tags with immutable full commit SHAs for all GitHub Action uses to
prevent unexpected workflow changes: update uses: actions/checkout@v6, uses:
dtolnay/rust-toolchain@stable, uses: actions/cache@v5 and the other action uses
referenced in this file to their corresponding full-length commit SHAs (e.g.,
uses: actions/checkout@<full-commit-sha> and add an inline comment with the
human-readable tag like # v6.1.0) so each action reference is pinned immutably
while preserving the original tag in a comment for clarity.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/lintdiff-explain-builder/tests/explain_builder_tests.rs (1)
1340-1387: 🧹 Nitpick | 🔵 TrivialUse structural JSON assertions instead of
containschecks in serde tests.Line 1340–Line 1387 uses substring checks that can pass even when payload shape regresses. Parse to
serde_json::Value(or deserialize back toExplainSection) and assert exact keys/shape for stronger regression protection.Example hardening for one test
- let json = serde_json::to_string(§ion).unwrap(); - assert!(json.contains("\"type\":\"text\"")); - assert!(json.contains("Hello")); + let json = serde_json::to_string(§ion).unwrap(); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(v["type"], "text"); + assert_eq!(v["content"], "Hello");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/lintdiff-explain-builder/tests/explain_builder_tests.rs` around lines 1340 - 1387, The tests (e.g., given_explain_section_text_when_serialized_then_valid_json, given_explain_section_bullets_when_serialized_then_valid_json, given_explain_section_code_when_serialized_then_valid_json, given_explain_section_table_when_serialized_then_valid_json) use fragile substring assertions on serde_json::to_string output; instead parse the JSON string into serde_json::Value or deserialize back into ExplainSection and assert the exact structure and keys (e.g., assert value["type"] == "text"/"bullets"/"code"/"table", check presence and types of fields like "content", "items", "language", "headers", "rows") to make the tests robust against payload shape regressions—update each test to parse the JSON and perform structural assertions on the Value or deserialized ExplainSection rather than using contains().crates/lintdiff-report-builder/src/lib.rs (1)
152-164:⚠️ Potential issue | 🟠 MajorSeverity enum serialization values conflict with v1 report schema.
The
serde(rename_all = "lowercase")at line 152 produceshint|note|warning|error|fatal, butschemas/lintdiff.report.v1.jsonallows onlyinfo|warn|error. Reports withhint,note,fatal, orwarningvalues will fail schema validation. Additionally,warningshould serialize aswarnto match the schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/lintdiff-report-builder/src/lib.rs` around lines 152 - 164, The Severity enum's serde renaming must be changed so its serialized strings match the v1 schema (info|warn|error): replace the global #[cfg_attr(..., serde(rename_all = "lowercase"))] with per-variant serde renames on the Severity enum so that Hint and Note serialize as "info", Warning serializes as "warn", and Error and Fatal serialize as "error"; keep the #[default] on Warning if desired and ensure the serde attributes are applied to the enum variants (e.g., #[serde(rename = "info")] on Hint/Note, #[serde(rename = "warn")] on Warning, #[serde(rename = "error")] on Error/Fatal) so schema validation will pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/lintdiff-explain-builder/src/lib.rs`:
- Around line 50-60: You changed the enum ExplainSection (variants Text and
Bullets) from tuple-like to struct-like which breaks downstream pattern matches
and existing serde wire formats; implement backward-compatible deserialization
for ExplainSection so both legacy tuple-like JSON (e.g. {"Text":"..."} and
{"Bullets":["a","b"]}) and the new struct-like JSON (e.g.
{"Text":{"content":"..."}} and {"Bullets":{"items":[...]}}) deserialize
correctly. Add a custom impl of serde::Deserialize for ExplainSection (or a
helper Visitor) that detects when a variant payload is a primitive/string/seq
and maps it to the new struct fields, otherwise parses the new struct shape;
reference the ExplainSection enum and its Text and Bullets variants in the
implementation so existing consumers continue to work without migration.
---
Outside diff comments:
In `@crates/lintdiff-explain-builder/tests/explain_builder_tests.rs`:
- Around line 1340-1387: The tests (e.g.,
given_explain_section_text_when_serialized_then_valid_json,
given_explain_section_bullets_when_serialized_then_valid_json,
given_explain_section_code_when_serialized_then_valid_json,
given_explain_section_table_when_serialized_then_valid_json) use fragile
substring assertions on serde_json::to_string output; instead parse the JSON
string into serde_json::Value or deserialize back into ExplainSection and assert
the exact structure and keys (e.g., assert value["type"] ==
"text"/"bullets"/"code"/"table", check presence and types of fields like
"content", "items", "language", "headers", "rows") to make the tests robust
against payload shape regressions—update each test to parse the JSON and perform
structural assertions on the Value or deserialized ExplainSection rather than
using contains().
In `@crates/lintdiff-report-builder/src/lib.rs`:
- Around line 152-164: The Severity enum's serde renaming must be changed so its
serialized strings match the v1 schema (info|warn|error): replace the global
#[cfg_attr(..., serde(rename_all = "lowercase"))] with per-variant serde renames
on the Severity enum so that Hint and Note serialize as "info", Warning
serializes as "warn", and Error and Fatal serialize as "error"; keep the
#[default] on Warning if desired and ensure the serde attributes are applied to
the enum variants (e.g., #[serde(rename = "info")] on Hint/Note, #[serde(rename
= "warn")] on Warning, #[serde(rename = "error")] on Error/Fatal) so schema
validation will pass.
🪄 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: f179b8f9-bb07-45a8-a763-f37d317f444b
📒 Files selected for processing (4)
crates/lintdiff-explain-builder/src/lib.rscrates/lintdiff-explain-builder/tests/explain_builder_tests.rscrates/lintdiff-report-builder/src/lib.rscrates/lintdiff-severity-map/tests/severity_map_tests.rs
| pub enum ExplainSection { | ||
| /// Plain text content. | ||
| Text(String), | ||
| Text { | ||
| /// The plain text content. | ||
| content: String, | ||
| }, | ||
| /// Bullet list items. | ||
| Bullets(Vec<String>), | ||
| Bullets { | ||
| /// Items in the bullet list. | ||
| items: Vec<String>, | ||
| }, |
There was a problem hiding this comment.
Public API + serde wire-shape break needs compatibility handling or explicit migration note.
Line 50–Line 60 changes ExplainSection::Text/Bullets from tuple-like to struct-like variants. This is a breaking change for downstream Rust pattern matches and potentially for persisted/interop JSON payloads. Please either add backward-compatible deserialization (legacy alias handling) or explicitly gate this with a versioned migration.
Suggested compatibility patch (serde legacy-key alias)
pub enum ExplainSection {
/// Plain text content.
Text {
/// The plain text content.
+ #[cfg_attr(feature = "serde", serde(alias = "0"))]
content: String,
},
/// Bullet list items.
Bullets {
/// Items in the bullet list.
+ #[cfg_attr(feature = "serde", serde(alias = "0"))]
items: Vec<String>,
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/lintdiff-explain-builder/src/lib.rs` around lines 50 - 60, You changed
the enum ExplainSection (variants Text and Bullets) from tuple-like to
struct-like which breaks downstream pattern matches and existing serde wire
formats; implement backward-compatible deserialization for ExplainSection so
both legacy tuple-like JSON (e.g. {"Text":"..."} and {"Bullets":["a","b"]}) and
the new struct-like JSON (e.g. {"Text":{"content":"..."}} and
{"Bullets":{"items":[...]}}) deserialize correctly. Add a custom impl of
serde::Deserialize for ExplainSection (or a helper Visitor) that detects when a
variant payload is a primitive/string/seq and maps it to the new struct fields,
otherwise parses the new struct shape; reference the ExplainSection enum and its
Text and Bullets variants in the implementation so existing consumers continue
to work without migration.
This PR consolidates CI stability fixes and absorbs open Dependabot updates so open PRs can be retired: