fix(upload): stop retrying 4xx errors and add tests for 6 packages

[appleboy]

Summary

• Bug fix: Upload and analysis packages now distinguish 4xx (non-retryable) from 5xx (retryable) HTTP errors, preventing wasteful retries on client errors like 400/404
• Bug fix: ScanMetadata.Version is now populated in upload payloads (was always nil)
• Test coverage: Added comprehensive test suites for 6 previously untested packages (41 new tests total)

Test Coverage Added

Package	Tests	Coverage
internal/pipeline/	10	96.5%
internal/output/	8	88.3%
internal/analysis/	7	88.7%
internal/tlsutil/	5	87.5%
internal/upload/	7	86.8%
internal/cli/	4	46.9%

Test plan

• make test — all tests pass with race detector
• make lint — 0 lint issues
• make build — compiles successfully
• Verify 4xx errors return immediately (no retry) via TestUpload_4xxNoRetry and TestAnalyze_4xxNoRetry
• Verify 5xx errors trigger retries via TestUpload_5xxRetries and TestAnalyze_5xxRetries

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR improves reliability and efficiency of result uploading and remote analysis by treating 4xx HTTP responses as non-retryable, while also ensuring upload payloads include scanner version metadata. It also adds first-pass test suites across several internal packages to increase confidence in core behaviors.

Changes:

• Stop retrying on HTTP 4xx responses in both upload and remote analysis (retry only on 5xx/network errors).
• Populate ScanMetadata.Version in upload payloads.
• Add new unit tests for internal/pipeline, internal/output, internal/analysis, internal/tlsutil, internal/upload, and internal/cli.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
internal/upload/uploader.go	Adds non-retryable clientError handling for 4xx and includes ScanMetadata.Version in payloads.
internal/upload/uploader_test.go	New tests for upload success/headers/metadata + retry/no-retry behaviors.
internal/analysis/analyzer.go	Adds non-retryable clientError handling for 4xx in remote analysis retries.
internal/analysis/analyzer_test.go	New tests for remote analysis request formation, merge behavior, and retry/no-retry semantics.
internal/tlsutil/tlsutil_test.go	New tests for transport cloning and TLS config cloning/skip-verify behavior.
internal/pipeline/pipeline_test.go	New tests covering pipeline control flow (inspect-only, paths vs discovery, nil components, continuation on failures).
internal/output/output_test.go	New tests for text/json formatting, summaries, truncation, and error-print toggles.
internal/cli/cli_test.go	New tests for control server CLI parsing behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

internal/upload/uploader.go:120

• The retry loop uses real time-based backoff (1s, 2s) via time.After, which makes unit tests slower and can be sensitive to CI load/context deadlines. If test runtime becomes an issue, consider making backoff/sleep injectable or configurable so tests can run with minimal delays, and optionally use a cancellable timer pattern instead of time.After.

	// Retry with exponential backoff (only retry on 5xx / network errors)
	maxRetries := 3
	for attempt := range maxRetries {
		err = u.doUpload(ctx, server, body)
		if err == nil {
			slog.Info("upload successful", "url", server.URL)
			return nil
		}
		// Do not retry client errors (4xx) or non-retryable errors (e.g., bad URL)
		var nre *nonRetryableError
		if errors.As(err, &nre) {
			return fmt.Errorf("upload failed: %w", err)
		}
		var ce *clientError
		if errors.As(err, &ce) {
			return fmt.Errorf(
				"upload failed due to non-retryable client error after %d attempt(s) (status=%d): %w",
				attempt+1,
				ce.StatusCode,
				err,
			)
		}
		if attempt < maxRetries-1 {
			backoff := time.Duration(1<<uint(attempt)) * time.Second
			slog.Debug("retrying upload", "attempt", attempt+1, "backoff", backoff)
			select {
			case <-time.After(backoff):
			case <-ctx.Done():
				return ctx.Err()
			}

internal/analysis/analyzer.go:163

• The retry loop uses real time-based backoff (1s, 2s) via time.After, which slows tests and can be sensitive to CI load/context deadlines. If this becomes an issue, consider injecting/configuring the backoff/sleeper so tests can run with minimal delays, and optionally use a cancellable timer pattern instead of time.After.

	// Retry with exponential backoff (only retry on 5xx / network errors)
	var resp analysisResponse
	maxRetries := 3
	for attempt := range maxRetries {
		err = a.doRequest(ctx, body, &resp)
		if err == nil {
			break
		}
		// Do not retry non-retryable errors (bad URL, JSON decode, etc.)
		var nre *nonRetryableError
		if errors.As(err, &nre) {
			return fmt.Errorf("analysis API: %w", err)
		}
		// Do not retry client errors (4xx)
		var ce *clientError
		if errors.As(err, &ce) {
			return fmt.Errorf("analysis API: %w", err)
		}
		if attempt < maxRetries-1 {
			backoff := time.Duration(1<<uint(attempt)) * time.Second
			slog.Debug("retrying analysis", "attempt", attempt+1, "backoff", backoff)
			select {
			case <-time.After(backoff):
			case <-ctx.Done():
				return ctx.Err()
			}
		}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.