feat: revalidate developer artifact cleanup - #171
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthrough개발 아티팩트에 bounded metadata manifest, fingerprint, 파일시스템 객체 식별자 검증을 추가했습니다. 정리 명령과 UI는 검증된 항목만 처리합니다. macOS 클라우드 열거, iCloud 스냅샷 크기, reclaim active-use 증거에도 제한을 추가했습니다. Changes개발 아티팩트 정리
macOS 클라우드 디렉터리 probe
iCloud 스냅샷 소스 크기 제한
Reclaim active-use 증거
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CleanupUI
participant cleanDevArtifacts
participant clean_dev_artifacts_inner
participant artifact_manifest
participant trash_delete_if_identity
participant FileSystem
CleanupUI->>cleanDevArtifacts: 선택된 DevArtifact 전달
cleanDevArtifacts->>clean_dev_artifacts_inner: Tauri 명령 호출
clean_dev_artifacts_inner->>artifact_manifest: 현재 아티팩트 재스캔
artifact_manifest->>FileSystem: bounded metadata 조회
FileSystem-->>artifact_manifest: 통계와 fingerprint 반환
artifact_manifest-->>clean_dev_artifacts_inner: 현재 manifest 반환
clean_dev_artifacts_inner->>trash_delete_if_identity: expected object ID 전달
trash_delete_if_identity->>FileSystem: staging 이동과 identity 재검증
FileSystem-->>trash_delete_if_identity: 휴지통 이동 결과
trash_delete_if_identity-->>CleanupUI: 정리 결과 반환
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src-tauri/src/dev_artifacts.rs`:
- Around line 91-125: The fingerprint currently identifies paths only by
metadata, allowing replaced filesystem objects to pass validation and be deleted
later via a fresh path lookup. In src-tauri/src/dev_artifacts.rs#L91-L125,
update the manifest records produced by the directory scan to include a
non-reusable filesystem object identifier for each relevant entry. In
src-tauri/src/commands.rs#L149-L174, replace post-validation request.path
resolution with platform-specific operations that verify the recorded object
identifier and move that same object atomically, eliminating the race between
verification and trashing.
🪄 Autofix
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 Plus
Run ID: 12011899-a781-4cdb-8a58-e1739bbc4361
📒 Files selected for processing (32)
src-tauri/src/bin/disksage-reclaim-plan.rssrc-tauri/src/commands.rssrc-tauri/src/dataset_metadata.rssrc-tauri/src/dev_artifacts.rssrc-tauri/src/dupes.rssrc-tauri/src/inventory.rssrc-tauri/src/lib.rssrc-tauri/src/llm/backend.rssrc-tauri/src/llm/engine.rssrc-tauri/src/llm/installed_model.rssrc-tauri/src/llm/installed_model_io_tests.rssrc-tauri/src/llm/mod.rssrc-tauri/src/llm/model.rssrc-tauri/src/llm/model_concurrency_tests.rssrc-tauri/src/llm/parse.rssrc-tauri/src/llm/prompt.rssrc-tauri/src/llm/verdict.rssrc-tauri/src/ontology.rssrc-tauri/src/organize.rssrc-tauri/src/reasoning.rssrc-tauri/src/rules.rssrc-tauri/src/safety.rssrc-tauri/src/scanner.rssrc-tauri/src/settings.rssrc-tauri/src/userrules.rssrc-tauri/src/web/mod.rssrc-tauri/tests/cloud_transfer_coverage_contract.rssrc-tauri/tests/cloud_transfer_tenant_authority_gate.rssrc-tauri/tests/icloud_local_eviction_batch_documentation_test.rssrc-tauri/tests/package_metadata_contract.rssrc/lib/Cleanup.sveltesrc/lib/api.ts
8177527 to
943bf63
Compare
|
Addressed in |
|
Follow-up hardening in |
|
Follow-up on the Windows build failure: |
|
Follow-up safety hardening in
No cloud write, source eviction, cache purge, or worktree removal was performed. |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src-tauri/src/icloud_sync_health.rs (1)
901-916: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
client.db-wal의 제한 초과 경로도 테스트하세요.현재 테스트는
client.db만 1바이트 초과하는 경우를 검증합니다.client.db가 제한 이하이고client.db-wal만MAX_SNAPSHOT_SOURCE_BYTES + 1인 경우에도icloud-sync-health-snapshot-source-too-large를 반환하는지 확인하세요.🤖 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 `@src-tauri/src/icloud_sync_health.rs` around lines 901 - 916, Extend oversized_cloud_docs_database_fails_closed_before_snapshot_copy to also create a within-limit client.db and an oversized client.db-wal, then verify clone_client_database_snapshot returns the existing "icloud-sync-health-snapshot-source-too-large" error.src-tauri/src/commands.rs (1)
150-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
flat_map을map으로 단순화하십시오.각 분기가 정확히 한 개 원소를 가진
Vec을 반환합니다.map을 사용하면 중간Vec할당이 사라지고 의도가 명확해집니다.♻️ 제안 리팩터링
- .flat_map(|request| { + .map(|request| { let matches = current.iter().find(|candidate| {if matches.is_none() { - return vec![CleanResult { + return CleanResult { path: request.path.clone(), ok: false, error: "개발 아티팩트가 변경되었거나 메타데이터 스캔이 불완전합니다. 정리 전에 다시 스캔하세요".into(), - }]; + }; }- Ok(()) => vec![CleanResult { + Ok(()) => CleanResult { path: request.path.clone(), ok: true, error: String::new(), - }], - Err(error) => vec![CleanResult { + }, + Err(error) => CleanResult { path: request.path.clone(), ok: false, error: error.to_string(), - }], + },🤖 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 `@src-tauri/src/commands.rs` around lines 150 - 195, Update the iterator in the request-processing flow to use map instead of flat_map, since each branch currently returns exactly one CleanResult wrapped in a Vec. Return a single CleanResult directly from the matches and trash_delete_if_identity branches, preserving their existing success and error behavior, then collect the mapped results.
🤖 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 `@src-tauri/src/cloud.rs`:
- Around line 375-387: Restrict the macOS branch in directory_access_issue to
all(not(coverage), target_os = "macos") so it only calls run_bounded_find when
that helper is compiled; let coverage builds use the existing read_dir fallback.
In `@src-tauri/src/commands.rs`:
- Around line 149-167: Update the artifact matching predicate in the requests
iteration to compare age_days monotonically: replace the candidate.age_days
equality check with candidate.age_days >= request.age_days, while preserving all
other matching conditions.
In `@src-tauri/src/icloud_sync_health.rs`:
- Around line 413-425: Update the snapshot copy routine invoked by the
surrounding health-check flow, including clone_snapshot_file, to enforce
MAX_SNAPSHOT_SOURCE_BYTES during copying rather than only in the
before_db/before_wal checks. Stop immediately when the source exceeds the limit,
clean up any partial destination files, and return the existing
icloud-sync-health-snapshot-source-too-large error without allowing oversized
data to remain in the temporary directory.
In `@src-tauri/src/safety.rs`:
- Around line 365-374: Update restore_staged_if_source_absent so every failed
restoration is surfaced with the staged path: handle both the source-reappeared
case and std::fs::rename failure, and include staged in the returned error or
journal entry. Preserve cleanup only after a successful rename, and do not
silently discard restoration errors.
- Around line 439-446: Preserve the OS-trash undo contract in the safety.rs flow
around trash::delete by keeping staging_dir available for restoration, or
restore the object to its original path before trashing it; document the chosen
tradeoff in the nearby comment and do not treat necessary staging cleanup as a
successful undo-preserving operation. After the behavior is finalized, update
README.md line 30 so “journaled and undoable” accurately describes the supported
reversal scope.
- Around line 134-140: Update the comment in the filesystem_object_id fallback
to reference the actual winapi-util crate used by the implementation and
Cargo.toml, replacing the incorrect same-file reference while preserving the
explanation of the Windows identity behavior.
- Around line 148-170: Wrap the filesystem metadata fallback in
filesystem_object_id with #[cfg(not(windows))] so Windows builds exclude the
code after the Windows-specific return and avoid unreachable_code warnings. Keep
the existing winapi-util Windows implementation and API usage unchanged.
---
Nitpick comments:
In `@src-tauri/src/commands.rs`:
- Around line 150-195: Update the iterator in the request-processing flow to use
map instead of flat_map, since each branch currently returns exactly one
CleanResult wrapped in a Vec. Return a single CleanResult directly from the
matches and trash_delete_if_identity branches, preserving their existing success
and error behavior, then collect the mapped results.
In `@src-tauri/src/icloud_sync_health.rs`:
- Around line 901-916: Extend
oversized_cloud_docs_database_fails_closed_before_snapshot_copy to also create a
within-limit client.db and an oversized client.db-wal, then verify
clone_client_database_snapshot returns the existing
"icloud-sync-health-snapshot-source-too-large" error.
🪄 Autofix
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 Plus
Run ID: 57e6b033-0c5e-4d82-a2ec-f284e267593a
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
README.mdsrc-tauri/Cargo.tomlsrc-tauri/src/cloud.rssrc-tauri/src/commands.rssrc-tauri/src/dev_artifacts.rssrc-tauri/src/icloud_sync_health.rssrc-tauri/src/lib.rssrc-tauri/src/safety.rssrc/lib/api.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src-tauri/src/lib.rs
- src/lib/api.ts
- src-tauri/src/dev_artifacts.rs
|
Follow-up
Focused tests: safety 43, commands 22, cloud 46, iCloud health 9; full Rust lib 545 passed / 1 ignored. |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head88ea9f7826625ea7b81024fdfb9542d00ffaec75. -
Head SHA:
88ea9f7826625ea7b81024fdfb9542d00ffaec75 -
Workflow run: 31490697391
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 3
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (11 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (11 files)"]
R1 --> V1["required checks"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (14 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (14 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: 2026-07-21-apfs-reclaim-evidence-design.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: 2026-07-21-apfs-reclaim-evidence-design.md"]
R2 --> V2["docs review"]
|
|
@coderabbitai review |
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headd0ff837d9081df2ef0c6094ef4f22cb249d0a651. -
Head SHA:
d0ff837d9081df2ef0c6094ef4f22cb249d0a651 -
Workflow run: 31498750321
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 3
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (11 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (11 files)"]
R1 --> V1["required checks"]
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src-tauri/src/reclaim.rs`:
- Around line 438-444: Update src-tauri/src/reclaim.rs lines 438-444 to select
the active-use query based on RootKind::File versus RootKind::Directory; update
active_use_evidence in src-tauri/src/git_worktree.rs lines 607-611 so regular
files use lsof without +D while directories retain recursive probing; revise the
test in src-tauri/src/reclaim.rs lines 537-552 to assert that a regular-file
root uses the exact-file query instead of merely allowing probe errors.
🪄 Autofix
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 Plus
Run ID: bb09b544-ed7a-4224-8544-2074d86c307f
📒 Files selected for processing (9)
README.mddocs/superpowers/specs/2026-07-21-apfs-reclaim-evidence-design.mdsrc-tauri/src/bin/disksage-reclaim-plan.rssrc-tauri/src/cloud.rssrc-tauri/src/commands.rssrc-tauri/src/git_worktree.rssrc-tauri/src/icloud_sync_health.rssrc-tauri/src/reclaim.rssrc-tauri/src/safety.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- src-tauri/src/cloud.rs
- src-tauri/src/commands.rs
- README.md
- src-tauri/src/safety.rs
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headbe67175c0bc7a619fb5b82c16f5433007b480a40. -
Head SHA:
be67175c0bc7a619fb5b82c16f5433007b480a40 -
Workflow run: 31504366418
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 3
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (14 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (14 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: 2026-07-21-apfs-reclaim-evidence-design.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: 2026-07-21-apfs-reclaim-evidence-design.md"]
R2 --> V2["docs review"]
1 participant
Summary
Safety
Validation
cargo test --locked --quiet --lib(542 passed, 1 ignored)npm run check(0 errors, 0 warnings)npm test -- --run(81 passed)npm run build(success)Base: current
main7b81efd43c48439d7a4a5508f6d09ac15f141546; final headfd11e8ccab93d644ce81459fc78c0415bc903431.Summary by CodeRabbit