Review summary

Change reviewed

PR buildkite/agent#4321 aligns backing-blob retention with the existing cache-entry metadata policy from Linear issue A-1777: fallback restores should consume the matched cache without extending its lifetime, while exact hits should continue to refresh retention.

The implementation keeps fallback policy in the restore orchestration rather than adding it to Blob.Download. internal/cache/restore.go calls a new optional store.RetentionRefresher capability only after the download passes digest verification and only when retrieveResp.Fallback is false. NscStore.RefreshRetention retains the existing best-effort nsc artifact extend --ensure_minimum 72h behavior, and S3Blob.RefreshRetention retains the existing best-effort self-CopyObject behavior used to update LastModified. LocalFileBlob remains unaffected because it does not implement the optional capability.

What I examined

• Read the complete 461-line PR diff and all seven changed files.
• Read the A-1777 Linear issue, including its acceptance criteria for consistent exact-versus-fallback behavior and preservation of exact-hit refreshes.
• Traced the production restore path from Restore through downloadCache, digest verification, the optional retention capability, and the NSC/S3 implementations.
• Searched all cache-package Download, RefreshRetention, NewBlobStore, NscStore, and S3Blob call sites and struct literals to check interface satisfaction and ensure retention refresh had not been left in another download path.
• Checked the S3 client seam and confirmed the concrete SDK client still supplies the uploader, downloader, and new objectCopier capability in production.
• Reviewed the focused tests covering exact matches, fallback matches, stores without retention, NSC refresh behavior and best-effort failures, S3 copy inputs and best-effort failures, and prefix handling.
• Reviewed the PR conversation. The latest commit addresses the human reviewer's design concern by restoring the original Blob.Download contract and moving the decision into the restore flow.
• Ran git diff --check successfully against main.

I could not run Go tests or go vet in this review environment because the Go executable is not installed. The author reports running go test ./... locally. At submission time, GitHub showed the category and Socket checks passing, while the buildkite/agent build (#14247) and this buildsworth review build (#24859) were still pending. Direct Buildkite CLI lookup was unavailable with the current credentials.

Findings and verdict

I found no correctness, security, data-integrity, API-contract, or material pattern issues requiring an inline comment. The policy gate is centralized at the point that has both the fallback result and the successfully verified blob, exact-hit behavior remains represented by focused tests, and refresh failures preserve the existing non-fatal restore behavior.

Submitted an APPROVE review with no inline findings: #4321 (review)

Trigger source: automatic.