Smarter disk cache: immutable completed GHA resources

pkg/githubapi/cache.go

@@ -5,20 +5,30 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
+	"encoding/json"
+	"io"
 	"net/http"
 	"net/http/httputil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 )
 
-// cacheTTL controls how long cached responses remain valid.
+// cacheTTL is the default freshness window for mutable GitHub responses
+// (lists, in-progress runs, PR metadata, …).
 const cacheTTL = 5 * time.Minute
 
+// cacheTTLImmutable is how long completed GHA resources stay fresh without a
+// network round-trip. Completed runs/jobs/logs do not change; re-fetching them
+// wastes rate budget and wall clock. Cap avoids unbounded disk trust if GitHub
+// ever mutates history (deletes, redactions).
+const cacheTTLImmutable = 30 * 24 * time.Hour
+
 // cacheVersion is incremented to invalidate all existing cached responses.
 // Bump this when response parsing or enrichment logic changes.
-const cacheVersion = "v4"
+const cacheVersion = "v5"
 
 // CachedTransport implements http.RoundTripper and caches GET requests to disk.
 type CachedTransport struct {
@@ -155,10 +165,64 @@ func (t *CachedTransport) readFromCache(path string, req *http.Request) (*http.R
 	// Don't os.Remove stale entries here: another process may be concurrently
 	// renaming a fresh entry into place, and removing would delete it. A stale
 	// file is simply overwritten by the next save (or touched on a 304).
-	fresh := time.Since(info.ModTime()) <= cacheTTL
+	age := time.Since(info.ModTime())
+	fresh := age <= cacheTTL
+	if !fresh && age <= cacheTTLImmutable && isImmutableCachedResponse(req, resp) {
+		// Completed Actions resources are effectively write-once: serve without
+		// revalidation until the immutable window ends (or cacheVersion bumps).
+		fresh = true
+	}
 	return resp, fresh
 }
 
+// Single-run / single-job JSON paths (not list or nested collections).
+var (
+	reActionsRun  = regexp.MustCompile(`^/repos/[^/]+/[^/]+/actions/runs/\d+$`)
+	reActionsJob  = regexp.MustCompile(`^/repos/[^/]+/[^/]+/actions/jobs/\d+$`)
+	reActionsLogs = regexp.MustCompile(`^/repos/[^/]+/[^/]+/actions/jobs/\d+/logs$`)
+)
+
+// isImmutableCachedResponse reports whether this cached GET is safe to treat as
+// long-lived. Only GitHub API completed workflow runs, completed jobs, and job
+// logs qualify. List endpoints and in-progress resources stay on short TTL.
+func isImmutableCachedResponse(req *http.Request, resp *http.Response) bool {
+	if req == nil || resp == nil || resp.Body == nil {
+		return false
+	}
+	host := req.URL.Host
+	if host != "api.github.com" && !strings.HasSuffix(host, ".api.github.com") {
+		return false
+	}
+	path := req.URL.Path
+	switch {
+	case reActionsLogs.MatchString(path):
+		// Logs are only available after completion and do not change.
+		return true
+	case reActionsRun.MatchString(path), reActionsJob.MatchString(path):
+		return bodyStatusCompleted(resp)
+	default:
+		return false
+	}
+}
+
+// bodyStatusCompleted peeks at a JSON body for status=="completed" and restores
+// resp.Body so the caller can still read it.
+func bodyStatusCompleted(resp *http.Response) bool {
+	raw, err := io.ReadAll(resp.Body)
+	_ = resp.Body.Close()
+	resp.Body = io.NopCloser(bytes.NewReader(raw))
+	if err != nil || len(raw) == 0 {
+		return false
+	}
+	var meta struct {
+		Status string `json:"status"`
+	}
+	if err := json.Unmarshal(raw, &meta); err != nil {
+		return false
+	}
+	return meta.Status == "completed"
+}
+
 // touch resets a cache entry's TTL by bumping its modification time to now,
 // used after a 304 confirms the stored body is still current. Best-effort:
 // a concurrent rename or removal racing with this is harmless.

pkg/githubapi/cache_test.go

@@ -215,6 +215,118 @@ func TestCachedTransportRevalidatesWithETag(t *testing.T) {
 	assert.Empty(t, req.Header.Get("If-None-Match"))
 }
 
+// rtFunc is a RoundTripper that never hits the network (unit-test double).
+type rtFunc func(*http.Request) (*http.Response, error)
+
+func (f rtFunc) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) }
+
+func TestCachedTransportImmutableCompletedRunSurvivesTTL(t *testing.T) {
+	cacheDir := t.TempDir()
+	callCount := 0
+	body := `{"id":1,"status":"completed","conclusion":"success","name":"CI"}`
+	base := rtFunc(func(req *http.Request) (*http.Response, error) {
+		callCount++
+		return &http.Response{
+			StatusCode: http.StatusOK,
+			Header:     make(http.Header),
+			Body:       io.NopCloser(strings.NewReader(body)),
+			Request:    req,
+		}, nil
+	})
+	transport := NewCachedTransport(base, cacheDir)
+	client := &http.Client{Transport: transport}
+
+	url := "https://api.github.com/repos/o/r/actions/runs/42"
+	req, _ := http.NewRequest(http.MethodGet, url, nil)
+	resp, err := client.Do(req)
+	assert.NoError(t, err)
+	got, _ := io.ReadAll(resp.Body)
+	resp.Body.Close()
+	assert.Equal(t, body, string(got))
+	assert.Equal(t, 1, callCount)
+
+	files, _ := os.ReadDir(cacheDir)
+	assert.Len(t, files, 1)
+	cachePath := cacheDir + "/" + files[0].Name()
+	// Past short TTL, still within immutable window.
+	stale := time.Now().Add(-(cacheTTL + time.Hour))
+	assert.NoError(t, os.Chtimes(cachePath, stale, stale))
+
+	resp2, err := client.Do(req)
+	assert.NoError(t, err)
+	got2, _ := io.ReadAll(resp2.Body)
+	resp2.Body.Close()
+	assert.Equal(t, body, string(got2))
+	assert.Equal(t, 1, callCount, "completed run must stay cache-fresh past short TTL")
+}
+
+func TestCachedTransportInProgressRunExpires(t *testing.T) {
+	cacheDir := t.TempDir()
+	callCount := 0
+	body := `{"id":1,"status":"in_progress","conclusion":null}`
+	base := rtFunc(func(req *http.Request) (*http.Response, error) {
+		callCount++
+		return &http.Response{
+			StatusCode: http.StatusOK,
+			Header:     make(http.Header),
+			Body:       io.NopCloser(strings.NewReader(body)),
+			Request:    req,
+		}, nil
+	})
+	transport := NewCachedTransport(base, cacheDir)
+	client := &http.Client{Transport: transport}
+
+	url := "https://api.github.com/repos/o/r/actions/runs/99"
+	req, _ := http.NewRequest(http.MethodGet, url, nil)
+	_, err := client.Do(req)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, callCount)
+
+	files, _ := os.ReadDir(cacheDir)
+	cachePath := cacheDir + "/" + files[0].Name()
+	stale := time.Now().Add(-(cacheTTL + time.Minute))
+	assert.NoError(t, os.Chtimes(cachePath, stale, stale))
+
+	_, err = client.Do(req)
+	assert.NoError(t, err)
+	assert.Equal(t, 2, callCount, "in_progress run must revalidate after short TTL")
+}
+
+func TestCachedTransportImmutableJobLogs(t *testing.T) {
+	cacheDir := t.TempDir()
+	callCount := 0
+	logBody := "2026-01-01T00:00:00.0000000Z hello"
+	base := rtFunc(func(req *http.Request) (*http.Response, error) {
+		callCount++
+		return &http.Response{
+			StatusCode: http.StatusOK,
+			Header:     make(http.Header),
+			Body:       io.NopCloser(strings.NewReader(logBody)),
+			Request:    req,
+		}, nil
+	})
+	transport := NewCachedTransport(base, cacheDir)
+	client := &http.Client{Transport: transport}
+
+	url := "https://api.github.com/repos/o/r/actions/jobs/7/logs"
+	req, _ := http.NewRequest(http.MethodGet, url, nil)
+	_, err := client.Do(req)
+	assert.NoError(t, err)
+	assert.Equal(t, 1, callCount)
+
+	files, _ := os.ReadDir(cacheDir)
+	cachePath := cacheDir + "/" + files[0].Name()
+	stale := time.Now().Add(-(cacheTTL + 2*time.Hour))
+	assert.NoError(t, os.Chtimes(cachePath, stale, stale))
+
+	resp, err := client.Do(req)
+	assert.NoError(t, err)
+	got, _ := io.ReadAll(resp.Body)
+	resp.Body.Close()
+	assert.Equal(t, logBody, string(got))
+	assert.Equal(t, 1, callCount, "job logs are immutable once fetched")
+}
+
 func TestCachedTransportConcurrentSameURL(t *testing.T) {
 	cacheDir := t.TempDir()