Review summary: #4323

Change reviewed

This PR moves cache-registry retention refresh out of the optimistic retrieve response and into a new agent confirmation step. It adds CacheEntryConfirmReq/CacheEntryConfirmResp and Client.CacheEntryConfirm in api/cache.go, extends the internal cache API interface, and calls the new endpoint from client.Restore only after an exact-match blob has been downloaded, digest-verified, archive-validated, and extracted successfully. Fallback restores deliberately skip confirmation, preserving the existing non-refresh behavior.

The confirmation is best-effort: failures are logged but do not turn a successful restore into an error. Its retry loop reuses the cache retry policy but runs under a five-second child-context deadline so a failed confirmation cannot consume repeated full HTTP client timeouts. RestoreResult.TotalDuration and the cache.duration_ms span attribute include confirmation time, and the restore span records cache.confirmed.

Context and evidence examined

• Read Linear issue A-1775, including its requirement to refresh registry retention only after the backing blob is known to be usable, preserve fallback behavior, and avoid making refresh failures fail builds.
• Read the full six-file PR diff and the complete changed files: api/cache.go, api/cache_test.go, internal/cache/client.go, internal/cache/integration_test.go, internal/cache/restore.go, and internal/cache/restore_test.go.
• Traced the surrounding restore flow, including missing-blob, digest-mismatch, invalid-archive, extraction, stale-entry invalidation, and blob-retention refresh paths.
• Checked api.BreakOnNonRetryable and the API client's HTTP behavior to verify retry classification, non-2xx handling, and context cancellation for the new call.
• Inspected the merged companion server PR (buildkite/buildkite#33703) to verify that the route, request address (target_paths, cache_key, and echoed scopes), success response, and 200 status match the agent client implementation.
• Read the complete PR conversation, all review comments, and review-thread resolution state. The prior concerns about an unbounded synchronous retry loop and duration telemetry are fixed and their threads are resolved. No L2 approval grant was present.
• Compared the current head with the commit reviewed in the preceding buildsworth pass. The only intervening change is a merge from main; none of the six PR files changed.
• Ran git diff --check origin/main...HEAD, which passed.

Tests and CI

The added tests cover the API success/error behavior, scope propagation, fallback and missing-address guards, a deadline spanning the whole retry operation, a successful exact restore confirming once, a failed confirmation remaining non-fatal, and a missing-blob restore not confirming.

I attempted to run go test ./api and go test ./internal/cache, but this review environment does not have the Go executable installed, so no local tests ran. The previous Buildkite agent build for the same cache implementation, build #14292 at commit 94bb086e, passed. The current PR build #14296 was still pending when this review was submitted; the other completed GitHub checks were successful or skipped as expected.

Findings and verdict

I found no new correctness, security, data-integrity, resource, or contract issues, and posted no inline comments. The implementation continues to address the earlier findings, including the total latency bound and end-to-end duration accounting.

I submitted a COMMENT rather than an approval. Although clean on this pass, the PR changes the agent's latency-sensitive cache restore and API interaction path, which is outside the supplied L1 approval ceiling and therefore still requires human approval.

Trigger source: automatic.