diff --git a/cmd/gopeed/main.go b/cmd/gopeed/main.go index dbe7c050..c6b96705 100644 --- a/cmd/gopeed/main.go +++ b/cmd/gopeed/main.go @@ -19,13 +19,13 @@ func main() { var wg sync.WaitGroup wg.Add(1) - var checksumOpt *base.ChecksumOption + var checksumOpt *http.ChecksumOption if args.checksum != nil && *args.checksum != "" { algo := "md5" if args.checksumAlgo != nil && *args.checksumAlgo != "" { algo = *args.checksumAlgo } - checksumOpt = &base.ChecksumOption{ + checksumOpt = &http.ChecksumOption{ Algorithm: algo, Expected: *args.checksum, } @@ -55,9 +55,11 @@ func main() { } }). Create(&base.Options{ - Path: *args.dir, - Extra: http.OptsExtra{Connections: *args.connections}, - Checksum: checksumOpt, + Path: *args.dir, + Extra: http.OptsExtra{ + Connections: *args.connections, + Checksum: checksumOpt, + }, }) if err != nil { panic(err) diff --git a/internal/protocol/http/fetcher.go b/internal/protocol/http/fetcher.go index 8d279518..42764df2 100644 --- a/internal/protocol/http/fetcher.go +++ b/internal/protocol/http/fetcher.go @@ -285,6 +285,12 @@ type Fetcher struct { primaryReadyOnce sync.Once primaryReadyCh chan struct{} + // checksumFailed marks that the last completed download failed + // checksum verification, so the next retry must force a full + // re-download instead of skipping connections that already + // reported Completed. + checksumFailed atomic.Bool + // Start pending mechanism startPending atomic.Bool resolvedCh chan struct{} // Signal when resolve completes @@ -721,12 +727,26 @@ func (f *Fetcher) doStart() error { // If retrying after error, reset connection states for retry if state == stateError { + forceFullReset := f.checksumFailed.Swap(false) + if forceFullReset { + // Checksum mismatch: the file content itself is wrong even + // though every connection reported Completed. Truncate the + // corrupt file so stale bytes cannot linger if the retry is + // interrupted before every connection re-writes its chunk. + if err := os.Truncate(f.meta.SingleFilepath(), 0); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to reset file after checksum mismatch: %w", err) + } + } f.connMu.Lock() for _, conn := range f.connections { - // Reset connections that can be retried - if !conn.Completed && conn.State != connCompleted { + // On a normal error, only reset connections that didn't + // finish. On a checksum mismatch, reset EVERY connection — + // "Completed" only means the download loop finished, not + // that the bytes were verified correct. + shouldReset := forceFullReset || (!conn.Completed && conn.State != connCompleted) + if shouldReset { if !f.hasSequentialPrefixLocked(conn) { - f.resetConnectionForRestart(conn) + f.resetConnectionForRestart(conn, forceFullReset) } conn.State = connNotStarted conn.failed = false @@ -1099,7 +1119,7 @@ func (f *Fetcher) runConnection(conn *connection) { // it only when no safe validator exists or If-Range returns a full 200. f.connMu.Lock() if !f.hasSequentialPrefixLocked(conn) { - f.resetConnectionForRestart(conn) + f.resetConnectionForRestart(conn, false) } f.connMu.Unlock() } @@ -1206,7 +1226,7 @@ func (f *Fetcher) downloadChunkOnce(conn *connection, client *http.Client, buf [ resumeProbe := f.canProbeSequentialResumeLocked(conn) intentionalRestart := sequentialSizeUnknown if sequentialSizeUnknown || (hasSequentialPrefix && !resumeProbe) { - f.resetConnectionForRestart(conn) + f.resetConnectionForRestart(conn, false) f.resolveDataPos.Store(0) f.rangeValidatorPinned = false intentionalRestart = true @@ -1641,7 +1661,7 @@ func (f *Fetcher) fallbackToSequentialDownload(conn *connection, ifRange string) f.rangeReprobeEligible = true f.rangeValidatorPinned = false f.ifRange = ifRange - f.resetConnectionForRestart(conn) + f.resetConnectionForRestart(conn, false) f.resolveDataPos.Store(0) return nil } @@ -1657,7 +1677,7 @@ func (f *Fetcher) restartSequentialDownload(conn *connection, ifRange string) { f.rangeReprobeEligible = true f.rangeValidatorPinned = false f.ifRange = ifRange - f.resetConnectionForRestart(conn) + f.resetConnectionForRestart(conn, false) f.resolveDataPos.Store(0) } @@ -2040,16 +2060,25 @@ func (f *Fetcher) helpOtherConnection(helper *connection) bool { return true } -func (f *Fetcher) resetConnectionForRestart(conn *connection) { - if f.meta.Res.Range { +func (f *Fetcher) resetConnectionForRestart(conn *connection, force bool) { + if f.meta.Res.Range && !force { return } + if f.meta.Res.Range && force { + // Checksum mismatch: the on-disk content for this chunk is + // confirmed wrong, so its bytes must be re-fetched from the + // origin server even though range support would normally let + // us just resume from the existing Downloaded position. + if conn.Chunk != nil { + conn.Chunk.Downloaded = 0 + } + } // Without range support a new request always starts from byte 0, // so pause/retry must restart instead of continuing from the old offset. if conn.Chunk == nil { conn.Chunk = newChunk(0, 0) - } else { + } else if !f.meta.Res.Range { conn.Chunk.Begin = 0 conn.Chunk.setEnd(0) conn.Chunk.Downloaded = 0 @@ -2113,7 +2142,7 @@ func (f *Fetcher) resumeConnections() { } } if !f.hasSequentialPrefixLocked(conn) { - f.resetConnectionForRestart(conn) + f.resetConnectionForRestart(conn, false) } // Reset the connection state for resume conn.ctx, conn.cancel = context.WithCancel(f.ctx) @@ -2238,10 +2267,14 @@ func (f *Fetcher) onDownloadComplete() { } func (f *Fetcher) verifyChecksum() error { - if f.meta == nil || f.meta.Opts == nil || f.meta.Opts.Checksum == nil { + if f.meta == nil || f.meta.Opts == nil || f.meta.Opts.Extra == nil { return nil } - chk := f.meta.Opts.Checksum + extra, ok := f.meta.Opts.Extra.(*fhttp.OptsExtra) + if !ok || extra.Checksum == nil { + return nil + } + chk := extra.Checksum if chk.Algorithm == "" && chk.Expected == "" { return nil } @@ -2272,6 +2305,7 @@ func (f *Fetcher) verifyChecksum() error { actual := hex.EncodeToString(h.Sum(nil)) expected := strings.ToLower(strings.TrimSpace(chk.Expected)) if !strings.EqualFold(actual, expected) { + f.checksumFailed.Store(true) return fmt.Errorf("checksum mismatch: expected %s, got %s", expected, actual) } return nil diff --git a/internal/protocol/http/fetcher_checksum_retry_test.go b/internal/protocol/http/fetcher_checksum_retry_test.go new file mode 100644 index 00000000..a7d3573a --- /dev/null +++ b/internal/protocol/http/fetcher_checksum_retry_test.go @@ -0,0 +1,108 @@ +package http + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + gohttp "net/http" + "net/http/httptest" + "os" + "strings" + "sync/atomic" + "testing" + + "github.com/GopeedLab/gopeed/pkg/base" + "github.com/GopeedLab/gopeed/pkg/protocol/http" +) + +func TestFetcher_ChecksumMismatch_Retry(t *testing.T) { + corruptContent := []byte("Corrupt Initial Download Data!") + correctContent := []byte("Correct Retried Download Data!") + + correctSha256Hasher := sha256.New() + correctSha256Hasher.Write(correctContent) + correctSha256 := hex.EncodeToString(correctSha256Hasher.Sum(nil)) + + var reqCount atomic.Int32 + server := httptest.NewServer(gohttp.HandlerFunc(func(w gohttp.ResponseWriter, r *gohttp.Request) { + count := reqCount.Add(1) + var served []byte + if count == 1 { + // First request: serve corrupted content + served = corruptContent + } else { + // Second / subsequent request: serve correct content + served = correctContent + } + w.Header().Set(base.HttpHeaderContentLength, fmt.Sprintf("%d", len(served))) + w.Header().Set(base.HttpHeaderAcceptRanges, base.HttpHeaderBytes) + w.WriteHeader(gohttp.StatusOK) + _, _ = w.Write(served) + })) + defer server.Close() + + downloadDir := t.TempDir() + fileName := "test_checksum_retry.txt" + + f := buildFetcher() + defer f.Close() + + opts := &base.Options{ + Path: downloadDir, + Name: fileName, + Extra: &http.OptsExtra{ + Connections: 1, + Checksum: &http.ChecksumOption{ + Algorithm: "sha256", + Expected: correctSha256, + }, + }, + } + + // 1. Initial attempt -> will download corruptContent, failing checksum verification + if err := f.Resolve(&base.Request{URL: server.URL + "/retry_test.txt"}, opts); err != nil { + t.Fatalf("Resolve error: %v", err) + } + if err := f.Start(); err != nil { + t.Fatalf("Start error: %v", err) + } + err := f.Wait() + if err == nil { + t.Fatal("expected checksum mismatch error on initial attempt, got nil") + } + if !strings.Contains(err.Error(), "checksum mismatch") { + t.Fatalf("expected 'checksum mismatch' in error, got %v", err) + } + if f.getState() != stateError { + t.Fatalf("expected stateError after checksum mismatch, got %v", f.getState()) + } + + // 2. Retry download -> should trigger forced full reset, truncate file, and re-download correctContent + if err := f.Start(); err != nil { + t.Fatalf("Start error on retry: %v", err) + } + if err := f.Wait(); err != nil { + t.Fatalf("Wait error on retry (expected success): %v", err) + } + if f.getState() != stateDone { + t.Fatalf("expected stateDone after successful retry, got %v", f.getState()) + } + + // 3. Verify on-disk file content matches the re-downloaded bytes + filePath := f.meta.SingleFilepath() + fileBytes, err := os.ReadFile(filePath) + if err != nil { + t.Fatalf("failed to read completed file: %v", err) + } + if string(fileBytes) != string(correctContent) { + t.Fatalf("on-disk file content mismatch:\ngot: %q\nwant: %q", string(fileBytes), string(correctContent)) + } + + // 4. Compute hash independently and verify + actualHasher := sha256.New() + actualHasher.Write(fileBytes) + actualHash := hex.EncodeToString(actualHasher.Sum(nil)) + if actualHash != correctSha256 { + t.Fatalf("re-computed on-disk hash mismatch: got %s, want %s", actualHash, correctSha256) + } +} diff --git a/internal/protocol/http/fetcher_checksum_test.go b/internal/protocol/http/fetcher_checksum_test.go index 2f7b5370..3d760011 100644 --- a/internal/protocol/http/fetcher_checksum_test.go +++ b/internal/protocol/http/fetcher_checksum_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/GopeedLab/gopeed/internal/fetcher" "github.com/GopeedLab/gopeed/internal/test" "github.com/GopeedLab/gopeed/pkg/base" "github.com/GopeedLab/gopeed/pkg/protocol/http" @@ -44,10 +45,10 @@ func TestFetcher_ChecksumVerification(t *testing.T) { Name: "test_valid_md5.txt", Extra: &http.OptsExtra{ Connections: 1, - }, - Checksum: &base.ChecksumOption{ - Algorithm: "md5", - Expected: correctMd5, + Checksum: &http.ChecksumOption{ + Algorithm: "md5", + Expected: correctMd5, + }, }, } @@ -71,10 +72,10 @@ func TestFetcher_ChecksumVerification(t *testing.T) { Name: "test_valid_sha256.txt", Extra: &http.OptsExtra{ Connections: 1, - }, - Checksum: &base.ChecksumOption{ - Algorithm: "sha256", - Expected: correctSha256, + Checksum: &http.ChecksumOption{ + Algorithm: "sha256", + Expected: correctSha256, + }, }, } @@ -98,10 +99,10 @@ func TestFetcher_ChecksumVerification(t *testing.T) { Name: "test_wrong_md5.txt", Extra: &http.OptsExtra{ Connections: 1, - }, - Checksum: &base.ChecksumOption{ - Algorithm: "md5", - Expected: wrongHash, + Checksum: &http.ChecksumOption{ + Algorithm: "md5", + Expected: wrongHash, + }, }, } @@ -129,10 +130,10 @@ func TestFetcher_ChecksumVerification(t *testing.T) { Name: "test_unsupported.txt", Extra: &http.OptsExtra{ Connections: 1, - }, - Checksum: &base.ChecksumOption{ - Algorithm: "invalid-algo", - Expected: wrongHash, + Checksum: &http.ChecksumOption{ + Algorithm: "invalid-algo", + Expected: wrongHash, + }, }, } @@ -149,6 +150,106 @@ func TestFetcher_ChecksumVerification(t *testing.T) { if !strings.Contains(err.Error(), "unsupported checksum algorithm") { t.Fatalf("expected 'unsupported checksum algorithm' in error, got %v", err) } + if f.checksumFailed.Load() { + t.Fatal("checksumFailed flag should NOT be set on unsupported algorithm error") + } + }) + + t.Run("Case-Insensitive Hash Comparison", func(t *testing.T) { + f := buildFetcher() + defer f.Close() + + opts := &base.Options{ + Path: t.TempDir(), + Name: "test_case_insensitive.txt", + Extra: &http.OptsExtra{ + Connections: 1, + Checksum: &http.ChecksumOption{ + Algorithm: "sha256", + Expected: strings.ToUpper(correctSha256), + }, + }, + } + + if err := f.Resolve(&base.Request{URL: server.URL + "/test.txt"}, opts); err != nil { + t.Fatalf("Resolve error: %v", err) + } + if err := f.Start(); err != nil { + t.Fatalf("Start error: %v", err) + } + if err := f.Wait(); err != nil { + t.Fatalf("Wait error (expected success with upper case hash): %v", err) + } + }) + + t.Run("Empty Expected Checksum (Skipped)", func(t *testing.T) { + f := buildFetcher() + defer f.Close() + + opts := &base.Options{ + Path: t.TempDir(), + Name: "test_empty_expected.txt", + Extra: &http.OptsExtra{ + Connections: 1, + Checksum: &http.ChecksumOption{ + Algorithm: "", + Expected: "", + }, + }, + } + + if err := f.Resolve(&base.Request{URL: server.URL + "/test.txt"}, opts); err != nil { + t.Fatalf("Resolve error: %v", err) + } + if err := f.Start(); err != nil { + t.Fatalf("Start error: %v", err) + } + if err := f.Wait(); err != nil { + t.Fatalf("Wait error (expected skip and success): %v", err) + } + }) + + t.Run("Large File Multi-Chunk Streaming (>64KB)", func(t *testing.T) { + largeData := make([]byte, 128*1024) + for i := range largeData { + largeData[i] = byte(i % 256) + } + largeSha256Hasher := sha256.New() + largeSha256Hasher.Write(largeData) + largeSha256 := hex.EncodeToString(largeSha256Hasher.Sum(nil)) + + largeServer := httptest.NewServer(gohttp.HandlerFunc(func(w gohttp.ResponseWriter, r *gohttp.Request) { + w.Header().Set(base.HttpHeaderContentLength, fmt.Sprintf("%d", len(largeData))) + w.Header().Set(base.HttpHeaderAcceptRanges, base.HttpHeaderBytes) + w.WriteHeader(gohttp.StatusOK) + _, _ = w.Write(largeData) + })) + defer largeServer.Close() + + f := buildFetcher() + defer f.Close() + + opts := &base.Options{ + Path: t.TempDir(), + Name: "test_large_file.bin", + Extra: &http.OptsExtra{ + Connections: 1, + Checksum: &http.ChecksumOption{ + Algorithm: "sha256", + Expected: largeSha256, + }, + }, + } + + if err := f.Resolve(&base.Request{URL: largeServer.URL + "/large.bin"}, opts); err != nil { + t.Fatalf("Resolve error: %v", err) + } + if err := f.Start(); err != nil { + t.Fatalf("Start error: %v", err) + } + if err := f.Wait(); err != nil { + t.Fatalf("Wait error (expected large file checksum success): %v", err) + } }) t.Run("No Checksum (Opt-in backward compatibility)", func(t *testing.T) { @@ -174,3 +275,33 @@ func TestFetcher_ChecksumVerification(t *testing.T) { } }) } + +func TestResetConnectionForRestart_ForceOnRangeDownload(t *testing.T) { + f := buildFetcher() + f.meta = &fetcher.FetcherMeta{ + Res: &base.Resource{ + Range: true, + }, + } + conn := &connection{ + Chunk: newChunk(0, 1024), + Downloaded: 512, + Completed: true, + } + conn.Chunk.Downloaded = 512 + + // Non-forced reset on range-supported download does not reset Chunk.Downloaded + f.resetConnectionForRestart(conn, false) + if conn.Chunk.Downloaded != 512 { + t.Fatalf("expected Chunk.Downloaded to remain 512 when force is false, got %d", conn.Chunk.Downloaded) + } + + // Forced reset on range-supported download resets Chunk.Downloaded to 0 + f.resetConnectionForRestart(conn, true) + if conn.Chunk.Downloaded != 0 { + t.Fatalf("expected Chunk.Downloaded to be reset to 0 when force is true, got %d", conn.Chunk.Downloaded) + } + if conn.Completed { + t.Fatal("expected Completed to be false after reset") + } +} diff --git a/phase.txt b/phase.txt deleted file mode 100644 index d7ac436f..00000000 --- a/phase.txt +++ /dev/null @@ -1,323 +0,0 @@ -GOPEED BACKEND CONTRIBUTION — MILESTONE & PHASE PLAN -====================================================== -Repo: https://github.com/GopeedLab/gopeed -Scope: Backend only (Go). No Flutter/UI work. -Target environment: Windows 10/11, Go 1.25+, no cgo/MinGW required for this scope. - -This document is written so an implementing agent (human or AI) can pick it up -and execute phase by phase without needing outside context. Each phase lists: -GOAL, FILES INVOLVED, STEPS, and DONE-WHEN (acceptance criteria). - -Work through the milestones in order. Do not start Milestone 2 until Milestone -1 is fully working end to end. - - -====================================================== -MILESTONE 0: ENVIRONMENT SETUP (Windows) -====================================================== -Purpose: Get a working local backend you can build, run, and test, with zero -dependency on Flutter/cgo/MinGW. - ---- Phase 0.1: Install prerequisites --- -GOAL: Have Go, Git, and a code editor ready. -STEPS: - 1. Install Go 1.25+ from https://go.dev/dl/ (Windows MSI installer). - Verify with: go version - 2. Install Git for Windows from https://git-scm.com/download/win - Verify with: git --version - 3. Install VS Code (or any editor) with the Go extension for - gofmt/go vet/debugging support. -DONE-WHEN: `go version` and `git --version` both print correctly in a fresh -PowerShell window. - ---- Phase 0.2: Fork and clone --- -GOAL: Have your own copy of the repo checked out locally. -STEPS: - 1. On GitHub, fork GopeedLab/gopeed into your own account (see contribute.txt - for the exact fork/PR process — this phase only covers the technical clone). - 2. In PowerShell: - git clone https://github.com//gopeed.git - cd gopeed - git remote add upstream https://github.com/GopeedLab/gopeed.git -DONE-WHEN: `git remote -v` shows both `origin` (your fork) and `upstream` -(GopeedLab/gopeed). - ---- Phase 0.3: Run the backend standalone --- -GOAL: Confirm the Go backend runs independently of Flutter. -STEPS: - 1. From the repo root, run: - go run cmd/api/main.go - 2. The API server starts on port 9999. Note: on Windows this uses TCP - (not a unix socket, which is what's used on Linux/macOS) — this is - already handled by the existing code, nothing to configure. - 3. Open http://localhost:9999 in a browser, or hit an API endpoint with - curl/Postman to confirm it responds. -DONE-WHEN: The server starts without errors and responds to a basic HTTP -request. - ---- Phase 0.4: Run existing tests --- -GOAL: Confirm the test suite passes on a clean checkout before you change -anything (this is your baseline). -STEPS: - 1. Run: go test ./... 2>&1 | Select-String -Pattern "FAIL|ok" - 2. Note any pre-existing failures (there may be a few environment-specific - ones, e.g. tests requiring network access or specific ports free). -DONE-WHEN: You have a clear list of which tests pass/fail BEFORE your changes, -so you can tell later whether you introduced regressions. - - -====================================================== -MILESTONE 1: HTTP DOWNLOAD CHECKSUM VERIFICATION -====================================================== -Purpose: Gopeed currently has NO production checksum/integrity verification -anywhere in the download path (confirmed by grepping the source — md5/sha256 -only appear in test utilities, never in internal/protocol/http/fetcher.go or -pkg/download/). This is the root cause behind reports like issue #1425 -(file size matches after a flaky download, but MD5 doesn't). This milestone -adds an opt-in checksum-verification feature to HTTP downloads. - ---- Phase 1.1: Explore the existing download model --- -GOAL: Understand where task options and request structs live, so you know -where to add a new field. -FILES: - - pkg/download/model.go (task/options structs) - - pkg/rest/model/task.go (REST API request/response shapes) - - pkg/base/model.go (base Request/Options types shared across protocols) -STEPS: - 1. Open pkg/base/model.go and find the `Options` struct (this is what - gets passed down into protocol-specific fetchers, e.g. HTTP's Fetcher). - 2. Open pkg/rest/model/task.go and find the struct used when a client - POSTs a new download task (likely something like CreateTaskReq). - 3. Trace how `Options` flows from the REST layer down into - internal/protocol/http/fetcher.go's `Resolve`/`Start` functions. -DONE-WHEN: You can draw (even just in comments) the path: REST request -> -pkg/download task creation -> base.Options -> http.Fetcher. - ---- Phase 1.2: Add the checksum option to the data model --- -GOAL: Add a new, optional field so existing tasks/clients are unaffected. -FILES: pkg/base/model.go, pkg/rest/model/task.go -STEPS: - 1. In pkg/base/model.go, add to the Options struct (or an ExtraOptions-style - sub-struct if one exists): - Checksum *ChecksumOption `json:"checksum,omitempty"` - Define: - type ChecksumOption struct { - Algorithm string `json:"algorithm"` // "md5" | "sha1" | "sha256" - Expected string `json:"expected"` // hex-encoded expected hash - } - 2. Mirror the same optional field in pkg/rest/model/task.go so it's - accepted from the REST API / MCP layer without breaking existing - callers that omit it. -DONE-WHEN: The project still compiles (`go build ./...`) with the new fields -added and unused. - ---- Phase 1.3: Implement the verification step in the HTTP fetcher --- -GOAL: After a download completes (all chunks written), if a checksum option -was provided, hash the completed file and compare. -FILES: internal/protocol/http/fetcher.go -STEPS: - 1. Locate where the fetcher considers the download "complete" — this is - after all `f.file.WriteAt(...)` calls across connections have finished - and chunks are reconciled (search for where the Fetcher transitions to - a "done"/completed state, near the download-loop / connection-completion - logic). - 2. Add a new unexported method, e.g.: - func (f *Fetcher) verifyChecksum() error - It should: - a. Check if f's Options has a non-nil Checksum field; if nil, return - nil immediately (feature is opt-in, zero cost otherwise). - b. Open the completed file for reading (os.Open on the final file - path — check how `f.file` / final path is referenced elsewhere - in this file for the correct pattern). - c. Stream the file through the correct hash.Hash implementation based - on Algorithm (crypto/md5, crypto/sha1, or crypto/sha256), using a - buffered copy (io.Copy into the hasher) — do NOT load the whole - file into memory. - d. Compare the resulting hex digest (encoding/hex) against Expected - (case-insensitive compare). - e. Return a descriptive error on mismatch, e.g. - fmt.Errorf("checksum mismatch: expected %s, got %s", expected, actual) - 3. Call verifyChecksum() at the point where the fetcher currently marks - itself complete/successful, BEFORE it reports success upward. If it - returns an error, the task must transition to a failed state with that - error message surfaced (follow the existing pattern used for other - terminal errors in this file, e.g. how isTerminalRangeError is handled). -DONE-WHEN: A download with a correct checksum finishes as "done"; a download -with a deliberately wrong checksum finishes as "failed" with a clear error -message, and the partial/complete file is NOT silently reported as good. - ---- Phase 1.4: Wire it through the REST/MCP layers --- -GOAL: Make the new option actually reachable from the public API and from -the MCP `create_task` tool, not just internally. -FILES: pkg/rest/api.go, pkg/mcpserver/server.go -STEPS: - 1. In pkg/rest/api.go, find the handler for task creation and confirm the - new Checksum field on the request struct is passed through unchanged - into the base.Options that gets handed to the downloader. - 2. In pkg/mcpserver/server.go, find the `create_task` tool's input schema - and handler; add the optional checksum fields there too, following the - exact pattern used for existing optional fields in that tool. -DONE-WHEN: You can create a task via a raw REST POST (curl/Postman) with a -`checksum` object in the request body and see it take effect. - ---- Phase 1.5: Tests --- -GOAL: Prove the feature works and doesn't regress existing behavior. -FILES: internal/protocol/http/fetcher_test.go (create if it doesn't exist; -check internal/test/util.go for existing FileMd5/DirMd5 helpers you can -reuse), plus any relevant pkg/download tests. -STEPS: - 1. Write a test that downloads a small known-content file (use the - existing test HTTP server helpers in internal/test/httptest.go) with a - correct checksum option -> assert task succeeds. - 2. Write a test with an intentionally wrong checksum -> assert task fails - with the expected error. - 3. Write a test with no checksum option set -> assert behavior is - byte-for-byte identical to before this change (no regression). - 4. Run go test ./internal/protocol/http/... and go test ./pkg/download/... -DONE-WHEN: All three test cases pass, and Phase 0.4's baseline test suite -still passes (no regressions elsewhere). - ---- Phase 1.6: Documentation and PR prep --- -GOAL: Make the change reviewable and mergeable. -STEPS: - 1. Add/update any relevant API documentation (check if there's an OpenAPI - spec or docs folder referenced from the REST layer). - 2. Write a clear PR description referencing issue #1425 and explaining - that this is an opt-in feature (no behavior change for existing - clients/tasks that don't set a checksum). - 3. Run `go vet ./...` and `gofmt -l .` to confirm formatting/lint clean. -DONE-WHEN: PR is ready to open (see contribute.txt for the actual submission -steps). - - -====================================================== -MILESTONE 2: NEW MCP TOOL (e.g. get_download_speed / set_task_priority) -====================================================== -Purpose: Extend the AI-agent-facing MCP surface with a genuinely useful new -tool, following the existing pattern exactly. - ---- Phase 2.1: Study the existing tool pattern --- -GOAL: Understand how a tool is registered end to end. -FILES: pkg/mcpserver/server.go -STEPS: - 1. Read the full file (525 lines) once, top to bottom. - 2. Pick one simple existing tool (e.g. `pause_task` or `get_task_status`) - and trace: schema definition -> handler function -> which internal - download-manager function it calls -> how the result is shaped and - returned to the MCP caller. -DONE-WHEN: You can explain, in your own words, the 4-step shape every tool -follows in this file. - ---- Phase 2.2: Pick and scope the new tool --- -GOAL: Decide exactly what the tool does and its input/output shape. -STEPS: - 1. Recommended: `get_download_speed` — returns current instantaneous - download speed (and maybe ETA) for one task or all active tasks. - This likely already has underlying data available via - pkg/base/stats.go or the existing `get_task_stats` tool's data source - — check there first so you're not duplicating logic, just exposing it - differently. - 2. Write down: tool name, input parameters (e.g. optional task ID), - output fields, and error cases (task not found, task not running). -DONE-WHEN: You have a one-paragraph spec written before touching code. - ---- Phase 2.3: Implement --- -GOAL: Add the tool following the exact existing pattern. -FILES: pkg/mcpserver/server.go -STEPS: - 1. Add the tool's schema definition alongside the others. - 2. Add the handler function; reuse existing download-manager/controller - functions (pkg/download/downloader.go, internal/controller/controller.go) - rather than writing new state-access logic — this tool should be a - thin read-only wrapper. - 3. Register the tool in the same place the others are registered. -DONE-WHEN: The MCP server lists the new tool and it can be invoked -successfully against a running download. - ---- Phase 2.4: Tests + PR --- -STEPS: - 1. Add a test in the mcpserver package covering the happy path and the - "task not found" error path. - 2. Run go test ./pkg/mcpserver/... - 3. Prepare PR description explaining the tool's purpose and giving an - example natural-language use case (matching the style of the README's - AI Integration table). -DONE-WHEN: Tests pass and PR is ready. - - -====================================================== -MILESTONE 3: WEBHOOK ENHANCEMENTS -====================================================== -Purpose: pkg/download/webhook.go is small (229 lines) and self-contained — -a good second/parallel track. Add a new event type or richer payload data. - ---- Phase 3.1: Study current webhook events --- -FILES: pkg/download/webhook.go, pkg/rest/model/webhook.go -STEPS: - 1. List all current event types fired (e.g. task completed, task error). - 2. Identify a gap — e.g. no "task paused" event, or payload lacks final - file size / duration / average speed. -DONE-WHEN: You have a specific, small gap identified (do not try to redesign -the whole webhook system). - ---- Phase 3.2: Implement --- -STEPS: - 1. Add the new event constant/type. - 2. Add the trigger call at the correct point in the download lifecycle - (pkg/download/downloader.go or wherever the corresponding state - transition already happens for existing events). - 3. Extend the payload struct in pkg/rest/model/webhook.go with the new - field(s), keeping existing fields untouched for backward compatibility. -DONE-WHEN: Triggering the relevant download state change fires the new/ -enhanced webhook, verifiable by pointing it at a local test HTTP listener -(e.g. `python -m http.server` or a tiny Go test server) and inspecting the -received payload. - ---- Phase 3.3: Tests + PR --- -DONE-WHEN: A test covers the new event firing with the correct payload -shape, and the PR is ready. - - -====================================================== -MILESTONE 4: CLI PARITY -====================================================== -Purpose: cmd/gopeed/main.go is only ~98 lines — thin compared to the REST -API surface. Close a specific gap. - ---- Phase 4.1: Gap analysis --- -FILES: cmd/gopeed/main.go, cmd/gopeed/flags.go, pkg/rest/model/task.go -STEPS: - 1. List every REST endpoint/capability in pkg/rest/model/task.go. - 2. List every command currently supported by the CLI. - 3. Pick ONE missing capability (e.g. `gopeed pause `, - `gopeed list --status=running`) to implement — keep scope small. -DONE-WHEN: One specific missing CLI command is chosen and written down. - ---- Phase 4.2: Implement --- -STEPS: - 1. Add the new flag/subcommand in cmd/gopeed/flags.go following the - existing pattern. - 2. Wire it to call the corresponding REST endpoint (the CLI is a thin - HTTP client over the same API the Flutter app uses). -DONE-WHEN: Running the new CLI command against a live `go run cmd/api/main.go` -instance produces the expected effect and output. - ---- Phase 4.3: Tests + PR --- -DONE-WHEN: Manual verification documented in the PR description (CLI tools -are often tested manually/via a small integration test rather than heavy -unit testing); PR ready. - - -====================================================== -GENERAL RULES FOR EVERY MILESTONE -====================================================== -1. Never break backward compatibility — new fields must be optional - (`omitempty` in JSON, nil-safe checks in Go) so existing clients/tasks - are unaffected. -2. Run `gofmt -l .` and `go vet ./...` before every commit. -3. Run the full test suite (`go test ./...`) before opening a PR; compare - against the Phase 0.4 baseline to catch regressions. -4. Keep each PR scoped to ONE milestone (or even one phase within a - milestone) — small, reviewable PRs merge faster than large ones. -5. Reference the specific GitHub issue number in the PR description when - the milestone addresses a reported bug (e.g. Milestone 1 -> #1425). diff --git a/pkg/base/model.go b/pkg/base/model.go index 06e3da42..7b49b297 100644 --- a/pkg/base/model.go +++ b/pkg/base/model.go @@ -124,11 +124,6 @@ type FileInfo struct { Req *Request `json:"req"` } -type ChecksumOption struct { - Algorithm string `json:"algorithm"` // "md5" | "sha1" | "sha256" - Expected string `json:"expected"` // hex-encoded expected hash -} - // Options for download type Options struct { // Download file name @@ -141,8 +136,6 @@ type Options struct { SelectFiles []int `json:"selectFiles"` // Extra info for specific fetcher Extra any `json:"extra"` - // Checksum verification option - Checksum *ChecksumOption `json:"checksum,omitempty"` } func (o *Options) InitSelectFiles(fileSize int) { diff --git a/pkg/base/model_test.go b/pkg/base/model_test.go index 23ac3ac6..c0cf50f2 100644 --- a/pkg/base/model_test.go +++ b/pkg/base/model_test.go @@ -435,35 +435,3 @@ func TestDownloaderStoreConfig_Merge(t *testing.T) { }) } } - -func TestOptions_Checksum(t *testing.T) { - opts := &Options{ - Name: "test.zip", - Path: "/downloads", - Checksum: &ChecksumOption{ - Algorithm: "sha256", - Expected: "abcdef1234567890", - }, - } - - cloned := opts.Clone() - if cloned.Checksum == nil { - t.Fatal("expected cloned options to have non-nil Checksum") - } - if cloned.Checksum.Algorithm != "sha256" || cloned.Checksum.Expected != "abcdef1234567890" { - t.Fatalf("cloned checksum mismatch: got %+v", cloned.Checksum) - } - - // Verify deep clone isolation - cloned.Checksum.Expected = "modified" - if opts.Checksum.Expected != "abcdef1234567890" { - t.Fatal("modifying cloned checksum affected original options") - } - - // Verify nil checksum options clone - optsNil := &Options{Name: "nil_check.zip"} - clonedNil := optsNil.Clone() - if clonedNil.Checksum != nil { - t.Fatal("expected nil checksum in cloned options") - } -} diff --git a/pkg/mcpserver/server.go b/pkg/mcpserver/server.go index a7526381..10c6ec53 100644 --- a/pkg/mcpserver/server.go +++ b/pkg/mcpserver/server.go @@ -12,6 +12,7 @@ import ( "github.com/GopeedLab/gopeed/pkg/base" "github.com/GopeedLab/gopeed/pkg/download" + phttp "github.com/GopeedLab/gopeed/pkg/protocol/http" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -65,12 +66,13 @@ type btRequestExtra struct { } type httpTaskExtra struct { - Connections int `json:"connections,omitempty" jsonschema:"Number of HTTP download connections"` - AutoTorrent *bool `json:"autoTorrent,omitempty" jsonschema:"Automatically create a BitTorrent task after downloading a torrent file"` - DeleteTorrentAfterDownload *bool `json:"deleteTorrentAfterDownload,omitempty" jsonschema:"Delete the torrent file after automatically creating its BitTorrent task"` - AutoExtract *bool `json:"autoExtract,omitempty" jsonschema:"Automatically extract the downloaded archive"` - ArchivePassword string `json:"archivePassword,omitempty" jsonschema:"Password for an encrypted archive"` - DeleteAfterExtract bool `json:"deleteAfterExtract,omitempty" jsonschema:"Delete archive files after successful extraction"` + Connections int `json:"connections,omitempty" jsonschema:"Number of HTTP download connections"` + AutoTorrent *bool `json:"autoTorrent,omitempty" jsonschema:"Automatically create a BitTorrent task after downloading a torrent file"` + DeleteTorrentAfterDownload *bool `json:"deleteTorrentAfterDownload,omitempty" jsonschema:"Delete the torrent file after automatically creating its BitTorrent task"` + AutoExtract *bool `json:"autoExtract,omitempty" jsonschema:"Automatically extract the downloaded archive"` + ArchivePassword string `json:"archivePassword,omitempty" jsonschema:"Password for an encrypted archive"` + DeleteAfterExtract bool `json:"deleteAfterExtract,omitempty" jsonschema:"Delete archive files after successful extraction"` + Checksum *checksumOption `json:"checksum,omitempty" jsonschema:"Optional checksum verification settings"` } // downloadRequest is the MCP-facing form of base.Request. The REST model uses @@ -137,12 +139,11 @@ type checksumOption struct { // downloadOptions is the MCP-facing form of base.Options. Only HTTP currently // defines protocol-specific task options, so Extra has one concrete schema. type downloadOptions struct { - Name string `json:"name,omitempty" jsonschema:"Optional output file or directory name"` - Path string `json:"path,omitempty" jsonschema:"Optional download directory"` - AsDefaultPath bool `json:"asDefaultPath,omitempty" jsonschema:"Save path as the default download directory after task creation"` - SelectFiles []int `json:"selectFiles,omitempty" jsonschema:"Optional zero-based file indexes to download from a multi-file resource"` - Extra *httpTaskExtra `json:"extra,omitempty" jsonschema:"Optional HTTP task settings"` - Checksum *checksumOption `json:"checksum,omitempty" jsonschema:"Optional checksum verification settings"` + Name string `json:"name,omitempty" jsonschema:"Optional output file or directory name"` + Path string `json:"path,omitempty" jsonschema:"Optional download directory"` + AsDefaultPath bool `json:"asDefaultPath,omitempty" jsonschema:"Save path as the default download directory after task creation"` + SelectFiles []int `json:"selectFiles,omitempty" jsonschema:"Optional zero-based file indexes to download from a multi-file resource"` + Extra *httpTaskExtra `json:"extra,omitempty" jsonschema:"Optional HTTP task settings"` } func requestExtraSchema() *jsonschema.Schema { @@ -192,11 +193,23 @@ func (o *downloadOptions) baseOptions() *base.Options { if o == nil { return nil } - var checksum *base.ChecksumOption - if o.Checksum != nil { - checksum = &base.ChecksumOption{ - Algorithm: o.Checksum.Algorithm, - Expected: o.Checksum.Expected, + var extra any + if o.Extra != nil { + var checksum *phttp.ChecksumOption + if o.Extra.Checksum != nil { + checksum = &phttp.ChecksumOption{ + Algorithm: o.Extra.Checksum.Algorithm, + Expected: o.Extra.Checksum.Expected, + } + } + extra = &phttp.OptsExtra{ + Connections: o.Extra.Connections, + AutoTorrent: o.Extra.AutoTorrent, + DeleteTorrentAfterDownload: o.Extra.DeleteTorrentAfterDownload, + AutoExtract: o.Extra.AutoExtract, + ArchivePassword: o.Extra.ArchivePassword, + DeleteAfterExtract: o.Extra.DeleteAfterExtract, + Checksum: checksum, } } return &base.Options{ @@ -204,8 +217,7 @@ func (o *downloadOptions) baseOptions() *base.Options { Path: o.Path, AsDefaultPath: o.AsDefaultPath, SelectFiles: o.SelectFiles, - Extra: o.Extra, - Checksum: checksum, + Extra: extra, } } @@ -284,27 +296,11 @@ type getTaskStatsOutput struct { Stats any `json:"stats"` } -type getDownloadSpeedInput struct { - ID string `json:"id,omitempty" jsonschema:"Optional Gopeed task ID. If omitted, returns global download and upload speed across all active tasks."` -} - -type TaskSpeedInfo struct { - ID string `json:"id"` - Name string `json:"name,omitempty"` - Status base.Status `json:"status"` - Speed int64 `json:"speed"` // bytes/s - UploadSpeed int64 `json:"uploadSpeed"` // bytes/s - Downloaded int64 `json:"downloaded"` - Total int64 `json:"total"` - Progress float64 `json:"progress"` // percentage 0.0 - 100.0 - ETA *int64 `json:"eta,omitempty"` // estimated seconds remaining, if running and speed > 0 -} +type getDownloadSpeedInput struct{} type getDownloadSpeedOutput struct { - TotalSpeed int64 `json:"totalSpeed"` // total download speed in bytes/s - TotalUploadSpeed int64 `json:"totalUploadSpeed"` // total upload speed in bytes/s - ActiveTaskCount int `json:"activeTaskCount"` - Tasks []TaskSpeedInfo `json:"tasks,omitempty"` + TotalSpeed int64 `json:"totalSpeed"` + ActiveTaskCount int `json:"activeTaskCount"` } type taskActionOutput struct { @@ -456,57 +452,12 @@ func registerTools(server *mcp.Server, downloader *download.Downloader) { mcp.AddTool(server, &mcp.Tool{ Name: "get_download_speed", Title: "Get download speed", - Description: "Get instantaneous download/upload speed and progress/ETA for a specific task or across all active tasks.", + Description: "Returns the combined download speed across all currently active tasks and how many tasks are actively downloading. Use get_task_status instead if you need the speed, progress, or ETA of one specific task.", Annotations: &mcp.ToolAnnotations{ReadOnlyHint: true, OpenWorldHint: &closedWorld}, - }, func(_ context.Context, _ *mcp.CallToolRequest, input *getDownloadSpeedInput) (*mcp.CallToolResult, *getDownloadSpeedOutput, error) { - if input != nil && strings.TrimSpace(input.ID) != "" { - task, err := requireTask(downloader, input.ID) - if err != nil { - return nil, nil, err - } - status, err := downloader.RuntimeStatus(task.ID) - if err != nil { - return nil, nil, err - } - var progress float64 - if status.Total > 0 { - progress = float64(status.Downloaded) / float64(status.Total) * 100.0 - } - var eta *int64 - if status.Status == base.DownloadStatusRunning && status.Speed > 0 && status.Total > status.Downloaded { - remainingBytes := status.Total - status.Downloaded - etaSeconds := remainingBytes / status.Speed - eta = &etaSeconds - } - taskInfo := TaskSpeedInfo{ - ID: task.ID, - Name: task.Name(), - Status: status.Status, - Speed: status.Speed, - UploadSpeed: status.UploadSpeed, - Downloaded: status.Downloaded, - Total: status.Total, - Progress: progress, - ETA: eta, - } - activeCount := 0 - if status.Status == base.DownloadStatusRunning { - activeCount = 1 - } - return nil, &getDownloadSpeedOutput{ - TotalSpeed: status.Speed, - TotalUploadSpeed: status.UploadSpeed, - ActiveTaskCount: activeCount, - Tasks: []TaskSpeedInfo{taskInfo}, - }, nil - } - - // All tasks query + }, func(_ context.Context, _ *mcp.CallToolRequest, _ *getDownloadSpeedInput) (*mcp.CallToolResult, *getDownloadSpeedOutput, error) { allTasks := downloader.GetTasks() var totalSpeed int64 - var totalUploadSpeed int64 activeCount := 0 - tasksInfo := make([]TaskSpeedInfo, 0) for _, task := range allTasks { status, err := downloader.RuntimeStatus(task.ID) @@ -516,36 +467,12 @@ func registerTools(server *mcp.Server, downloader *download.Downloader) { if status.Status == base.DownloadStatusRunning { activeCount++ totalSpeed += status.Speed - totalUploadSpeed += status.UploadSpeed } - var progress float64 - if status.Total > 0 { - progress = float64(status.Downloaded) / float64(status.Total) * 100.0 - } - var eta *int64 - if status.Status == base.DownloadStatusRunning && status.Speed > 0 && status.Total > status.Downloaded { - remainingBytes := status.Total - status.Downloaded - etaSeconds := remainingBytes / status.Speed - eta = &etaSeconds - } - tasksInfo = append(tasksInfo, TaskSpeedInfo{ - ID: task.ID, - Name: task.Name(), - Status: status.Status, - Speed: status.Speed, - UploadSpeed: status.UploadSpeed, - Downloaded: status.Downloaded, - Total: status.Total, - Progress: progress, - ETA: eta, - }) } return nil, &getDownloadSpeedOutput{ - TotalSpeed: totalSpeed, - TotalUploadSpeed: totalUploadSpeed, - ActiveTaskCount: activeCount, - Tasks: tasksInfo, + TotalSpeed: totalSpeed, + ActiveTaskCount: activeCount, }, nil }) diff --git a/pkg/mcpserver/speed_test.go b/pkg/mcpserver/speed_test.go index 4e26b6d8..28df1eed 100644 --- a/pkg/mcpserver/speed_test.go +++ b/pkg/mcpserver/speed_test.go @@ -2,14 +2,29 @@ package mcpserver import ( "context" + "net/http" "net/http/httptest" "testing" + "time" + "github.com/GopeedLab/gopeed/pkg/base" "github.com/GopeedLab/gopeed/pkg/download" "github.com/modelcontextprotocol/go-sdk/mcp" ) func TestGetDownloadSpeedTool(t *testing.T) { + // Serve a test payload that takes a little bit of time or streams bytes + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", "1048576") + w.WriteHeader(http.StatusOK) + chunk := make([]byte, 8192) + for i := 0; i < 128; i++ { + _, _ = w.Write(chunk) + time.Sleep(10 * time.Millisecond) + } + })) + defer testServer.Close() + downloader := download.NewDownloader(&download.DownloaderConfig{ Storage: download.NewMemStorage(), StorageDir: t.TempDir(), @@ -29,7 +44,7 @@ func TestGetDownloadSpeedTool(t *testing.T) { } defer session.Close() - // 1. Query all tasks speed (empty list) + // 1. Zero tasks test (returns 0 totalSpeed and 0 activeTaskCount) result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ Name: "get_download_speed", Arguments: map[string]any{}, @@ -47,18 +62,42 @@ func TestGetDownloadSpeedTool(t *testing.T) { if activeCount, ok := structured["activeTaskCount"].(float64); !ok || activeCount != 0 { t.Fatalf("activeTaskCount = %v, want 0", structured["activeTaskCount"]) } + if totalSpeed, ok := structured["totalSpeed"].(float64); !ok || totalSpeed != 0 { + t.Fatalf("totalSpeed = %v, want 0", structured["totalSpeed"]) + } - // 2. Query non-existent task ID -> should return error - notFoundResult, err := session.CallTool(context.Background(), &mcp.CallToolParams{ - Name: "get_download_speed", - Arguments: map[string]any{ - "id": "non-existent-task-id", - }, + // 2. Start a real download task + taskID, err := downloader.CreateDirect(&base.Request{ + URL: testServer.URL, + }, &base.Options{ + Path: t.TempDir(), }) if err != nil { t.Fatal(err) } - if !notFoundResult.IsError { - t.Fatal("get_download_speed should return error for non-existent task ID") + defer func() { + _ = downloader.Delete(&download.TaskFilter{IDs: []string{taskID}}, true) + }() + + // Query get_download_speed while downloading + result, err = session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "get_download_speed", + Arguments: map[string]any{}, + }) + if err != nil { + t.Fatal(err) + } + if result.IsError { + t.Fatalf("get_download_speed returned error: %+v", result.Content) + } + structured, ok = result.StructuredContent.(map[string]any) + if !ok { + t.Fatalf("structured content type = %T", result.StructuredContent) + } + if _, ok := structured["activeTaskCount"].(float64); !ok { + t.Fatalf("activeTaskCount missing or wrong type: %v", structured["activeTaskCount"]) + } + if _, ok := structured["totalSpeed"].(float64); !ok { + t.Fatalf("totalSpeed missing or wrong type: %v", structured["totalSpeed"]) } } diff --git a/pkg/protocol/http/model.go b/pkg/protocol/http/model.go index 9e96ffe2..c99edf01 100644 --- a/pkg/protocol/http/model.go +++ b/pkg/protocol/http/model.go @@ -6,6 +6,13 @@ type ReqExtra struct { Body string `json:"body"` } +// ChecksumOption specifies an expected checksum to verify against +// the completed download. Only supported for HTTP downloads. +type ChecksumOption struct { + Algorithm string `json:"algorithm"` // "md5" | "sha1" | "sha256" + Expected string `json:"expected"` // hex-encoded expected hash +} + type OptsExtra struct { Connections int `json:"connections"` // AutoTorrent when task download complete, and it is a .torrent file, it will be auto create a new task for the torrent file @@ -21,6 +28,8 @@ type OptsExtra struct { ArchivePassword string `json:"archivePassword"` // DeleteAfterExtract when true, deletes the archive file after successful extraction DeleteAfterExtract bool `json:"deleteAfterExtract"` + // Checksum verifies the completed file against an expected hash after download + Checksum *ChecksumOption `json:"checksum,omitempty"` } // Stats for download diff --git a/pkg/protocol/http/model_test.go b/pkg/protocol/http/model_test.go new file mode 100644 index 00000000..e9348a88 --- /dev/null +++ b/pkg/protocol/http/model_test.go @@ -0,0 +1,15 @@ +package http + +import "testing" + +func TestOptsExtra_Checksum(t *testing.T) { + extra := &OptsExtra{ + Checksum: &ChecksumOption{ + Algorithm: "sha256", + Expected: "abcdef1234567890", + }, + } + if extra.Checksum.Algorithm != "sha256" || extra.Checksum.Expected != "abcdef1234567890" { + t.Fatalf("unexpected checksum: %+v", extra.Checksum) + } +} diff --git a/test_report.txt b/test_report.txt deleted file mode 100644 index ab77703d..00000000 --- a/test_report.txt +++ /dev/null @@ -1,131 +0,0 @@ -====================================================================== -GOPEED BACKEND IMPLEMENTATION & VERIFICATION TEST REPORT -====================================================================== -Date: 2026-09-22 -Repository: https://github.com/GopeedLab/gopeed -Scope: Backend Only (Go 1.26.5 / Windows x86_64) -Status: ALL TESTS PASSED - NO REGRESSIONS - ----------------------------------------------------------------------- -1. OVERVIEW OF IMPLEMENTATION & CHANGES ----------------------------------------------------------------------- - -Milestone 1: HTTP Download Checksum Verification -- Added `ChecksumOption` struct (Algorithm: md5/sha1/sha256, Expected: hex string) in pkg/base/model.go. -- Added optional `Checksum *ChecksumOption` (`json:"checksum,omitempty"`) to `base.Options`. -- Implemented streaming file checksum verification in `internal/protocol/http/fetcher.go` via `verifyChecksum()`. - Zero memory overhead (streams directly from file on disk via 64KB buffer). -- Wired checksum verification into the completion pathways of HTTP fetcher. Fails gracefully with descriptive mismatch error if hash does not match, marking task as failed. -- Wired optional checksum fields into MCP `downloadOptions` and `pkg/rest/model/task.go`. - -Milestone 2: New MCP Tool (get_download_speed) -- Implemented `get_download_speed` tool in `pkg/mcpserver/server.go`. -- Supports querying instantaneous download/upload speed, total/downloaded bytes, progress percentage, and ETA calculation for: - a) A specific task ID. - b) Global aggregate speed across all active running tasks when task ID is omitted. -- Registered in MCP tool registry with read-only annotations. - -Milestone 3: Webhook Lifecycle Enhancements -- Added `WebhookEventDownloadPause` ("DOWNLOAD_PAUSE") event type in `pkg/download/webhook.go`. -- Triggered `DOWNLOAD_PAUSE` event on task pause in `pkg/download/downloader.go`. - -Milestone 4: CLI Parity -- Added `-checksum` and `-checksum-algo` (default: md5) flags to `cmd/gopeed/flags.go`. -- Configured `cmd/gopeed/main.go` to construct `ChecksumOption` and pass it to download tasks. - ----------------------------------------------------------------------- -2. TEST EXECUTION SUMMARY ----------------------------------------------------------------------- - -Package: github.com/GopeedLab/gopeed/internal/protocol/http -- TestFetcher_ChecksumVerification/Valid_MD5: PASSED -- TestFetcher_ChecksumVerification/Valid_SHA256: PASSED -- TestFetcher_ChecksumVerification/Wrong_MD5_mismatch: PASSED (Proper error returned) -- TestFetcher_ChecksumVerification/Unsupported_Algorithm: PASSED (Proper error returned) -- TestFetcher_ChecksumVerification/No_Checksum_(Opt-in_backward_compatibility): PASSED -Result: PASS (All 5/5 subtests passed) - -Package: github.com/GopeedLab/gopeed/pkg/mcpserver -- TestStreamableHTTPTools: PASSED (All 10 tools verified including get_download_speed) -- TestStreamableHTTPRejectsCrossOriginRequest: PASSED -- TestGetDownloadSpeedTool: PASSED (Global speed & task not found error cases verified) -Result: PASS (3/3 tests passed) - -Package: github.com/GopeedLab/gopeed/pkg/download -- TestWebhook_TriggerOnPause: PASSED (Verified DOWNLOAD_PAUSE webhook event reception) -- TestWebhook_TriggerOnDone: PASSED -- TestWebhook_TriggerOnError: PASSED -- TestWebhook_SendTestWebhook: PASSED -Result: PASS - -Package: github.com/GopeedLab/gopeed/pkg/base -- TestDownloaderStoreConfig_Init: PASSED -- TestDownloaderStoreConfig_Merge: PASSED -- TestPieceMapCRUDAndBytes: PASSED -- TestPieceMapValidationAndRestore: PASSED -- TestPieceMapJSON: PASSED -- TestPieceMapCloneIsIndependent: PASSED -Result: PASS - -Package: github.com/GopeedLab/gopeed/pkg/rest -- TestNativeInvokeWithoutRESTListener: PASSED -- TestStartAPIServerIsNotBlockedByPendingDispatch: PASSED -- TestAPIServerLifecycleUsesPersistedConfigWithoutRestartingCore: PASSED -- TestRestartAPIServerFailsFastAndLeavesListenerStopped: PASSED -- TestNativeRESTSettingPersistsInGoStorage: PASSED -- TestNativeAPIServerAutoStartFailureKeepsCoreAvailable: PASSED -- TestInfo: PASSED -- TestResolve: PASSED -- TestCreateTask: PASSED -- TestCreateResolvedTaskUsesSubmittedOptions: PASSED -- TestCreateDirectTask: PASSED -- TestGetTaskStatus: PASSED -- TestCreateDirectTaskBatch: PASSED -- TestCreateDirectTaskBatchWithOpt: PASSED -- TestPauseAndContinueTask: PASSED -- TestPatchTask: PASSED -- TestPatchTaskNotFound: PASSED -- TestPauseAllAndContinueALLTasks: PASSED -- TestDeleteTask: PASSED -- TestDeleteTaskForce: PASSED -- TestDeleteAllTasks: PASSED -- TestDeleteTasksByStatues: PASSED -- TestGetTasks: PASSED -- TestGetAndPutConfig: PASSED -- TestInstallExtension: PASSED -- TestGetExtensions: PASSED -- TestUpdateExtensionSettings: PASSED -- TestSwitchExtension: PASSED -- TestDeleteExtension: PASSED -- TestUpdateCheckExtension: PASSED -- TestFsExtension: PASSED -- TestFsExtensionFail: PASSED -- TestWebFsEnhance: PASSED -- TestDoProxy: PASSED -- TestTestWebhook: PASSED -- TestApiToken: PASSED -- TestMCPRouteToggle: PASSED -- TestBuildServerRejectsWebAPITokenWithoutWebAuth: PASSED -- TestAuthorization: PASSED -- TestWebSessionStoreExpiry: PASSED -- TestBuildServerPropagatesWebViewProvider: PASSED -Result: PASS - -Build Verification: -- cmd/api: Compiled successfully (0 errors) -- cmd/gopeed: Compiled successfully (0 errors) - -Lint & Code Style: -- gofmt: Clean (all modified files formatted according to Go standards) -- go vet: Clean (0 warnings, 0 errors across all modified packages) - ----------------------------------------------------------------------- -3. BACKWARD COMPATIBILITY & UI INTEGRITY CONFIRMATION ----------------------------------------------------------------------- -- Zero changes to Flutter/UI codebase. -- All new options are purely opt-in (tagged with `omitempty` in JSON / nil-safe checks in Go). -- Existing API contracts, REST requests, and MCP tool workflows continue to function without any breaking alterations. - -====================================================================== -END OF REPORT -====================================================================== diff --git a/ui/flutter/lib/api/model/options.dart b/ui/flutter/lib/api/model/options.dart index 17aadd0d..35a50ac2 100644 --- a/ui/flutter/lib/api/model/options.dart +++ b/ui/flutter/lib/api/model/options.dart @@ -17,6 +17,21 @@ class Options { Map toJson() => _$OptionsToJson(this); } +@JsonSerializable() +class ChecksumOption { + String algorithm; + String expected; + + ChecksumOption({ + this.algorithm = '', + this.expected = '', + }); + + factory ChecksumOption.fromJson(Map json) => _$ChecksumOptionFromJson(json); + + Map toJson() => _$ChecksumOptionToJson(this); +} + @JsonSerializable() class OptsExtraHttp { int connections; @@ -25,6 +40,7 @@ class OptsExtraHttp { bool? autoExtract; String archivePassword; bool deleteAfterExtract; + ChecksumOption? checksum; OptsExtraHttp({ this.connections = 0, @@ -33,6 +49,7 @@ class OptsExtraHttp { this.autoExtract, this.archivePassword = '', this.deleteAfterExtract = false, + this.checksum, }); factory OptsExtraHttp.fromJson(Map json) => _$OptsExtraHttpFromJson(json); diff --git a/ui/flutter/lib/api/model/options.g.dart b/ui/flutter/lib/api/model/options.g.dart index 52b86453..557ea04f 100644 --- a/ui/flutter/lib/api/model/options.g.dart +++ b/ui/flutter/lib/api/model/options.g.dart @@ -22,6 +22,16 @@ Map _$OptionsToJson(Options instance) => { 'extra': ?instance.extra, }; +ChecksumOption _$ChecksumOptionFromJson(Map json) => ChecksumOption( + algorithm: json['algorithm'] as String? ?? '', + expected: json['expected'] as String? ?? '', +); + +Map _$ChecksumOptionToJson(ChecksumOption instance) => { + 'algorithm': instance.algorithm, + 'expected': instance.expected, +}; + OptsExtraHttp _$OptsExtraHttpFromJson(Map json) => OptsExtraHttp( connections: (json['connections'] as num?)?.toInt() ?? 0, autoTorrent: json['autoTorrent'] as bool?, @@ -29,6 +39,9 @@ OptsExtraHttp _$OptsExtraHttpFromJson(Map json) => OptsExtraHtt autoExtract: json['autoExtract'] as bool?, archivePassword: json['archivePassword'] as String? ?? '', deleteAfterExtract: json['deleteAfterExtract'] as bool? ?? false, + checksum: json['checksum'] == null + ? null + : ChecksumOption.fromJson(json['checksum'] as Map), ); Map _$OptsExtraHttpToJson(OptsExtraHttp instance) => { @@ -38,4 +51,5 @@ Map _$OptsExtraHttpToJson(OptsExtraHttp instance) => { final _httpHeadersController = AppHttpHeadersController(defaultNames: const ['User-Agent', 'Cookie', 'Referer']); final _trackersController = TextEditingController(); final _archivePasswordController = TextEditingController(); + final _checksumHashController = TextEditingController(); final _formScrollController = ScrollController(); String _httpMethod = 'GET'; @@ -74,6 +75,7 @@ class _CreateTaskWindowPageState extends ConsumerState { Map? _initialLabels; RequestProxyMode _proxyMode = RequestProxyMode.follow; String _proxyScheme = 'http'; + String _checksumAlgorithm = 'sha256'; bool _skipVerifyCert = false; bool? _autoTorrent; bool? _deleteTorrentAfterDownload; @@ -134,6 +136,7 @@ class _CreateTaskWindowPageState extends ConsumerState { _httpHeadersController.dispose(); _trackersController.dispose(); _archivePasswordController.dispose(); + _checksumHashController.dispose(); _formScrollController.dispose(); super.dispose(); } @@ -243,6 +246,12 @@ class _CreateTaskWindowPageState extends ConsumerState { _autoExtract = extra.autoExtract; _archivePasswordController.text = extra.archivePassword; _deleteAfterExtract = extra.deleteAfterExtract; + if (extra.checksum != null) { + if (extra.checksum!.algorithm.isNotEmpty) { + _checksumAlgorithm = extra.checksum!.algorithm; + } + _checksumHashController.text = extra.checksum!.expected; + } } } }); @@ -668,6 +677,32 @@ class _CreateTaskWindowPageState extends ConsumerState { ), ), ], + const SizedBox(height: 14), + AppFormPair( + direction: stacked ? Axis.vertical : Axis.horizontal, + firstLabel: 'Checksum Algorithm', + secondLabel: 'Checksum Hash', + firstFlex: 382, + secondFlex: 618, + first: AppChoiceSegmentedControl( + key: const ValueKey('create-task-checksum-algorithm'), + showIcons: false, + value: _checksumAlgorithm, + buttonKeyPrefix: 'create-task-checksum-algorithm', + alignment: WrapAlignment.start, + options: const [ + AppChoiceOption(value: 'md5', label: 'MD5', icon: Icons.tag), + AppChoiceOption(value: 'sha1', label: 'SHA-1', icon: Icons.tag), + AppChoiceOption(value: 'sha256', label: 'SHA-256', icon: Icons.tag), + ], + onChanged: (value) => setState(() => _checksumAlgorithm = value), + ), + second: _WindowTextField( + key: const ValueKey('create-task-checksum-expected'), + controller: _checksumHashController, + hintText: 'Expected hash (optional)', + ), + ), ] else ...[ AppFormRow( direction: stacked ? Axis.vertical : Axis.horizontal, @@ -1024,6 +1059,13 @@ class _CreateTaskWindowPageState extends ConsumerState { api_options.Options _buildOptions({String? name, List selectFiles = const []}) { final connections = int.tryParse(_connectionsController.text.trim()) ?? 0; + final checksumExpected = _checksumHashController.text.trim(); + final checksum = checksumExpected.isNotEmpty + ? api_options.ChecksumOption( + algorithm: _checksumAlgorithm, + expected: checksumExpected, + ) + : null; return api_options.Options( name: name ?? _renameController.text.trim(), path: _directoryController.text.trim(), @@ -1036,6 +1078,7 @@ class _CreateTaskWindowPageState extends ConsumerState { autoExtract: _autoExtract, archivePassword: _archivePasswordController.text, deleteAfterExtract: _deleteAfterExtract, + checksum: checksum, ).toJson(), ); } diff --git a/ui/flutter_old/pubspec.lock b/ui/flutter_old/pubspec.lock index 56f11284..5a9f98b0 100644 --- a/ui/flutter_old/pubspec.lock +++ b/ui/flutter_old/pubspec.lock @@ -157,10 +157,10 @@ packages: dependency: transitive description: name: characters - sha256: f71061c654a3380576a52b451dd5532377954cf9dbd272a78fc8479606670803 + sha256: faf38497bda5ead2a8c7615f4f7939df04333478bf32e4173fcb06d428b5716b url: "https://pub.dev" source: hosted - version: "1.4.0" + version: "1.4.1" checkable_treeview: dependency: "direct main" description: @@ -810,18 +810,18 @@ packages: dependency: transitive description: name: matcher - sha256: dc58c723c3c24bf8d3e2d3ad3f2f9d7bd9cf43ec6feaa64181775e60190153f2 + sha256: dc0b7dc7651697ea4ff3e69ef44b0407ea32c487a39fff6a4004fa585e901861 url: "https://pub.dev" source: hosted - version: "0.12.17" + version: "0.12.19" material_color_utilities: dependency: transitive description: name: material_color_utilities - sha256: f7142bb1154231d7ea5f96bc7bde4bda2a0945d2806bb11670e30b850d56bdec + sha256: "9c337007e82b1889149c82ed242ed1cb24a66044e30979c44912381e9be4c48b" url: "https://pub.dev" source: hosted - version: "0.11.1" + version: "0.13.0" menu_base: dependency: transitive description: @@ -1328,10 +1328,10 @@ packages: dependency: transitive description: name: test_api - sha256: ab2726c1a94d3176a45960b6234466ec367179b87dd74f1611adb1f3b5fb9d55 + sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" url: "https://pub.dev" source: hosted - version: "0.7.7" + version: "0.7.10" timezone: dependency: transitive description: