Implement optimistic probe fallback and concurrent bootstrap

[SuperCoolPencil]

Overview

This PR improves the robustness of the download lifecycle by handling cases where the initial HEAD/GET probe fails. Instead of failing the download immediately, it now falls back to an optimistic configuration and attempts a metadata bootstrap during the download phase.

Changes

• Concurrent Downloader: Added bootstrapMetadata to discover file size using Range: bytes=0-0 if it's unknown at start.
• Single Downloader: Added logic to capture Content-Length from the first successful download request if the size was previously unknown.
• Lifecycle Manager: Modified probe logic to treat probe errors as non-fatal, falling back to optimistic range support.
• Tests: Added a test case verifying that probe failures no longer block the download dispatch.

Greptile Summary

This PR adds an optimistic probe-fallback path: when the initial HEAD/GET probe fails, the download is queued with SupportsRange=true and TotalSize=0, the concurrent downloader attempts a Range: bytes=0-0 bootstrap to discover the file size, and on failure the single-threaded downloader takes over. The lifecycle manager now also treats probe errors as non-fatal (except for invalid-scheme and request-creation failures), and context.DeadlineExceeded is correctly excluded from the fallback trigger alongside context.Canceled. The remaining findings are all P2 — the scheme detection still uses fragile strings.Contains on a runtime error message, the bootstrap happy-path test omits file-content verification, and the effectiveTotalSize state-fallback path has a minor silent-stale-value risk when both downloaders return no size information.

Confidence Score: 5/5

Safe to merge; all remaining findings are P2 style or observability improvements that do not block the primary download path

All previously flagged P0/P1 issues (stale tail bytes, missing DeadlineExceeded guard, bootstrapMetadata using wrong context) have been addressed. The three new comments are P2: fragile string matching for scheme detection, incomplete test coverage in the bootstrap happy-path, and a minor stale-size observability gap. None block correctness of the new fallback path.

internal/processing/manager.go — scheme detection via strings.Contains; internal/engine/concurrent/concurrent_test.go — bootstrap happy-path test coverage

Important Files Changed

Filename	Overview
internal/download/manager.go	Adds concurrent-to-single fallback with state reset, file truncation, and DeadlineExceeded guard; minor concern about stale state size being used as effectiveTotalSize fallback
internal/processing/manager.go	Probe error now non-fatal with url.Error/net.OpError guards; string matching for unsupported protocol scheme is still fragile
internal/engine/concurrent/downloader.go	Adds bootstrapMetadata using Range: bytes=0-0 to discover file size; correctly uses downloadCtx; TotalSize exposed for caller
internal/engine/single/downloader.go	Captures Content-Length from first GET response when fileSize is unknown; TotalSize exposed for caller; clean implementation
internal/processing/probe.go	Adds ErrProbeRequestCreation sentinel and wraps request-creation failures using dual %w (Go 1.20+) for clean errors.Is/As detection
internal/download/manager_test.go	Two new tests: bootstrap without probe metadata, and optimistic concurrent fallback to single; both are meaningful integration paths
internal/processing/manager_test.go	Adds test verifying 403 probe failure still dispatches with optimistic supportsRange=true and totalSize=0
internal/engine/concurrent/concurrent_test.go	Three bootstrap tests added; happy-path test doesn't verify file contents after bootstrap, only that TotalSize was set

Prompt To Fix All With AI

This is a comment left during a code review.
Path: internal/engine/concurrent/downloader.go
Line: 753-754

Comment:
**Bootstrap test asserts chunks but doesn't verify content**

`TestConcurrentDownloader_Download_BootstrapSize` checks that `downloader.TotalSize` is set correctly after bootstrap, but the fake server handler for chunk requests returns `make([]byte, 1024)` (zeroed bytes) without validating the Range header, and without checking `Content-Range` on subsequent responses. The test confirms metadata discovery but doesn't exercise the subsequent chunk-dispatch path against a correctly range-aware handler — masking any mismatch between the bootstrapped size and actual data written. Calling `testutil.VerifyFileSize` on the `.surge` file (as done in `TestConcurrentDownloader_Download`) would add confidence here.

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

---

This is a comment left during a code review.
Path: internal/processing/manager.go
Line: 267-270

Comment:
**`strings.Contains` for scheme detection is still fragile**

The terminal-error check still calls `strings.Contains(urlErr.Error(), "unsupported protocol scheme")` for scheme validation. While the `*net.OpError` guard narrows the scope, the check is brittle because `urlErr.Error()` embeds a runtime-generated message that could change across Go versions, and it also embeds the full URL, making false-positives possible if a URL contains that substring.

A more robust alternative is to parse the scheme with `url.Parse` directly and reject it before the probe is even attempted:

```go
if parsed, parseErr := url.Parse(req.URL); parseErr == nil {
    switch strings.ToLower(parsed.Scheme) {
    case "http", "https":
        // supported, continue
    default:
        return "", "", fmt.Errorf("unsupported URL scheme %q", parsed.Scheme)
    }
}
```

This is an up-front guard that never reaches the probe, avoids fragile string matching entirely, and is immune to changes in Go's error message text.

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

---

This is a comment left during a code review.
Path: internal/download/manager.go
Line: 145-151

Comment:
**`effectiveTotalSize` fallback from state silently uses stale size**

When `cfg.TotalSize <= 0`, `effectiveTotalSize` is populated from `cfg.State.GetProgress()`. However, this state was previously set for a different download session (e.g. a prior concurrent attempt that wrote a non-zero total before failing). If the fallback single-stream download then discovers a different size via `Content-Length`, `cfg.TotalSize` and `effectiveTotalSize` are updated correctly — but if the single downloader also gets no `Content-Length`, `effectiveTotalSize` silently retains the stale state value and the `DownloadCompleteMsg` total will be incorrect. Consider logging a debug note when the state-based fallback path is taken so that mismatches are observable.

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

_{Reviews (6): Last reviewed commit: "fix(download): add context.DeadlineExcee..." | Re-trigger Greptile}

[github-actions]

Binary Size Analysis

⚠️ Size Increased

Version	Human Readable	Raw Bytes
Main	19.18 MB	20111652
PR	19.19 MB	20119844
Difference	8.00 KB	8192

[SuperCoolPencil]

@junaid2005p This fixes some website downloads not starting due to 403/405/500 status code

Please review