Skip to content

refactor: simplify artifact scanning and cleanup flow#85

Merged
mors119 merged 1 commit into
FrilLab:mainfrom
mors119:refactor/simplify-flow
Jun 26, 2026
Merged

refactor: simplify artifact scanning and cleanup flow#85
mors119 merged 1 commit into
FrilLab:mainfrom
mors119:refactor/simplify-flow

Conversation

@mors119

@mors119 mors119 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

What

refactor: simplify scanner and analyzer architecture

  • replace artifact type detection with ecosystem-based detectors
  • unify scanner through Detector trait
  • simplify cleanup planning and execution
  • encapsulate analysis logic in Analyzer
  • remove redundant helper modules and legacy code
  • update CLI and tests for new architecture

Closes #

Checklist

Required

  • cargo check passes
  • cargo fmt --check passes
  • cargo clippy --workspace --all-targets -- -D warnings passes
  • cargo test passes

Functional Validation

  • I verified the behavior locally
  • I added or updated tests when necessary

Documentation

  • README or docs were updated if needed
  • New configuration or behavior is documented

Safety

  • Cleanup behavior was reviewed for safety
  • No destructive behavior was introduced without confirmation

Summary by CodeRabbit

  • New Features

    • Added ecosystem filters for scanning and cleanup, letting you target Rust, Node, or Java artifacts.
    • Added a guided cleanup flow with preview, confirmation, and result reporting.
  • Bug Fixes

    • Improved scan and cleanup messages when no artifacts are found.
    • Added clearer invalid-path feedback with the problematic path included.
    • Updated artifact and cleanup output to show ecosystem, path, size, age, and status more clearly.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR replaces Rust-only scanning with ecosystem-filtered Rust, Node, and Java discovery, refactors analysis into an Analyzer type with shared models, and updates cleanup planning, execution, and CLI output to use the new scan and analysis data shapes.

Changes

Ecosystem scan, analysis, and cleanup

Layer / File(s) Summary
Scan pipeline
apps/dustfril-cli/src/cli.rs, apps/dustfril-cli/src/commands/scan.rs, crates/dustfril-core/src/api/scan.rs, crates/dustfril-core/src/fs/*, crates/dustfril-core/src/models/scan.rs, crates/dustfril-core/src/scanner/*, crates/dustfril-core/src/lib.rs, crates/dustfril-core/Cargo.toml
Adds ecosystem flags, scan models, filesystem walking, detector-driven scanning, and updated scan command/API wiring for Rust, Node, and Java artifacts.
Analyzer refactor and shared cleanup models
apps/dustfril-cli/src/commands/analyze.rs, crates/dustfril-core/src/api/analyze.rs, crates/dustfril-core/src/analyzer/*, crates/dustfril-core/src/models/analysis.rs, crates/dustfril-core/src/models/cleanup.rs, crates/dustfril-core/src/models/mod.rs
Moves analysis into Analyzer, computes age and cleanup recommendations in the analyzer, and updates the analyze command to use the new result and recommendation shapes.
Cleanup planning and execution
apps/dustfril-cli/src/commands/clean.rs, crates/dustfril-core/src/api/clean.rs, crates/dustfril-core/src/cleaner/*, crates/dustfril-core/src/error.rs
Validates cleanup paths, builds cleanup plans from safe-to-clean candidates, deletes files or directories by path type, and updates cleanup preview/result output.

Sequence Diagram(s)

sequenceDiagram
  participant CommandsScan as commands/scan.rs
  participant ApiScan as api::scan
  participant ScannerScan as scanner::scan
  participant WalkDirs as walk_dirs
  participant Detectors as DETECTORS
  CommandsScan->>ApiScan: scan(root, ecosystems)
  ApiScan->>ScannerScan: scan(root, ecosystems)
  ScannerScan->>WalkDirs: collect directories under root
  ScannerScan->>Detectors: select detectors for ecosystems
  Detectors-->>ScannerScan: detector list
  ScannerScan-->>ApiScan: ScanResult
  ApiScan-->>CommandsScan: ScanResult
Loading
sequenceDiagram
  participant CommandsClean as commands/clean.rs
  participant ApiClean as api::clean
  participant Analyzer as Analyzer
  participant CreateCleanupPlan as cleaner::create_cleanup_plan
  participant ExecuteCleanup as cleaner::execute_cleanup
  CommandsClean->>ApiClean: build_plan(scan)
  ApiClean->>Analyzer: analyze(scan)
  Analyzer-->>ApiClean: AnalysisResult
  ApiClean->>CreateCleanupPlan: create_cleanup_plan(analysis)
  CreateCleanupPlan-->>ApiClean: CleanupPlan
  ApiClean-->>CommandsClean: CleanupPlan
  CommandsClean->>ExecuteCleanup: execute_cleanup(plan)
  ExecuteCleanup-->>CommandsClean: CleanupResult
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90+ minutes

Possibly related PRs

  • FrilLab/dustfril#34: Updates the scan command and core scan API around detector-based artifact discovery.
  • FrilLab/dustfril#39: Changes the analysis pipeline and the analyzer/recommendation logic.
  • FrilLab/dustfril#45: Implements and adjusts the cleanup flow end to end, including planning and execution.

Poem

🐇 Hop hop, I sniff the paths anew,
Rust, Node, and Java all peek through.
I nibble old bytes and tidy the burrow,
Then dance past cleanup with a cheerful furrow.
Thump! The scan is swift, the carrot stack true.

🚥 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactor: simplifying scanning and cleanup flow.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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: 7

🤖 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 `@apps/dustfril-cli/src/commands/scan.rs`:
- Around line 31-34: The scan command output currently prints only each
artifact’s path, so the new ecosystem information is lost. Update the
user-facing printing in the scan flow in the `scan` command so it includes the
artifact ecosystem alongside the path, using the existing `result.artifacts`
iteration and `artifact` fields. Make the output more readable by formatting
both values together instead of only using `artifact.path`.

In `@crates/dustfril-core/src/analyzer/analyze.rs`:
- Around line 15-24: The artifact ordering in analyze::analyze now comes from
Rayon collection order instead of the previous largest-first behavior. Update
the analyze_artifact pipeline in AnalysisResult construction to sort the
collected artifacts by descending size_bytes before returning, so the CLI output
from analysis_result.artifacts still surfaces the largest cleanup targets first.
- Around line 78-87: The directory traversal in calculate_directory_size and
find_latest_modified is following symlinks via entry.metadata(), which can
recurse into linked directories and count content outside the artifact. Update
the entry handling to use symlink_metadata() in the analyze.rs traversal logic,
detect symlink file types before any recursion, and skip them entirely so only
real files and directories are processed.

In `@crates/dustfril-core/src/analyzer/tests.rs`:
- Around line 47-52: The recommendation assertion in the analyzer test is too
broad and accepts every CleanupRecommendation variant, so it does not verify
behavior. Update the test fixture and/or assertions in analyzer/tests.rs so the
analyzed artifact produced by the relevant test case yields one specific,
deterministic recommendation, and then assert that exact variant using
analysis.artifacts[0].recommendation.

In `@crates/dustfril-core/src/fs/walk.rs`:
- Around line 7-10: The WalkDir iterator in walk_directories is swallowing
traversal errors by converting every Result into an optional entry, which can
hide missing roots or unreadable subtrees. Update the walk logic to propagate
WalkDir failures instead of using a drop-on-error filter, and keep the existing
directory filtering in walk_directories so callers can distinguish an actual
empty scan from an error.

In `@crates/dustfril-core/src/models/cleanup.rs`:
- Around line 49-54: The CleanupCandidate plan is missing the planned filesystem
kind, which causes execute_cleanup to re-detect the live entry instead of
deleting the analyzed artifact. Update CleanupCandidate in cleanup.rs to retain
the expected kind (or equivalent stronger identity data) alongside path,
ecosystem, size_bytes, and age_days, then thread that field through the cleanup
planning and execution flow so execute_cleanup can compare the planned kind
against the current entry and reject mismatches before deleting.
- Around line 29-34: The CleanupRecommendation enum changed the serialized wire
value for the review state, so update its serde behavior to preserve
compatibility with the old "Review" value. In CleanupRecommendation, add a serde
alias or explicit rename handling on the NeedsReview variant so both the new
name and the legacy value are accepted during deserialization while keeping the
intended serialization format stable.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cd10da77-810d-4592-980d-4f531eb9e079

📥 Commits

Reviewing files that changed from the base of the PR and between 717c040 and 2ea8e34.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (54)
  • apps/dustfril-cli/src/cli.rs
  • apps/dustfril-cli/src/commands/analyze.rs
  • apps/dustfril-cli/src/commands/clean.rs
  • apps/dustfril-cli/src/commands/scan.rs
  • crates/dustfril-core/Cargo.toml
  • crates/dustfril-core/src/analyzer/age.rs
  • crates/dustfril-core/src/analyzer/analyze.rs
  • crates/dustfril-core/src/analyzer/metadata.rs
  • crates/dustfril-core/src/analyzer/mod.rs
  • crates/dustfril-core/src/analyzer/recommendation.rs
  • crates/dustfril-core/src/analyzer/size.rs
  • crates/dustfril-core/src/analyzer/tests.rs
  • crates/dustfril-core/src/api/analyze.rs
  • crates/dustfril-core/src/api/clean.rs
  • crates/dustfril-core/src/api/scan.rs
  • crates/dustfril-core/src/cleaner/executor.rs
  • crates/dustfril-core/src/cleaner/plan.rs
  • crates/dustfril-core/src/cleaner/tests.rs
  • crates/dustfril-core/src/detector/java/build.rs
  • crates/dustfril-core/src/detector/java/gradle.rs
  • crates/dustfril-core/src/detector/java/maven.rs
  • crates/dustfril-core/src/detector/java/mod.rs
  • crates/dustfril-core/src/detector/mod.rs
  • crates/dustfril-core/src/detector/node/mod.rs
  • crates/dustfril-core/src/detector/node/node_modules.rs
  • crates/dustfril-core/src/detector/node/package_json.rs
  • crates/dustfril-core/src/detector/project.rs
  • crates/dustfril-core/src/detector/rust/git.rs
  • crates/dustfril-core/src/detector/rust/mod.rs
  • crates/dustfril-core/src/detector/rust/registry.rs
  • crates/dustfril-core/src/detector/rust/target.rs
  • crates/dustfril-core/src/detector/scan.rs
  • crates/dustfril-core/src/detector/tests.rs
  • crates/dustfril-core/src/error.rs
  • crates/dustfril-core/src/fs/mod.rs
  • crates/dustfril-core/src/fs/walk.rs
  • crates/dustfril-core/src/lib.rs
  • crates/dustfril-core/src/models/analysis.rs
  • crates/dustfril-core/src/models/analysis_result.rs
  • crates/dustfril-core/src/models/artifact.rs
  • crates/dustfril-core/src/models/artifact_type.rs
  • crates/dustfril-core/src/models/cleanup.rs
  • crates/dustfril-core/src/models/cleanup_candidate.rs
  • crates/dustfril-core/src/models/cleanup_plan.rs
  • crates/dustfril-core/src/models/cleanup_recommendation.rs
  • crates/dustfril-core/src/models/cleanup_result.rs
  • crates/dustfril-core/src/models/ecosystem.rs
  • crates/dustfril-core/src/models/mod.rs
  • crates/dustfril-core/src/models/scan.rs
  • crates/dustfril-core/src/models/scan_result.rs
  • crates/dustfril-core/src/scanner/detector.rs
  • crates/dustfril-core/src/scanner/mod.rs
  • crates/dustfril-core/src/scanner/scan.rs
  • crates/dustfril-core/src/scanner/tests.rs
💤 Files with no reviewable changes (21)
  • crates/dustfril-core/src/detector/project.rs
  • crates/dustfril-core/src/detector/mod.rs
  • crates/dustfril-core/src/detector/tests.rs
  • crates/dustfril-core/src/detector/rust/git.rs
  • crates/dustfril-core/src/analyzer/recommendation.rs
  • crates/dustfril-core/src/detector/rust/target.rs
  • crates/dustfril-core/src/analyzer/size.rs
  • crates/dustfril-core/src/analyzer/metadata.rs
  • crates/dustfril-core/src/detector/rust/registry.rs
  • crates/dustfril-core/src/models/cleanup_recommendation.rs
  • crates/dustfril-core/src/models/artifact.rs
  • crates/dustfril-core/src/models/artifact_type.rs
  • crates/dustfril-core/src/detector/rust/mod.rs
  • crates/dustfril-core/src/models/ecosystem.rs
  • crates/dustfril-core/src/models/cleanup_candidate.rs
  • crates/dustfril-core/src/models/scan_result.rs
  • crates/dustfril-core/src/models/analysis_result.rs
  • crates/dustfril-core/src/models/cleanup_result.rs
  • crates/dustfril-core/src/models/cleanup_plan.rs
  • crates/dustfril-core/src/detector/scan.rs
  • crates/dustfril-core/src/analyzer/age.rs

Comment on lines 31 to +34
println!("Found {} artifact(s)\n", result.artifacts.len());

for artifact in result.artifacts {
println!("[{}]", artifact.artifact_type);

println!(" {}\n", artifact.path.display());
println!(" {:?}\n", artifact.path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Show the ecosystem in scan output.

Lines 31-34 print only artifact.path via {:?}, which drops the new ecosystem information and makes the user-facing output harder to read.

Suggested fix
-    for artifact in result.artifacts {
-        println!("  {:?}\n", artifact.path);
+    for artifact in result.artifacts {
+        println!("  [{}] {}", artifact.ecosystem, artifact.path.display());
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
println!("Found {} artifact(s)\n", result.artifacts.len());
for artifact in result.artifacts {
println!("[{}]", artifact.artifact_type);
println!(" {}\n", artifact.path.display());
println!(" {:?}\n", artifact.path);
println!("Found {} artifact(s)\n", result.artifacts.len());
for artifact in result.artifacts {
println!(" [{}] {}", artifact.ecosystem, artifact.path.display());
🤖 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 `@apps/dustfril-cli/src/commands/scan.rs` around lines 31 - 34, The scan
command output currently prints only each artifact’s path, so the new ecosystem
information is lost. Update the user-facing printing in the scan flow in the
`scan` command so it includes the artifact ecosystem alongside the path, using
the existing `result.artifacts` iteration and `artifact` fields. Make the output
more readable by formatting both values together instead of only using
`artifact.path`.

Comment on lines +15 to +24
let artifacts: Vec<ArtifactAnalysis> = scan_result
.artifacts
.into_par_iter()
.map(Self::analyze_artifact)
.collect();

let total_size_bytes = artifacts.iter().map(|a| a.size_bytes).sum();

pub fn analyze(scan_result: ScanResult) -> DustResult<AnalysisResult> {
let mut result = AnalysisResult::default();
Ok(AnalysisResult {
artifacts,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the previous largest-first analysis ordering.

The old analyzer sorted artifacts by descending size, but this now returns Rayon collection order; the CLI prints analysis_result.artifacts directly, so large cleanup targets no longer surface first.

Proposed fix
-        let artifacts: Vec<ArtifactAnalysis> = scan_result
+        let mut artifacts: Vec<ArtifactAnalysis> = scan_result
             .artifacts
             .into_par_iter()
             .map(Self::analyze_artifact)
             .collect();
+        artifacts.sort_by_key(|artifact| std::cmp::Reverse(artifact.size_bytes));
 
         let total_size_bytes = artifacts.iter().map(|a| a.size_bytes).sum();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let artifacts: Vec<ArtifactAnalysis> = scan_result
.artifacts
.into_par_iter()
.map(Self::analyze_artifact)
.collect();
let total_size_bytes = artifacts.iter().map(|a| a.size_bytes).sum();
pub fn analyze(scan_result: ScanResult) -> DustResult<AnalysisResult> {
let mut result = AnalysisResult::default();
Ok(AnalysisResult {
artifacts,
let mut artifacts: Vec<ArtifactAnalysis> = scan_result
.artifacts
.into_par_iter()
.map(Self::analyze_artifact)
.collect();
artifacts.sort_by_key(|artifact| std::cmp::Reverse(artifact.size_bytes));
let total_size_bytes = artifacts.iter().map(|a| a.size_bytes).sum();
🤖 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 `@crates/dustfril-core/src/analyzer/analyze.rs` around lines 15 - 24, The
artifact ordering in analyze::analyze now comes from Rayon collection order
instead of the previous largest-first behavior. Update the analyze_artifact
pipeline in AnalysisResult construction to sort the collected artifacts by
descending size_bytes before returning, so the CLI output from
analysis_result.artifacts still surfaces the largest cleanup targets first.

Comment on lines +78 to +87
for entry in entries.flatten() {
let Ok(metadata) = entry.metadata() else {
continue;
};

if metadata.is_file() {
total_size += metadata.len();
} else if metadata.is_dir() {
total_size += calculate_directory_size(&entry.path());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify analyzer recursive traversal currently uses metadata() and not symlink_metadata().
rg -n -C3 'entry\.metadata\(\)|symlink_metadata|follow_links' crates/dustfril-core/src/analyzer crates/dustfril-core/src/fs

Repository: FrilLab/dustfril

Length of output: 1122


🏁 Script executed:

#!/bin/bash
sed -n '1,180p' crates/dustfril-core/src/analyzer/analyze.rs | cat -n

Repository: FrilLab/dustfril

Length of output: 4062


🏁 Script executed:

#!/bin/bash
python3 - <<'PY'
from pathlib import Path
p = Path("crates/dustfril-core/src/analyzer/analyze.rs")
for i, line in enumerate(p.read_text().splitlines(), 1):
    if 70 <= i <= 120:
        print(f"{i:4d}: {line}")
PY

Repository: FrilLab/dustfril

Length of output: 1638


🏁 Script executed:

#!/bin/bash
sed -n '1,170p' crates/dustfril-core/src/analyzer/analyze.rs | nl -ba | sed -n '70,120p'

Repository: FrilLab/dustfril

Length of output: 190


🏁 Script executed:

#!/bin/bash
nl -ba crates/dustfril-core/src/analyzer/analyze.rs | sed -n '1,150p'

Repository: FrilLab/dustfril

Length of output: 190


Skip symlinked directories in traversal

entry.metadata() follows symlinks, so both calculate_directory_size and find_latest_modified can descend into symlinked directories, count targets outside the artifact, and recurse through cycles. Use symlink_metadata() here and skip symlinks before recursing. crates/dustfril-core/src/analyzer/analyze.rs:71-120

🤖 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 `@crates/dustfril-core/src/analyzer/analyze.rs` around lines 78 - 87, The
directory traversal in calculate_directory_size and find_latest_modified is
following symlinks via entry.metadata(), which can recurse into linked
directories and count content outside the artifact. Update the entry handling to
use symlink_metadata() in the analyze.rs traversal logic, detect symlink file
types before any recursion, and skip them entirely so only real files and
directories are processed.

Comment on lines +47 to +52
assert!(matches!(
analysis.artifacts[0].recommendation,
CleanupRecommendation::Keep
| CleanupRecommendation::NeedsReview
| CleanupRecommendation::SafeToClean
));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

This recommendation test is currently vacuous.

Lines 47-52 accept every CleanupRecommendation variant, so the test only proves the field is well-typed. Please make the fixture deterministic enough to assert one specific recommendation; otherwise recommendation regressions will still pass.

🤖 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 `@crates/dustfril-core/src/analyzer/tests.rs` around lines 47 - 52, The
recommendation assertion in the analyzer test is too broad and accepts every
CleanupRecommendation variant, so it does not verify behavior. Update the test
fixture and/or assertions in analyzer/tests.rs so the analyzed artifact produced
by the relevant test case yields one specific, deterministic recommendation, and
then assert that exact variant using analysis.artifacts[0].recommendation.

Comment on lines +7 to +10
WalkDir::new(root)
.into_iter()
.filter_map(|e| e.ok())
.filter(|e| e.file_type().is_dir())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Propagate walk failures instead of dropping them.

Lines 7-10 turn every WalkDir error into “no entry,” so a missing root or unreadable subtree can surface as a successful but incomplete scan. That makes Ok(empty) indistinguishable from “nothing was found.”

🤖 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 `@crates/dustfril-core/src/fs/walk.rs` around lines 7 - 10, The WalkDir
iterator in walk_directories is swallowing traversal errors by converting every
Result into an optional entry, which can hide missing roots or unreadable
subtrees. Update the walk logic to propagate WalkDir failures instead of using a
drop-on-error filter, and keep the existing directory filtering in
walk_directories so callers can distinguish an actual empty scan from an error.

Comment on lines +29 to +34
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum CleanupRecommendation {
Keep,
NeedsReview,
SafeToClean,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n --fixed-strings 'CleanupRecommendation::Review' .
rg -n --fixed-strings '"Review"' . -g '*.rs' -g '*.json' -g '*.snap' -g '*.yaml' -g '*.yml'

Repository: FrilLab/dustfril

Length of output: 154


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the enum and its consumers/serialization context.
rg -n --context 3 'enum CleanupRecommendation|CleanupRecommendation' crates/dustfril-core/src
printf '\n---\n'
sed -n '1,220p' crates/dustfril-core/src/models/cleanup.rs
printf '\n---\n'
rg -n --context 2 'serde|Serialize|Deserialize|CleanupPlan|CleanupResult|CleanupCandidate' crates/dustfril-core/src/models crates/dustfril-core/src/cleaner

Repository: FrilLab/dustfril

Length of output: 18281


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the enum and its consumers/serialization context.
rg -n --context 3 'enum CleanupRecommendation|CleanupRecommendation' crates/dustfril-core/src
printf '\n---\n'
sed -n '1,220p' crates/dustfril-core/src/models/cleanup.rs
printf '\n---\n'
rg -n --context 2 'serde|Serialize|Deserialize|CleanupPlan|CleanupResult|CleanupCandidate' crates/dustfril-core/src/models crates/dustfril-core/src/cleaner

Repository: FrilLab/dustfril

Length of output: 18281


Preserve the old serde value for NeedsReview.

This public enum is serialized with the variant name by default, so renaming Review to NeedsReview changes the wire value and will break any persisted JSON or external client still sending "Review". Add a serde alias or explicit rename policy before shipping.

Compatibility fix
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 pub enum CleanupRecommendation {
     Keep,
+    #[serde(alias = "Review")]
     NeedsReview,
     SafeToClean,
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum CleanupRecommendation {
Keep,
NeedsReview,
SafeToClean,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum CleanupRecommendation {
Keep,
#[serde(alias = "Review")]
NeedsReview,
SafeToClean,
}
🤖 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 `@crates/dustfril-core/src/models/cleanup.rs` around lines 29 - 34, The
CleanupRecommendation enum changed the serialized wire value for the review
state, so update its serde behavior to preserve compatibility with the old
"Review" value. In CleanupRecommendation, add a serde alias or explicit rename
handling on the NeedsReview variant so both the new name and the legacy value
are accepted during deserialization while keeping the intended serialization
format stable.

Comment on lines +49 to +54
pub struct CleanupCandidate {
pub path: PathBuf,
pub ecosystem: Ecosystem,
pub size_bytes: u64,
pub age_days: Option<u64>,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Keep the planned filesystem kind in CleanupCandidate.

Dropping the expected kind from the cleanup plan forces execute_cleanup to re-detect the live entry at delete time. That changes cleanup from “delete the analyzed artifact” to “delete whatever currently exists at this path”, so a concurrent replacement can remove the wrong object. Preserve the planned kind (or stronger identity data) in the plan and reject mismatches during execution.

🤖 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 `@crates/dustfril-core/src/models/cleanup.rs` around lines 49 - 54, The
CleanupCandidate plan is missing the planned filesystem kind, which causes
execute_cleanup to re-detect the live entry instead of deleting the analyzed
artifact. Update CleanupCandidate in cleanup.rs to retain the expected kind (or
equivalent stronger identity data) alongside path, ecosystem, size_bytes, and
age_days, then thread that field through the cleanup planning and execution flow
so execute_cleanup can compare the planned kind against the current entry and
reject mismatches before deleting.

@mors119 mors119 merged commit f7609e6 into FrilLab:main Jun 26, 2026
3 checks passed
@mors119 mors119 deleted the refactor/simplify-flow branch June 26, 2026 03:34
@coderabbitai coderabbitai Bot mentioned this pull request Jun 27, 2026
10 tasks
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