diff --git a/pkg/githubapi/cache.go b/pkg/githubapi/cache.go index df645d9..0566475 100644 --- a/pkg/githubapi/cache.go +++ b/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. diff --git a/pkg/githubapi/cache_test.go b/pkg/githubapi/cache_test.go index 9af9456..81239f6 100644 --- a/pkg/githubapi/cache_test.go +++ b/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()