Skip to content

fix(scoop-download): Fix incorrect download success state#6473

Merged
niheaven merged 1 commit intoScoopInstaller:developfrom
z-Fng:fix-scoop-download
Dec 30, 2025
Merged

fix(scoop-download): Fix incorrect download success state#6473
niheaven merged 1 commit intoScoopInstaller:developfrom
z-Fng:fix-scoop-download

Conversation

@z-Fng
Copy link
Copy Markdown
Member

@z-Fng z-Fng commented Aug 31, 2025

Description

Motivation and Context

The scoop download command has the following problems:

  1. Download is marked as successful even when hash verification fails.
  2. After a failed download, subsequent successful downloads do not show the success prompt.
  3. Duplicate downloads of the same app.

changes

This PR makes the following changes:

  • fix(scoop-download):
    • Fix incorrect download success state.
    • Prevent duplicate downloads.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed download success notifications so the final success message is shown only when every individual download and validation check completes successfully; partial or per-item failures now prevent a misleading overall success report.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Aug 31, 2025

Walkthrough

Deduplicates input apps, adds a per-app failure flag ($dl_failure) and labeled outer loop, sets the flag on hash-check failure, and gates per-app success messages so success prints only when no per-app download/hash failure occurred.

Changes

Cohort / File(s) Summary
Download flow updates
libexec/scoop-download.ps1
- Deduplicate $apps (use Select-Object -Unique)
- Add labeled outer loop (app_loop) with continue app_loop control flow
- Introduce per-app $dl_failure flag initialized per iteration
- On hash-check failure set $dl_failure = $true and skip remaining URLs for that app
- Gate per-app success message on !$dl_failure
- Preserve existing manifest generation, URL validation, hash-warning and error logging behavior
Changelog
CHANGELOG.md
- Add bugfix entry: "Fix incorrect download success state" (#6473)

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User
  participant Script as scoop-download.ps1
  participant Remote as Remote URL(s)
  participant Cache as Local cache / Hasher

  User->>Script: run "scoop download <apps>"
  Script->>Script: dedupe apps
  loop per app (app_loop)
    Script->>Remote: attempt download from URL 1..N
    alt download fails
      Script->>Script: log error, set dl_failure = true
      Script-->>Script: continue app_loop
    else download succeeds
      Script->>Cache: verify hash
      alt hash mismatch or verify error
        Script->>Cache: purge invalid cache
        Script->>Script: log error/warning, set dl_failure = true
        Script-->>Script: continue app_loop
      else hash OK
        Script->>Script: mark app as successful
      end
    end
    Script->>User: if not dl_failure, print per-app success
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • niheaven

Poem

I thump my paw at tidy loops,
One hop per app — no dupe-filled groups.
If hashes sour, I bound away,
Skip the bad URLs without delay.
I cheer when every check will stay. 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely describes the main fix: correcting the download success state logic in scoop-download.
Linked Issues check ✅ Passed The PR addresses all three core requirements from issue #6413: prevents marking downloads as successful on hash failures, restores correct success-prompt behavior after failed downloads, and prevents duplicate app downloads.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the download success state issue. The modifications to scoop-download.ps1 and CHANGELOG.md are within the stated objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
libexec/scoop-download.ps1 (1)

60-60: Loop label is unnecessary with the proposed inner-loop control.

If you stop using continue app_loop (see suggestions below), drop the label for clarity.

-:app_loop foreach ($curr_app in $apps) {
+foreach ($curr_app in $apps) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 04b7ce7 and 6b950be.

📒 Files selected for processing (1)
  • libexec/scoop-download.ps1 (4 hunks)

Comment thread libexec/scoop-download.ps1
Comment thread libexec/scoop-download.ps1
Comment thread libexec/scoop-download.ps1 Outdated
@z-Fng z-Fng marked this pull request as draft November 5, 2025 08:41
@z-Fng z-Fng changed the title fix(scoop-download): Fix download success prompt, prevent duplicate downloads fix(scoop-download): Fix incorrect download success state Dec 24, 2025
@z-Fng z-Fng force-pushed the fix-scoop-download branch from 6b950be to 070699e Compare December 24, 2025 09:57
@z-Fng z-Fng marked this pull request as ready for review December 24, 2025 09:57
@z-Fng z-Fng force-pushed the fix-scoop-download branch from 070699e to a1c6c4b Compare December 24, 2025 10:07
@niheaven niheaven merged commit 309fb02 into ScoopInstaller:develop Dec 30, 2025
3 checks passed
@z-Fng z-Fng deleted the fix-scoop-download branch December 30, 2025 03:54
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.

2 participants