Review summary: #4281

Change reviewed

This PR implements the agent half of A-1774, making stale cache-entry expiration target the exact entry returned by cache retrieval rather than reconstructing an address from target_paths and cache_key.

The API model in api/cache.go now decodes an opaque entry_ref from retrieve responses, echoes it in expire requests while retaining the legacy address fields, and returns a typed {message, existed} expire response. In internal/cache/restore.go, all stale-entry invalidation attempts use that reference when available, retain the existing retry behavior, and only report the cache.invalidated telemetry attribute as true when the server confirms that an entry existed and was deleted. The integration-test mock now assigns per-entry references, expires by reference, and models an existed: false no-op.

Context and code paths examined

I read the full five-file diff and the complete changed files, then traced every CacheEntryExpire, CacheEntryExpireReq, CacheEntryExpireResp, and EntryRef use in the repository. I checked the three restore degradation paths that call invalidateStaleEntry: missing blobs, digest mismatches, and archive-validation failures. I also reviewed the retry classification in api.BreakOnNonRetryable and the cache HTTP request/response decoding behavior.

I read Linear issue A-1774 and its acceptance criteria. I also inspected companion backend PR buildkite/buildkite#33348, including its API controller and request specs, to verify the wire contract: the backend mints a registry/agent/job-bound reference for the exact retrieved sort key and creation marker, ignores legacy address fields when a reference is present, conditionally avoids deleting a replacement, and returns existed: false for an idempotent no-op. The backend specs cover scoped exact entries, fallback entries, policy changes, replacement races, malformed references, and the legacy path; the agent mock intentionally does not implement fallback matching.

Finding

The only issue I found was already raised in the existing Codex review thread, so I did not create a duplicate inline thread. I replied in agreement. The new Existed == false behavior correctly returns false from invalidateStaleEntry, and the missing-blob and digest-mismatch completion messages no longer claim an invalidation occurred. However, the archive-validation failure path still unconditionally reports Cache miss (archive failed verification, invalidated stale entry). When expiration is a no-op or fails, that progress text contradicts the server result and the cache.invalidated telemetry attribute. This is a non-blocking observability inconsistency; removing the invalidation suffix, as the other two paths do, is the smallest correction.

No additional correctness, security, or compatibility findings survived the scope and concrete-harm checks.

Verification performed

The following local checks passed with Go 1.26.5:

• git diff --check origin/main...HEAD
• go test ./api
• go test ./internal/cache
• go vet ./api ./internal/cache
• go test -race ./internal/cache

I also ran go test ./.... It reached the broader suite but failed in the known environment-dependent internal/job SSH-alias test because this container lacks the expected /etc/ssh/ssh_config.d/ alias configuration (github.com-alias1 and cool-alias did not resolve). The changed packages passed independently, and this broad-suite failure is unrelated to the PR.

At review time, GitHub reported the Buildkite agent build #14127 as failing and the category-label check as failing; both Socket checks passed. I attempted to query Buildkite build jobs and logs, but the available Buildkite credentials could not access the buildkite organization, so I could not identify the failing CI job or inspect its log.

Verdict

I submitted a COMMENT review. The change alters an exported cache API/wire contract and runtime cache-restore behavior in the agent, which is above the supplied L1 approval ceiling. The review points to the existing progress-message thread rather than duplicating it.

Review: #4281 (review)

Trigger source: automatic.