From 845f0cec7f077fce833601bb7e81e18c0e600d15 Mon Sep 17 00:00:00 2001
From: Sneha <sneha@buildkite.com>
Date: Mon, 7 Sep 2026 14:19:26 +1000
Subject: [PATCH 1/3] Honor the server's configured cache retention for backing
 blobs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The agent hard-coded the Namespace artifact lifetime to 72h on upload
(--expires_in) and on refresh (--ensure_minimum), so a cache registry
whose TTL is raised above 72h (via the new cache-retention service
quota) would keep live metadata past the point its backing blob had
already expired — turning later exact hits into hollow misses.

Thread the retention the server now returns (retention_seconds on the
cache store and retrieve responses) into the blob store: Upload and
RefreshRetention take a duration, and NscStore formats it for
--expires_in / --ensure_minimum via nscExpiry, falling back to 72h when
the server sends nothing (older server). S3 and local stores ignore it
(S3 lifetime is governed by the bucket lifecycle policy).

Stacks on the A-1775 confirm-restore work.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 api/cache.go                      | 27 +++++++++++++-------
 internal/cache/restore.go         |  6 ++---
 internal/cache/restore_test.go    | 21 ++++++++++------
 internal/cache/save.go            |  2 +-
 internal/cache/store/blob.go      | 12 ++++++---
 internal/cache/store/file.go      |  2 +-
 internal/cache/store/file_test.go | 14 +++++------
 internal/cache/store/nsc.go       | 41 +++++++++++++++++++------------
 internal/cache/store/nsc_test.go  | 41 ++++++++++++++++++++++++-------
 internal/cache/store/s3.go        | 12 ++++++---
 internal/cache/store/s3_test.go   |  2 +-
 11 files changed, 118 insertions(+), 62 deletions(-)

diff --git a/api/cache.go b/api/cache.go
index 1288de74e1..d90798b6d9 100644
--- a/api/cache.go
+++ b/api/cache.go
@@ -69,6 +69,10 @@ type CacheEntryCreateResp struct {
 	Multipart          bool     `json:"multipart"`
 	UploadInstructions []string `json:"upload_instructions"`
 	Message            string   `json:"message"`
+	// RetentionSeconds is how long the backing blob should be kept, matching the
+	// registry entry's TTL. Zero when the server does not send it (older server),
+	// in which case the store falls back to its own default.
+	RetentionSeconds int64 `json:"retention_seconds"`
 }
 
 // CacheEntryRetrieveReq is the request body for retrieving a cache entry.
@@ -79,15 +83,20 @@ type CacheEntryRetrieveReq struct {
 
 // CacheEntryRetrieveResp describes the cache entry to download.
 type CacheEntryRetrieveResp struct {
-	TargetPaths          []string       `json:"target_paths"`
-	CacheKey             []CacheKeyPart `json:"cache_key"`
-	Blobs                []CacheBlob    `json:"blobs"`
-	ExpiresAt            time.Time      `json:"expires_at"`
-	Store                string         `json:"store"`
-	Fallback             bool           `json:"fallback"`
-	Multipart            bool           `json:"multipart"`
-	DownloadInstructions []string       `json:"download_instructions"`
-	Message              string         `json:"message"`
+	TargetPaths []string       `json:"target_paths"`
+	CacheKey    []CacheKeyPart `json:"cache_key"`
+	Blobs       []CacheBlob    `json:"blobs"`
+	ExpiresAt   time.Time      `json:"expires_at"`
+	// RetentionSeconds is how far to push the backing blob's expiry on an
+	// exact-hit refresh, matching the registry entry's TTL. Zero when the server
+	// does not send it (older server), in which case the store falls back to its
+	// own default.
+	RetentionSeconds     int64    `json:"retention_seconds"`
+	Store                string   `json:"store"`
+	Fallback             bool     `json:"fallback"`
+	Multipart            bool     `json:"multipart"`
+	DownloadInstructions []string `json:"download_instructions"`
+	Message              string   `json:"message"`
 	// Scopes are the resolved entry's own scope labels (nil when unscoped),
 	// e.g. {"branch": "main"}. Echo these back on CacheEntryExpireReq to
 	// invalidate this exact entry — its scope may no longer match what the
diff --git a/internal/cache/restore.go b/internal/cache/restore.go
index 9fc10e6f52..927d14861e 100644
--- a/internal/cache/restore.go
+++ b/internal/cache/restore.go
@@ -537,7 +537,7 @@ func (c *client) downloadCache(ctx context.Context, retrieveResp api.CacheEntryR
 	}
 
 	// Extend the blob's retention now that it's confirmed good.
-	maybeRefreshRetention(ctx, blobStore, retrieveResp.Fallback, storeObjectName)
+	maybeRefreshRetention(ctx, blobStore, retrieveResp.Fallback, storeObjectName, time.Duration(retrieveResp.RetentionSeconds)*time.Second)
 
 	span.SetStatus(codes.Ok, "download completed")
 
@@ -549,12 +549,12 @@ func (c *client) downloadCache(ctx context.Context, retrieveResp api.CacheEntryR
 // shorter key sequence, and refreshing it would reset the clock on a blob
 // the caller didn't explicitly target (mirrors the backend's
 // `unless entry_result.fallback_used?` TTL-bump guard).
-func maybeRefreshRetention(ctx context.Context, blobStore store.Blob, fallback bool, key string) {
+func maybeRefreshRetention(ctx context.Context, blobStore store.Blob, fallback bool, key string, retention time.Duration) {
 	refresher, ok := blobStore.(store.RetentionRefresher)
 	if !ok || fallback {
 		return
 	}
-	refresher.RefreshRetention(ctx, key)
+	refresher.RefreshRetention(ctx, key, retention)
 }
 
 // extractCache extracts files from a cache archive
diff --git a/internal/cache/restore_test.go b/internal/cache/restore_test.go
index 8090e5cfba..336c2e21fc 100644
--- a/internal/cache/restore_test.go
+++ b/internal/cache/restore_test.go
@@ -448,10 +448,11 @@ func TestCleanPathWindowsUNCShareRoot(t *testing.T) {
 // fakeRefreshingBlob is a minimal store.Blob that also implements
 // store.RetentionRefresher, mirroring NscStore/S3Blob.
 type fakeRefreshingBlob struct {
-	refreshCalls []string
+	refreshCalls      []string
+	refreshRetentions []time.Duration
 }
 
-func (f *fakeRefreshingBlob) Upload(_ context.Context, _, _ string) (*store.TransferInfo, error) {
+func (f *fakeRefreshingBlob) Upload(_ context.Context, _, _ string, _ time.Duration) (*store.TransferInfo, error) {
 	return nil, nil
 }
 
@@ -459,15 +460,16 @@ func (f *fakeRefreshingBlob) Download(_ context.Context, _, _ string) (*store.Tr
 	return nil, nil
 }
 
-func (f *fakeRefreshingBlob) RefreshRetention(_ context.Context, key string) {
+func (f *fakeRefreshingBlob) RefreshRetention(_ context.Context, key string, retention time.Duration) {
 	f.refreshCalls = append(f.refreshCalls, key)
+	f.refreshRetentions = append(f.refreshRetentions, retention)
 }
 
 // fakeNonRefreshingBlob is a store.Blob with no RefreshRetention method,
 // mirroring LocalFileBlob (no retention concept to refresh).
 type fakeNonRefreshingBlob struct{}
 
-func (f *fakeNonRefreshingBlob) Upload(_ context.Context, _, _ string) (*store.TransferInfo, error) {
+func (f *fakeNonRefreshingBlob) Upload(_ context.Context, _, _ string, _ time.Duration) (*store.TransferInfo, error) {
 	return nil, nil
 }
 
@@ -476,14 +478,17 @@ func (f *fakeNonRefreshingBlob) Download(_ context.Context, _, _ string) (*store
 }
 
 func TestMaybeRefreshRetention(t *testing.T) {
-	t.Run("refreshes on an exact match", func(t *testing.T) {
+	t.Run("refreshes on an exact match with the given retention", func(t *testing.T) {
 		blob := &fakeRefreshingBlob{}
 
-		maybeRefreshRetention(t.Context(), blob, false, "key")
+		maybeRefreshRetention(t.Context(), blob, false, "key", 48*time.Hour)
 
 		if len(blob.refreshCalls) != 1 {
 			t.Errorf("refresh calls = %d, want 1", len(blob.refreshCalls))
 		}
+		if len(blob.refreshRetentions) != 1 || blob.refreshRetentions[0] != 48*time.Hour {
+			t.Errorf("refresh retentions = %v, want [48h]", blob.refreshRetentions)
+		}
 	})
 
 	// Mirrors the backend's `unless entry_result.fallback_used?` TTL-bump
@@ -491,7 +496,7 @@ func TestMaybeRefreshRetention(t *testing.T) {
 	t.Run("skips the refresh on a fallback match", func(t *testing.T) {
 		blob := &fakeRefreshingBlob{}
 
-		maybeRefreshRetention(t.Context(), blob, true, "key")
+		maybeRefreshRetention(t.Context(), blob, true, "key", 48*time.Hour)
 
 		if len(blob.refreshCalls) != 0 {
 			t.Errorf("refresh calls = %d, want 0 for a fallback match", len(blob.refreshCalls))
@@ -501,6 +506,6 @@ func TestMaybeRefreshRetention(t *testing.T) {
 	t.Run("does nothing for a store with no retention concept", func(t *testing.T) {
 		blob := &fakeNonRefreshingBlob{}
 
-		maybeRefreshRetention(t.Context(), blob, false, "key") // must not panic
+		maybeRefreshRetention(t.Context(), blob, false, "key", 48*time.Hour) // must not panic
 	})
 }
diff --git a/internal/cache/save.go b/internal/cache/save.go
index 8409989e90..b78fcc4be7 100644
--- a/internal/cache/save.go
+++ b/internal/cache/save.go
@@ -284,7 +284,7 @@ func (c *client) Save(ctx context.Context, cacheID string) (SaveResult, error) {
 		return result, fmt.Errorf("failed to create blob store: %w", err)
 	}
 
-	transferInfo, err := blobStore.Upload(ctx, archiveInfo.ArchivePath, storeObjectName)
+	transferInfo, err := blobStore.Upload(ctx, archiveInfo.ArchivePath, storeObjectName, time.Duration(createResp.RetentionSeconds)*time.Second)
 	if err != nil {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, "failed to upload cache")
diff --git a/internal/cache/store/blob.go b/internal/cache/store/blob.go
index 24cbed8100..5e108c2724 100644
--- a/internal/cache/store/blob.go
+++ b/internal/cache/store/blob.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"time"
 )
 
 // ErrBlobNotFound is returned by a Blob's Download when the requested object
@@ -13,8 +14,10 @@ var ErrBlobNotFound = errors.New("blob not found")
 
 // Blob interface defines the operations for blob storage
 type Blob interface {
-	// Upload uploads a file to blob storage
-	Upload(ctx context.Context, filePath, key string) (*TransferInfo, error)
+	// Upload uploads a file to blob storage. retention is how long the blob
+	// should be kept; a store that has no retention concept (or a zero/negative
+	// value, meaning the server didn't specify one) uses its own default.
+	Upload(ctx context.Context, filePath, key string, retention time.Duration) (*TransferInfo, error)
 
 	// Download downloads a file from blob storage
 	Download(ctx context.Context, key, destPath string) (*TransferInfo, error)
@@ -23,9 +26,10 @@ type Blob interface {
 // RetentionRefresher is implemented by Blob stores that support extending a
 // blob's effective retention/TTL on access (NscStore, S3Blob). Optional
 // because not every store has a retention concept to refresh (LocalFileBlob
-// does not implement it).
+// does not implement it). retention is the minimum lifetime to guarantee from
+// now; a zero/negative value falls back to the store's own default.
 type RetentionRefresher interface {
-	RefreshRetention(ctx context.Context, key string)
+	RefreshRetention(ctx context.Context, key string, retention time.Duration)
 }
 
 func NewBlobStore(ctx context.Context, store, bucketURL string) (Blob, error) {
diff --git a/internal/cache/store/file.go b/internal/cache/store/file.go
index 6bcc085ec2..28211ecb7e 100644
--- a/internal/cache/store/file.go
+++ b/internal/cache/store/file.go
@@ -127,7 +127,7 @@ func NewLocalFileBlob(ctx context.Context, fileURL string) (*LocalFileBlob, erro
 // semantics for concurrent uploads to the same key.
 //
 // Returns TransferInfo with bytes transferred, transfer speed, and duration.
-func (b *LocalFileBlob) Upload(ctx context.Context, srcPath, key string) (*TransferInfo, error) {
+func (b *LocalFileBlob) Upload(ctx context.Context, srcPath, key string, _ time.Duration) (*TransferInfo, error) {
 	_, span := trace.Start(ctx, "LocalFileBlob.Upload")
 	defer span.End()
 
diff --git a/internal/cache/store/file_test.go b/internal/cache/store/file_test.go
index fc79588c67..64114ce79f 100644
--- a/internal/cache/store/file_test.go
+++ b/internal/cache/store/file_test.go
@@ -169,7 +169,7 @@ func TestLocalFileBlobUpload(t *testing.T) {
 
 	key := "test/cache/artifact.txt"
 
-	info, err := blob.Upload(ctx, srcFile, key)
+	info, err := blob.Upload(ctx, srcFile, key, 0)
 	if err != nil {
 		t.Fatalf("Upload: %v", err)
 	}
@@ -231,7 +231,7 @@ func TestLocalFileBlobDownload(t *testing.T) {
 	key := "test/cache/artifact.txt"
 
 	// Upload first
-	_, err = blob.Upload(ctx, srcFile, key)
+	_, err = blob.Upload(ctx, srcFile, key, 0)
 	if err != nil {
 		t.Fatalf("Upload: %v", err)
 	}
@@ -285,7 +285,7 @@ func TestLocalFileBlobUploadOverwrite(t *testing.T) {
 	if err := os.WriteFile(srcFile1, content1, 0o600); err != nil {
 		t.Fatalf("WriteFile: %v", err)
 	}
-	_, err = blob.Upload(ctx, srcFile1, key)
+	_, err = blob.Upload(ctx, srcFile1, key, 0)
 	if err != nil {
 		t.Fatalf("Upload: %v", err)
 	}
@@ -297,7 +297,7 @@ func TestLocalFileBlobUploadOverwrite(t *testing.T) {
 		t.Fatalf("WriteFile: %v", err)
 	}
 
-	info, err := blob.Upload(ctx, srcFile2, key)
+	info, err := blob.Upload(ctx, srcFile2, key, 0)
 	if err != nil {
 		t.Fatalf("Upload: %v", err)
 	}
@@ -363,7 +363,7 @@ func TestLocalFileBlobUploadInvalidKey(t *testing.T) {
 		t.Fatalf("WriteFile: %v", err)
 	}
 
-	_, err = blob.Upload(ctx, srcFile, "../../../etc/passwd")
+	_, err = blob.Upload(ctx, srcFile, "../../../etc/passwd", 0)
 	if err == nil {
 		t.Fatal("expected error, got nil")
 	}
@@ -518,12 +518,12 @@ func TestLocalFileBlobConcurrentUpload(t *testing.T) {
 	done := make(chan error, 2)
 
 	go func() {
-		_, err := blob.Upload(ctx, srcFile1, key)
+		_, err := blob.Upload(ctx, srcFile1, key, 0)
 		done <- err
 	}()
 
 	go func() {
-		_, err := blob.Upload(ctx, srcFile2, key)
+		_, err := blob.Upload(ctx, srcFile2, key, 0)
 		done <- err
 	}()
 
diff --git a/internal/cache/store/nsc.go b/internal/cache/store/nsc.go
index 7524a56495..d38f148926 100644
--- a/internal/cache/store/nsc.go
+++ b/internal/cache/store/nsc.go
@@ -21,16 +21,25 @@ import (
 // nscScheme is the URL scheme that routes an agent-managed cache store to NSC.
 const nscScheme = "nsc"
 
-// nscDefaultExpiry is the artifact lifetime used both when uploading a cache
-// entry (--expires_in) and when refreshing it on access (--ensure_minimum).
-// Keep this aligned with CacheRegistry::Entry::DEFAULT_EXPIRES in the Buildkite
-// backend.
+// nscDefaultExpiry is the fallback artifact lifetime used for --expires_in
+// (upload) and --ensure_minimum (refresh on access) when the server does not
+// supply a retention (older server). Normally the retention comes from the
+// cache registry's configured TTL, sent per request; see nscExpiry.
 // Cache entries are content-addressed and short-lived, so we cap storage growth
-// rather than relying on NSC's no-expiry default. Every restore pushes the
-// expiry back out to this duration from now, keeping hot caches alive while
-// letting cold ones expire.
+// rather than relying on NSC's no-expiry default.
 const nscDefaultExpiry = "72h"
 
+// nscExpiry formats a retention duration for nsc's --expires_in / --ensure_minimum
+// flags, rounding up to whole hours. A zero or negative duration (the server did
+// not specify one) falls back to nscDefaultExpiry.
+func nscExpiry(retention time.Duration) string {
+	if retention <= 0 {
+		return nscDefaultExpiry
+	}
+	hours := int64((retention + time.Hour - 1) / time.Hour)
+	return fmt.Sprintf("%dh", hours)
+}
+
 // commandRunner executes an external command. It is a seam so tests can assert
 // the arguments passed to the nsc CLI without invoking the real binary.
 type commandRunner func(ctx context.Context, workingDir string, args ...string) (*CommandResult, error)
@@ -135,7 +144,7 @@ func validateKey(key string) error {
 	return nil
 }
 
-func (n *NscStore) Upload(ctx context.Context, filePath, key string) (*TransferInfo, error) {
+func (n *NscStore) Upload(ctx context.Context, filePath, key string, retention time.Duration) (*TransferInfo, error) {
 	_, span := trace.Start(ctx, "NscStore.Upload")
 	defer span.End()
 
@@ -150,7 +159,7 @@ func (n *NscStore) Upload(ctx context.Context, filePath, key string) (*TransferI
 	start := time.Now()
 
 	// Execute nsc artifact upload command
-	result, err := n.run(ctx, "", n.artifactArgs("upload", filePath, key, "--expires_in", nscDefaultExpiry)...)
+	result, err := n.run(ctx, "", n.artifactArgs("upload", filePath, key, "--expires_in", nscExpiry(retention))...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to execute nsc upload command: %w", err)
 	}
@@ -238,17 +247,17 @@ func (n *NscStore) Download(ctx context.Context, key, filePath string) (*Transfe
 	}, nil
 }
 
-// RefreshRetention pushes the artifact's expiry out to at least
-// nscDefaultExpiry from now via `nsc artifact extend --ensure_minimum`. Using
-// --ensure_minimum (rather than the additive --by) makes the refresh
-// idempotent, so calling it on every restore keeps a hot cache alive without
-// growing its expiry unbounded.
+// RefreshRetention pushes the artifact's expiry out to at least retention from
+// now (falling back to nscDefaultExpiry when unset) via `nsc artifact extend
+// --ensure_minimum`. Using --ensure_minimum (rather than the additive --by)
+// makes the refresh idempotent, so calling it on every restore keeps a hot
+// cache alive without growing its expiry unbounded.
 //
 // This is best-effort: any failure is logged and swallowed so a restore never
 // fails because its TTL could not be refreshed. Whether to call this at all
 // (e.g. skipping it on a fallback match) is the restore flow's decision, not
 // this store's — see store.RetentionRefresher.
-func (n *NscStore) RefreshRetention(ctx context.Context, key string) {
+func (n *NscStore) RefreshRetention(ctx context.Context, key string, retention time.Duration) {
 	if !n.extendSupported(ctx) {
 		// `nsc artifact extend` is still being rolled out by Namespace. Update
 		// the nsc CLI as a contingency so the command becomes available.
@@ -260,7 +269,7 @@ func (n *NscStore) RefreshRetention(ctx context.Context, key string) {
 		}
 	}
 
-	result, err := n.run(ctx, "", n.artifactArgs("extend", key, "--ensure_minimum", nscDefaultExpiry)...)
+	result, err := n.run(ctx, "", n.artifactArgs("extend", key, "--ensure_minimum", nscExpiry(retention))...)
 	switch {
 	case err != nil:
 		slog.Warn("failed to refresh cache TTL, continuing (non-fatal)", "key", key, "error", err)
diff --git a/internal/cache/store/nsc_test.go b/internal/cache/store/nsc_test.go
index 368bb3a53c..f82b625fb1 100644
--- a/internal/cache/store/nsc_test.go
+++ b/internal/cache/store/nsc_test.go
@@ -7,10 +7,33 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 )
 
+func TestNscExpiry(t *testing.T) {
+	tests := []struct {
+		name      string
+		retention time.Duration
+		want      string
+	}{
+		{name: "zero falls back to default", retention: 0, want: nscDefaultExpiry},
+		{name: "negative falls back to default", retention: -time.Hour, want: nscDefaultExpiry},
+		{name: "whole hours", retention: 72 * time.Hour, want: "72h"},
+		{name: "multi-day", retention: 7 * 24 * time.Hour, want: "168h"},
+		{name: "rounds partial hours up", retention: 90 * time.Minute, want: "2h"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := nscExpiry(tt.retention); got != tt.want {
+				t.Errorf("nscExpiry(%v) = %q, want %q", tt.retention, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestNscStore_Interface(t *testing.T) {
 	// This test ensures that NscStore properly implements the Blob interface
 	var _ Blob = (*NscStore)(nil)
@@ -261,11 +284,11 @@ func TestNscStore_PassesNamespace(t *testing.T) {
 	var captured []string
 	store := &NscStore{namespace: "my-namespace", run: fakeRunner(&captured)}
 
-	if _, err := store.Upload(ctx, testFile, "key"); err != nil {
+	if _, err := store.Upload(ctx, testFile, "key", 7*24*time.Hour); err != nil {
 		t.Fatalf("Upload: %v", err)
 	}
 
-	wantArgs := []string{"nsc", "artifact", "upload", testFile, "key", "--expires_in", "72h", "--namespace", "my-namespace"}
+	wantArgs := []string{"nsc", "artifact", "upload", testFile, "key", "--expires_in", "168h", "--namespace", "my-namespace"}
 	if diff := cmp.Diff(wantArgs, captured); diff != "" {
 		t.Errorf("upload args mismatch (-want +got):\n%s", diff)
 	}
@@ -333,9 +356,9 @@ func TestNscStore_RefreshRetention(t *testing.T) {
 	var calls [][]string
 	store := &NscStore{namespace: "my-namespace", run: recordingRunner(&calls, nil)}
 
-	store.RefreshRetention(ctx, "key")
+	store.RefreshRetention(ctx, "key", 7*24*time.Hour)
 
-	wantExtend := []string{"nsc", "artifact", "extend", "key", "--ensure_minimum", "72h", "--namespace", "my-namespace"}
+	wantExtend := []string{"nsc", "artifact", "extend", "key", "--ensure_minimum", "168h", "--namespace", "my-namespace"}
 	var gotExtend []string
 	for _, c := range calls {
 		if isCommand(c, "nsc", "artifact", "extend") && !isCommand(c, "nsc", "artifact", "extend", "--help") {
@@ -360,7 +383,7 @@ func TestNscStore_RefreshRetention_UpdatesCLIWhenExtendUnsupported(t *testing.T)
 	}
 	store := &NscStore{namespace: "ns", run: recordingRunner(&calls, respond)}
 
-	store.RefreshRetention(ctx, "key")
+	store.RefreshRetention(ctx, "key", 0)
 
 	var updated, extended bool
 	for _, c := range calls {
@@ -398,7 +421,7 @@ func TestNscStore_RefreshRetention_UpdatesCLIWhenExtendHelpLacksEnsureMinimum(t
 	}
 	store := &NscStore{namespace: "ns", run: recordingRunner(&calls, respond)}
 
-	store.RefreshRetention(ctx, "key")
+	store.RefreshRetention(ctx, "key", 0)
 
 	var updated, extended bool
 	for _, c := range calls {
@@ -432,7 +455,7 @@ func TestNscStore_RefreshRetention_SwallowsFailure(t *testing.T) {
 	var calls [][]string
 	store := &NscStore{namespace: "ns", run: recordingRunner(&calls, respond)}
 
-	store.RefreshRetention(ctx, "key") // must not panic
+	store.RefreshRetention(ctx, "key", 0) // must not panic
 }
 
 func TestNewNscStore_RequiresNamespace(t *testing.T) {
@@ -451,7 +474,7 @@ func TestNscStore_ValidationShortCircuits(t *testing.T) {
 		return &CommandResult{}, nil
 	}}
 
-	if _, err := store.Upload(ctx, "invalid;path", "valid-key"); err == nil {
+	if _, err := store.Upload(ctx, "invalid;path", "valid-key", 0); err == nil {
 		t.Error("Upload with unsafe path: expected error, got nil")
 	}
 	if _, err := store.Download(ctx, "invalid key with spaces", "dest.txt"); err == nil {
@@ -509,7 +532,7 @@ func TestNscStore_Integration(t *testing.T) {
 
 	// Test upload
 	key := "integration-test/test-file.txt"
-	transferInfo, err := store.Upload(ctx, testFile, key)
+	transferInfo, err := store.Upload(ctx, testFile, key, 0)
 	if err != nil {
 		t.Fatalf("Upload should succeed with valid NSC setup: %v", err)
 	}
diff --git a/internal/cache/store/s3.go b/internal/cache/store/s3.go
index 19d8196716..29e26162e9 100644
--- a/internal/cache/store/s3.go
+++ b/internal/cache/store/s3.go
@@ -300,8 +300,10 @@ func resolveTransferSettings(opts *Options) transferSettings {
 	}
 }
 
-// Upload uploads a file to S3 using multipart upload for parallel transfers
-func (b *S3Blob) Upload(ctx context.Context, filePath, key string) (*TransferInfo, error) {
+// Upload uploads a file to S3 using multipart upload for parallel transfers.
+// retention is ignored: S3 object lifetime is governed by the bucket's
+// lifecycle policy, which the agent does not set per object.
+func (b *S3Blob) Upload(ctx context.Context, filePath, key string, _ time.Duration) (*TransferInfo, error) {
 	ctx, span := trace.Start(ctx, "S3Blob.Upload")
 	defer span.End()
 
@@ -487,7 +489,11 @@ func (b *S3Blob) Download(ctx context.Context, key, destPath string) (*TransferI
 //
 // This refresh is best-effort only: any failure (incl. objects exceeding
 // S3's 5GB CopyObject limit) must not cause the overall restore operation to fail.
-func (b *S3Blob) RefreshRetention(ctx context.Context, key string) {
+//
+// retention is ignored: the self-copy only refreshes LastModified, and the
+// effective lifetime is whatever window the bucket's lifecycle policy applies
+// from that timestamp — the agent cannot lengthen it per object.
+func (b *S3Blob) RefreshRetention(ctx context.Context, key string, _ time.Duration) {
 	fullKey := b.getFullKey(key)
 	refreshObjectExpiry(ctx, b.client, b.bucketName, fullKey)
 }
diff --git a/internal/cache/store/s3_test.go b/internal/cache/store/s3_test.go
index bdd66f159c..cbe1728f82 100644
--- a/internal/cache/store/s3_test.go
+++ b/internal/cache/store/s3_test.go
@@ -639,7 +639,7 @@ func TestS3Blob_RefreshRetention(t *testing.T) {
 	copier := &fakeCopier{}
 	b := &S3Blob{client: copier, bucketName: "my-bucket", prefix: "prefix"}
 
-	b.RefreshRetention(t.Context(), "key")
+	b.RefreshRetention(t.Context(), "key", 0)
 
 	if len(copier.calls) != 1 {
 		t.Fatalf("CopyObject calls = %d, want 1", len(copier.calls))

From 059bcf1bd7bc31318216e627c079176142ba1a00 Mon Sep 17 00:00:00 2001
From: Sneha <sneha@buildkite.com>
Date: Mon, 7 Sep 2026 15:09:39 +1000
Subject: [PATCH 2/3] Rename nscDefaultExpiry/nscExpiry to
 nscDefaultRetention/nscRetentionArg
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The constant holds a retention duration (a fallback used only when the
server omits one), and the helper formats a retention for the nsc CLI —
'expiry' misnamed both. Keep the nsc prefix: these are NSC-specific (the
72h default and hours formatting are nsc artifact conventions; S3/local
ignore retention).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 internal/cache/store/nsc.go      | 20 ++++++++++----------
 internal/cache/store/nsc_test.go | 10 +++++-----
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/internal/cache/store/nsc.go b/internal/cache/store/nsc.go
index d38f148926..66f6d980bd 100644
--- a/internal/cache/store/nsc.go
+++ b/internal/cache/store/nsc.go
@@ -21,20 +21,20 @@ import (
 // nscScheme is the URL scheme that routes an agent-managed cache store to NSC.
 const nscScheme = "nsc"
 
-// nscDefaultExpiry is the fallback artifact lifetime used for --expires_in
+// nscDefaultRetention is the fallback artifact lifetime used for --expires_in
 // (upload) and --ensure_minimum (refresh on access) when the server does not
 // supply a retention (older server). Normally the retention comes from the
-// cache registry's configured TTL, sent per request; see nscExpiry.
+// cache registry's configured TTL, sent per request; see nscRetentionArg.
 // Cache entries are content-addressed and short-lived, so we cap storage growth
 // rather than relying on NSC's no-expiry default.
-const nscDefaultExpiry = "72h"
+const nscDefaultRetention = "72h"
 
-// nscExpiry formats a retention duration for nsc's --expires_in / --ensure_minimum
+// nscRetentionArg formats a retention duration for nsc's --expires_in / --ensure_minimum
 // flags, rounding up to whole hours. A zero or negative duration (the server did
-// not specify one) falls back to nscDefaultExpiry.
-func nscExpiry(retention time.Duration) string {
+// not specify one) falls back to nscDefaultRetention.
+func nscRetentionArg(retention time.Duration) string {
 	if retention <= 0 {
-		return nscDefaultExpiry
+		return nscDefaultRetention
 	}
 	hours := int64((retention + time.Hour - 1) / time.Hour)
 	return fmt.Sprintf("%dh", hours)
@@ -159,7 +159,7 @@ func (n *NscStore) Upload(ctx context.Context, filePath, key string, retention t
 	start := time.Now()
 
 	// Execute nsc artifact upload command
-	result, err := n.run(ctx, "", n.artifactArgs("upload", filePath, key, "--expires_in", nscExpiry(retention))...)
+	result, err := n.run(ctx, "", n.artifactArgs("upload", filePath, key, "--expires_in", nscRetentionArg(retention))...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to execute nsc upload command: %w", err)
 	}
@@ -248,7 +248,7 @@ func (n *NscStore) Download(ctx context.Context, key, filePath string) (*Transfe
 }
 
 // RefreshRetention pushes the artifact's expiry out to at least retention from
-// now (falling back to nscDefaultExpiry when unset) via `nsc artifact extend
+// now (falling back to nscDefaultRetention when unset) via `nsc artifact extend
 // --ensure_minimum`. Using --ensure_minimum (rather than the additive --by)
 // makes the refresh idempotent, so calling it on every restore keeps a hot
 // cache alive without growing its expiry unbounded.
@@ -269,7 +269,7 @@ func (n *NscStore) RefreshRetention(ctx context.Context, key string, retention t
 		}
 	}
 
-	result, err := n.run(ctx, "", n.artifactArgs("extend", key, "--ensure_minimum", nscExpiry(retention))...)
+	result, err := n.run(ctx, "", n.artifactArgs("extend", key, "--ensure_minimum", nscRetentionArg(retention))...)
 	switch {
 	case err != nil:
 		slog.Warn("failed to refresh cache TTL, continuing (non-fatal)", "key", key, "error", err)
diff --git a/internal/cache/store/nsc_test.go b/internal/cache/store/nsc_test.go
index f82b625fb1..c595284351 100644
--- a/internal/cache/store/nsc_test.go
+++ b/internal/cache/store/nsc_test.go
@@ -12,14 +12,14 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
-func TestNscExpiry(t *testing.T) {
+func TestNscRetentionArg(t *testing.T) {
 	tests := []struct {
 		name      string
 		retention time.Duration
 		want      string
 	}{
-		{name: "zero falls back to default", retention: 0, want: nscDefaultExpiry},
-		{name: "negative falls back to default", retention: -time.Hour, want: nscDefaultExpiry},
+		{name: "zero falls back to default", retention: 0, want: nscDefaultRetention},
+		{name: "negative falls back to default", retention: -time.Hour, want: nscDefaultRetention},
 		{name: "whole hours", retention: 72 * time.Hour, want: "72h"},
 		{name: "multi-day", retention: 7 * 24 * time.Hour, want: "168h"},
 		{name: "rounds partial hours up", retention: 90 * time.Minute, want: "2h"},
@@ -27,8 +27,8 @@ func TestNscExpiry(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := nscExpiry(tt.retention); got != tt.want {
-				t.Errorf("nscExpiry(%v) = %q, want %q", tt.retention, got, tt.want)
+			if got := nscRetentionArg(tt.retention); got != tt.want {
+				t.Errorf("nscRetentionArg(%v) = %q, want %q", tt.retention, got, tt.want)
 			}
 		})
 	}

From b43b2c7c9a5c7e31185a0e1a9a8534f98998a503 Mon Sep 17 00:00:00 2001
From: Sneha <sneha@buildkite.com>
Date: Tue, 8 Sep 2026 11:27:01 +1000
Subject: [PATCH 3/3] Consume cache retention as days, not seconds

Match the server switching retention_seconds -> retention_days: the
field is now RetentionDays and the store conversions use
time.Duration(days) * 24 * time.Hour.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 api/cache.go                 | 31 +++++++++++++------------------
 internal/cache/restore.go    |  2 +-
 internal/cache/save.go       |  2 +-
 internal/cache/store/blob.go |  6 ++----
 internal/cache/store/nsc.go  | 13 ++++---------
 5 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/api/cache.go b/api/cache.go
index d90798b6d9..ef818ea759 100644
--- a/api/cache.go
+++ b/api/cache.go
@@ -69,10 +69,9 @@ type CacheEntryCreateResp struct {
 	Multipart          bool     `json:"multipart"`
 	UploadInstructions []string `json:"upload_instructions"`
 	Message            string   `json:"message"`
-	// RetentionSeconds is how long the backing blob should be kept, matching the
-	// registry entry's TTL. Zero when the server does not send it (older server),
-	// in which case the store falls back to its own default.
-	RetentionSeconds int64 `json:"retention_seconds"`
+	// RetentionDays is how long the backing blob should be kept, matching the
+	// registry entry's TTL.
+	RetentionDays int64 `json:"retention_days"`
 }
 
 // CacheEntryRetrieveReq is the request body for retrieving a cache entry.
@@ -83,20 +82,16 @@ type CacheEntryRetrieveReq struct {
 
 // CacheEntryRetrieveResp describes the cache entry to download.
 type CacheEntryRetrieveResp struct {
-	TargetPaths []string       `json:"target_paths"`
-	CacheKey    []CacheKeyPart `json:"cache_key"`
-	Blobs       []CacheBlob    `json:"blobs"`
-	ExpiresAt   time.Time      `json:"expires_at"`
-	// RetentionSeconds is how far to push the backing blob's expiry on an
-	// exact-hit refresh, matching the registry entry's TTL. Zero when the server
-	// does not send it (older server), in which case the store falls back to its
-	// own default.
-	RetentionSeconds     int64    `json:"retention_seconds"`
-	Store                string   `json:"store"`
-	Fallback             bool     `json:"fallback"`
-	Multipart            bool     `json:"multipart"`
-	DownloadInstructions []string `json:"download_instructions"`
-	Message              string   `json:"message"`
+	TargetPaths          []string       `json:"target_paths"`
+	CacheKey             []CacheKeyPart `json:"cache_key"`
+	Blobs                []CacheBlob    `json:"blobs"`
+	ExpiresAt            time.Time      `json:"expires_at"`
+	RetentionDays        int64          `json:"retention_days"`
+	Store                string         `json:"store"`
+	Fallback             bool           `json:"fallback"`
+	Multipart            bool           `json:"multipart"`
+	DownloadInstructions []string       `json:"download_instructions"`
+	Message              string         `json:"message"`
 	// Scopes are the resolved entry's own scope labels (nil when unscoped),
 	// e.g. {"branch": "main"}. Echo these back on CacheEntryExpireReq to
 	// invalidate this exact entry — its scope may no longer match what the
diff --git a/internal/cache/restore.go b/internal/cache/restore.go
index 927d14861e..d9e9f2e430 100644
--- a/internal/cache/restore.go
+++ b/internal/cache/restore.go
@@ -537,7 +537,7 @@ func (c *client) downloadCache(ctx context.Context, retrieveResp api.CacheEntryR
 	}
 
 	// Extend the blob's retention now that it's confirmed good.
-	maybeRefreshRetention(ctx, blobStore, retrieveResp.Fallback, storeObjectName, time.Duration(retrieveResp.RetentionSeconds)*time.Second)
+	maybeRefreshRetention(ctx, blobStore, retrieveResp.Fallback, storeObjectName, time.Duration(retrieveResp.RetentionDays)*24*time.Hour)
 
 	span.SetStatus(codes.Ok, "download completed")
 
diff --git a/internal/cache/save.go b/internal/cache/save.go
index b78fcc4be7..ef244897de 100644
--- a/internal/cache/save.go
+++ b/internal/cache/save.go
@@ -284,7 +284,7 @@ func (c *client) Save(ctx context.Context, cacheID string) (SaveResult, error) {
 		return result, fmt.Errorf("failed to create blob store: %w", err)
 	}
 
-	transferInfo, err := blobStore.Upload(ctx, archiveInfo.ArchivePath, storeObjectName, time.Duration(createResp.RetentionSeconds)*time.Second)
+	transferInfo, err := blobStore.Upload(ctx, archiveInfo.ArchivePath, storeObjectName, time.Duration(createResp.RetentionDays)*24*time.Hour)
 	if err != nil {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, "failed to upload cache")
diff --git a/internal/cache/store/blob.go b/internal/cache/store/blob.go
index 5e108c2724..6b455c434b 100644
--- a/internal/cache/store/blob.go
+++ b/internal/cache/store/blob.go
@@ -15,8 +15,7 @@ var ErrBlobNotFound = errors.New("blob not found")
 // Blob interface defines the operations for blob storage
 type Blob interface {
 	// Upload uploads a file to blob storage. retention is how long the blob
-	// should be kept; a store that has no retention concept (or a zero/negative
-	// value, meaning the server didn't specify one) uses its own default.
+	// should be kept.
 	Upload(ctx context.Context, filePath, key string, retention time.Duration) (*TransferInfo, error)
 
 	// Download downloads a file from blob storage
@@ -26,8 +25,7 @@ type Blob interface {
 // RetentionRefresher is implemented by Blob stores that support extending a
 // blob's effective retention/TTL on access (NscStore, S3Blob). Optional
 // because not every store has a retention concept to refresh (LocalFileBlob
-// does not implement it). retention is the minimum lifetime to guarantee from
-// now; a zero/negative value falls back to the store's own default.
+// does not implement it).
 type RetentionRefresher interface {
 	RefreshRetention(ctx context.Context, key string, retention time.Duration)
 }
diff --git a/internal/cache/store/nsc.go b/internal/cache/store/nsc.go
index 66f6d980bd..0debee9ddc 100644
--- a/internal/cache/store/nsc.go
+++ b/internal/cache/store/nsc.go
@@ -23,15 +23,11 @@ const nscScheme = "nsc"
 
 // nscDefaultRetention is the fallback artifact lifetime used for --expires_in
 // (upload) and --ensure_minimum (refresh on access) when the server does not
-// supply a retention (older server). Normally the retention comes from the
-// cache registry's configured TTL, sent per request; see nscRetentionArg.
-// Cache entries are content-addressed and short-lived, so we cap storage growth
-// rather than relying on NSC's no-expiry default.
+// supply a retention (older server).
 const nscDefaultRetention = "72h"
 
 // nscRetentionArg formats a retention duration for nsc's --expires_in / --ensure_minimum
-// flags, rounding up to whole hours. A zero or negative duration (the server did
-// not specify one) falls back to nscDefaultRetention.
+// flags, rounding up to whole hours. A zero or negative duration falls back to nscDefaultRetention.
 func nscRetentionArg(retention time.Duration) string {
 	if retention <= 0 {
 		return nscDefaultRetention
@@ -249,9 +245,8 @@ func (n *NscStore) Download(ctx context.Context, key, filePath string) (*Transfe
 
 // RefreshRetention pushes the artifact's expiry out to at least retention from
 // now (falling back to nscDefaultRetention when unset) via `nsc artifact extend
-// --ensure_minimum`. Using --ensure_minimum (rather than the additive --by)
-// makes the refresh idempotent, so calling it on every restore keeps a hot
-// cache alive without growing its expiry unbounded.
+// --ensure_minimum`. Using --ensure_minimum  makes the refresh idempotent,
+// so calling it on every restore keeps a hot cache alive without growing its expiry unbounded.
 //
 // This is best-effort: any failure is logged and swallowed so a restore never
 // fails because its TTL could not be refreshed. Whether to call this at all