Skip to content

fix: diagnose repeated restricted downloads#62

Merged
mannie-exe merged 4 commits into
mainfrom
dev
Apr 14, 2026
Merged

fix: diagnose repeated restricted downloads#62
mannie-exe merged 4 commits into
mainfrom
dev

Conversation

@mannie-exe
Copy link
Copy Markdown
Contributor

Summary

  • surface rerun restricted entries when build --continue still fails
  • stop trusting unchanged preexisting restricted cache files for new pending sessions with a baseline
  • add regressions for stale-cache refresh, rerun diagnostics, and continue recovery

Test plan

  • cargo fmt --all
  • cargo test -q -p empack-lib --features test-utils restricted_build -- --nocapture
  • cargo test -q -p empack-lib --features test-utils commands -- --nocapture
  • cargo test -q -p empack-tests --test build_continue -- --nocapture
  • cargo test -q -p empack-lib --features test-utils parse_export_restricted_output_preserves_deceasedcraft_section_sign_filename -- --nocapture
  • git diff --check

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 13, 2026

Greptile Summary

This PR improves the build --continue failure path by surfacing which restricted downloads are still required after a continue attempt, displaying a stale-cache diagnostic when the same entries recur, and stopping the cache from trusting preexisting unchanged files when a candidate_baseline is present. New unit and integration tests cover the stale-cache refresh, rerun diagnostic, and continue-recovery flows.

Confidence Score: 5/5

Safe to merge; the one finding is dead code that doesn't affect runtime behavior.

All identified issues are P2 or lower. The unreachable fallback error path in commands.rs is cosmetic dead code — the correct, more-informative error is always returned. Core stale-cache logic and comparison helpers are well-tested.

crates/empack-lib/src/application/commands.rs (dead fallback branch lines 3623–3626)

Important Files Changed

Filename Overview
crates/empack-lib/src/application/commands.rs Adds rerun diagnostics and detailed entry surfacing to continue-build failure path; one unreachable fallback error branch is dead code.
crates/empack-lib/src/empack/restricted_build.rs Introduces CacheEntryStatus enum and cache_entry_status helper; correctly treats preexisting-unchanged cache files as stale when a baseline is present.
crates/empack-lib/src/application/commands.test.rs Adds stale-cache refresh, rerun-diagnostic, and continue-recovery unit tests; negative assertions on diagnostic text are trivially satisfied since warnings go through the status display channel, not the error.
crates/empack-lib/src/empack/restricted_build.test.rs Adds comprehensive unit tests for missing_cached_entries, stage_cached_entries_to_destinations, and import_matching_downloads_into_cache under the new stale-cache logic; covers exact, fallback, and legacy paths.
crates/empack-tests/tests/build_continue.rs Adds an end-to-end integration test verifying that a stale restricted cache entry is refreshed from a new exact manual download and that the continued build succeeds.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/empack-lib/src/application/commands.rs
Line: 3616-3626

Comment:
**Unreachable fallback error path**

`restricted_rerun_error_detail` only returns `None` when its input slice is empty — but this code runs inside `if !restricted_entries.is_empty()`, so `dedup_restricted_mod_infos` always yields at least one entry and the function always returns `Some(...)`. The fallback `return Err` on line 3623 is dead code: the less-informative message can never be emitted.

Alternatively, replace `restricted_rerun_error_detail` with a non-`Option`-returning helper to make the invariant explicit.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (3): Last reviewed commit: "fix: trim duplicate rerun diagnostics" | Re-trigger Greptile

Surface rerun restricted entries after build --continue and stop
trusting unchanged preexisting restricted cache files for new
pending sessions with a baseline.
Rename the restricted rerun comparison variants to satisfy the
workspace clippy configuration without changing behavior.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 96.51347% with 22 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/empack-lib/src/empack/restricted_build.rs 74.54% 14 Missing ⚠️
crates/empack-lib/src/application/commands.rs 94.52% 8 Missing ⚠️

📢 Thoughts on this report? Let us know!

Deduplicate rerun display entries by download URL, reuse the
existing diagnostic string when building the returned error, and
make cache staging enforce the same Current semantics as the
upstream missing-entry checks.
Normalize rerun summary deduping across CurseForge URL variants and
keep the diagnostic sentence in structured terminal output instead
of repeating it in the returned continue-build error.
@mannie-exe mannie-exe merged commit bdd29de into main Apr 14, 2026
12 checks passed
@mannie-exe mannie-exe deleted the dev branch April 14, 2026 03:37
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