Skip to content

fix delete failed#418

Merged
SuperCoolPencil merged 4 commits intomainfrom
fix-delete-failed
Apr 27, 2026
Merged

fix delete failed#418
SuperCoolPencil merged 4 commits intomainfrom
fix-delete-failed

Conversation

@SuperCoolPencil
Copy link
Copy Markdown
Member

@SuperCoolPencil SuperCoolPencil commented Apr 27, 2026

  • fix: handle missing downloads gracefully by ignoring not-found errors during deletion
  • fix: make cancellation idempotent and add resilience for download deletion in TUI
    Closes Cannot delete a non-downloaded file #373

Greptile Summary

This PR fixes a delete-failure bug by making Cancel idempotent (returning nil for missing downloads) and replacing brittle string-matching in the TUI with a proper errors.Is(err, types.ErrNotFound) guard. It also consolidates scattered fmt.Errorf literals into a shared sentinel errors package in internal/engine/types/errors.go.

Confidence Score: 5/5

Safe to merge; only a P2 style inconsistency found.

All findings are P2. The core fix (idempotent Cancel + sentinel error guard in TUI) is correct and well-tested. The only gap is one missed fmt.Errorf in ResumeBatch that was not converted alongside the rest of the refactoring.

internal/processing/pause_resume.go — ResumeBatch cold path at line 217 still uses fmt.Errorf instead of types.ErrNotFound.

Important Files Changed

Filename Overview
internal/engine/types/errors.go Introduces sentinel errors to replace scattered fmt.Errorf calls; clean and well-structured.
internal/processing/pause_resume.go Cancel made idempotent with clear comment explaining the rationale; one fmt.Errorf was missed in ResumeBatch cold path.
internal/tui/update_dashboard.go Delete handler now uses errors.Is(err, types.ErrNotFound) instead of brittle string matching — the primary TUI fix.
internal/tui/delete_resilience_test.go New test file covering ErrNotFound, success, and other-error paths for the TUI delete handler; good coverage of the defensive layer.
internal/core/local_service.go Inline fmt.Errorf calls replaced with sentinel errors; GetStatus now returns types.ErrNotFound for missing downloads.
internal/download/pool.go UpdateURL error strings replaced with ErrQueuedUpdate and ErrActiveUpdate sentinels.
internal/engine/concurrent/downloader.go 10-redirect guard replaced with ErrMaxRedirects sentinel; no behavioral change.
internal/engine/single/downloader.go Same ErrMaxRedirects sentinel change as concurrent downloader; consistent.
internal/processing/manager.go Enqueue/EnqueueWithID nil-func guards replaced with ErrServiceUnavailable sentinel; straightforward.
internal/processing/manager_test.go Cancel_NotFound test correctly inverted to expect nil error after idempotent Cancel change.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: internal/processing/pause_resume.go
Line: 217

Comment:
**Missed sentinel error in `ResumeBatch` cold path**

This `fmt.Errorf("download not found or completed")` was not converted as part of the sentinel-error refactoring in this PR. Any caller (including the TUI) that tries `errors.Is(err, types.ErrNotFound)` on a `ResumeBatch` result will silently miss this case.

```suggestion
errs[idx] = types.ErrNotFound
```

**Rule Used:** What: Eliminate duplicate logic, functions, or cod... ([source](https://app.greptile.com/review/custom-context?memory=f0a796a9-684f-4dfb-a5cf-8c99c16b63a1))

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

Reviews (2): Last reviewed commit: "refactor: centralize error handling by d..." | Re-trigger Greptile

Context used:

  • Rule used - What: Eliminate duplicate logic, functions, or cod... (source)

Comment thread internal/tui/update_dashboard.go Outdated
Comment thread internal/processing/pause_resume.go
Comment thread internal/tui/delete_resilience_test.go
@SuperCoolPencil SuperCoolPencil merged commit 5af64dd into main Apr 27, 2026
16 checks passed
@SuperCoolPencil SuperCoolPencil deleted the fix-delete-failed branch April 27, 2026 09:41
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.

Cannot delete a non-downloaded file

1 participant