Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 169 additions & 0 deletions pkg/agent/github_resolution_cache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package agent

import (
"encoding/json"
"os"
"path/filepath"
"sync"
"time"

"github.com/GoogleCloudPlatform/scion/pkg/config"
"github.com/GoogleCloudPlatform/scion/pkg/util"
)

const (
// DefaultResolutionCacheTTL is how long a cached resolution result is
// considered fresh. GitHub content can change, so this is a balance
// between freshness and avoiding rate limits.
DefaultResolutionCacheTTL = 5 * time.Minute

resolutionCacheFileName = "github-resolution-cache.json"
)

// GitHubResolutionCache caches the mapping from skill URI → ResolvedSkill
// to avoid redundant GitHub API calls during repeated provisioning.
// Entries expire after a configurable TTL.
type GitHubResolutionCache struct {
mu sync.RWMutex
dir string
ttl time.Duration
entries map[string]*resolutionCacheEntry
filePath string
}

type resolutionCacheEntry struct {
Skill ResolvedSkill `json:"skill"`
CachedAt time.Time `json:"cachedAt"`
ExpiresAt time.Time `json:"expiresAt"`
}

type resolutionCacheFile struct {
Entries map[string]*resolutionCacheEntry `json:"entries"`
}

// NewGitHubResolutionCache creates or loads a resolution cache at the
// given directory with the specified TTL.
func NewGitHubResolutionCache(dir string, ttl time.Duration) (*GitHubResolutionCache, error) {
if err := os.MkdirAll(dir, 0755); err != nil {
return nil, err
}
c := &GitHubResolutionCache{
dir: dir,
ttl: ttl,
entries: make(map[string]*resolutionCacheEntry),
filePath: filepath.Join(dir, resolutionCacheFileName),
}
c.load()
return c, nil
}

// Get returns a cached ResolvedSkill for the given URI if it exists
// and has not expired. The returned value is a deep copy safe for
// concurrent use.
func (c *GitHubResolutionCache) Get(uri string) (ResolvedSkill, bool) {
c.mu.RLock()
defer c.mu.RUnlock()

entry, ok := c.entries[uri]
if !ok {
return ResolvedSkill{}, false
}
if time.Now().After(entry.ExpiresAt) {
return ResolvedSkill{}, false
}
skill := entry.Skill
if len(entry.Skill.Files) > 0 {
skill.Files = make([]ResolvedFile, len(entry.Skill.Files))
copy(skill.Files, entry.Skill.Files)
}
return skill, true
}

// Put stores a resolved skill in the cache.
func (c *GitHubResolutionCache) Put(uri string, skill ResolvedSkill) {
c.mu.Lock()
now := time.Now()
c.entries[uri] = &resolutionCacheEntry{
Skill: skill,
CachedAt: now,
ExpiresAt: now.Add(c.ttl),
}
c.evictExpired()
snapshot := make(map[string]*resolutionCacheEntry, len(c.entries))
for k, v := range c.entries {
snapshot[k] = v
}
c.mu.Unlock()

c.save(snapshot)
}
Comment on lines +97 to +113

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Holding the write lock c.mu.Lock() during disk I/O (os.WriteFile) blocks all concurrent readers (Get) and writers (Put). We should copy the entries under the lock, release the lock, and then perform the marshaling and file write operations outside the lock.

func (c *GitHubResolutionCache) Put(uri string, skill ResolvedSkill) {
	c.mu.Lock()
	now := time.Now()
	c.entries[uri] = &resolutionCacheEntry{
		Skill:     skill,
		CachedAt:  now,
		ExpiresAt: now.Add(c.ttl),
	}
	c.evictExpired()

	// Copy entries to save outside the lock to avoid blocking readers with disk I/O.
	entriesCopy := make(map[string]*resolutionCacheEntry, len(c.entries))
	for k, v := range c.entries {
		entriesCopy[k] = v
	}
	c.mu.Unlock()

	c.save(entriesCopy)
}


// load reads the cache from disk. Best-effort: errors are silently ignored.
func (c *GitHubResolutionCache) load() {
data, err := os.ReadFile(c.filePath)
if err != nil {
return
}
var f resolutionCacheFile
if err := json.Unmarshal(data, &f); err != nil {
return
}
if f.Entries == nil {
return
}
now := time.Now()
for uri, entry := range f.Entries {
if now.Before(entry.ExpiresAt) {
c.entries[uri] = entry
}
}
Comment on lines +129 to +133

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the JSON cache file contains a null entry (which can happen if corrupted or manually edited), entry.ExpiresAt will panic with a nil pointer dereference. We should add a nil check before accessing entry.ExpiresAt.

Suggested change
for uri, entry := range f.Entries {
if now.Before(entry.ExpiresAt) {
c.entries[uri] = entry
}
}
for uri, entry := range f.Entries {
if entry != nil && now.Before(entry.ExpiresAt) {
c.entries[uri] = entry
}
}

util.Debugf("github: loaded %d resolution cache entries from disk", len(c.entries))
}

// save persists the given entries snapshot to disk atomically. Best-effort.
func (c *GitHubResolutionCache) save(entries map[string]*resolutionCacheEntry) {
f := resolutionCacheFile{Entries: entries}
data, err := json.MarshalIndent(f, "", " ")
if err != nil {
return
}
tmpPath := c.filePath + ".tmp"
if err := os.WriteFile(tmpPath, data, 0644); err != nil {
return
}
_ = os.Rename(tmpPath, c.filePath)
}

// evictExpired removes expired entries. Must be called with lock held.
func (c *GitHubResolutionCache) evictExpired() {
now := time.Now()
for uri, entry := range c.entries {
if now.After(entry.ExpiresAt) {
delete(c.entries, uri)
}
}
}

// githubResolutionCacheDir returns the directory for storing GitHub
// resolution cache files.
func githubResolutionCacheDir() (string, error) {
globalDir, err := config.GetGlobalDir()
if err != nil {
return "", err
}
return filepath.Join(globalDir, "cache", "github-resolution"), nil
}
151 changes: 151 additions & 0 deletions pkg/agent/github_resolution_cache_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package agent

import (
"os"
"path/filepath"
"testing"
"time"
)

func TestGitHubResolutionCache_PutAndGet(t *testing.T) {
dir := t.TempDir()
cache, err := NewGitHubResolutionCache(dir, 5*time.Minute)
if err != nil {
t.Fatalf("NewGitHubResolutionCache: %v", err)
}

skill := ResolvedSkill{
Name: "my-skill",
URI: "gh://owner/repo/my-skill@main",
Version: "abc123def456",
Hash: "sha256:deadbeef",
Files: []ResolvedFile{
{Path: "SKILL.md", URL: "https://example.com/SKILL.md", Hash: "sha256:abc", Size: 42},
},
}

cache.Put("gh://owner/repo/my-skill@main", skill)

got, ok := cache.Get("gh://owner/repo/my-skill@main")
if !ok {
t.Fatal("expected cache hit, got miss")
}
if got.Name != "my-skill" {
t.Errorf("expected name my-skill, got %s", got.Name)
}
if got.Hash != "sha256:deadbeef" {
t.Errorf("expected hash sha256:deadbeef, got %s", got.Hash)
}
if len(got.Files) != 1 {
t.Fatalf("expected 1 file, got %d", len(got.Files))
}
}

func TestGitHubResolutionCache_Miss(t *testing.T) {
dir := t.TempDir()
cache, err := NewGitHubResolutionCache(dir, 5*time.Minute)
if err != nil {
t.Fatalf("NewGitHubResolutionCache: %v", err)
}

_, ok := cache.Get("gh://owner/repo/nonexistent@main")
if ok {
t.Fatal("expected cache miss, got hit")
}
}

func TestGitHubResolutionCache_Expiry(t *testing.T) {
dir := t.TempDir()
cache, err := NewGitHubResolutionCache(dir, 1*time.Millisecond)
if err != nil {
t.Fatalf("NewGitHubResolutionCache: %v", err)
}

skill := ResolvedSkill{
Name: "expiring-skill",
URI: "gh://owner/repo/expiring@main",
}
cache.Put("gh://owner/repo/expiring@main", skill)

// Wait for expiry
time.Sleep(5 * time.Millisecond)

_, ok := cache.Get("gh://owner/repo/expiring@main")
if ok {
t.Fatal("expected cache miss after expiry, got hit")
}
}

func TestGitHubResolutionCache_PersistAndReload(t *testing.T) {
dir := t.TempDir()
cache, err := NewGitHubResolutionCache(dir, 5*time.Minute)
if err != nil {
t.Fatalf("NewGitHubResolutionCache: %v", err)
}

skill := ResolvedSkill{
Name: "persist-skill",
URI: "gh://owner/repo/persist@main",
Version: "abc123def456",
Hash: "sha256:persist",
}
cache.Put("gh://owner/repo/persist@main", skill)

// Verify file exists on disk
cacheFile := filepath.Join(dir, resolutionCacheFileName)
if _, err := os.Stat(cacheFile); err != nil {
t.Fatalf("cache file not persisted: %v", err)
}

// Create a new cache instance from the same directory
cache2, err := NewGitHubResolutionCache(dir, 5*time.Minute)
if err != nil {
t.Fatalf("NewGitHubResolutionCache (reload): %v", err)
}

got, ok := cache2.Get("gh://owner/repo/persist@main")
if !ok {
t.Fatal("expected cache hit after reload, got miss")
}
if got.Name != "persist-skill" {
t.Errorf("expected name persist-skill, got %s", got.Name)
}
}

func TestGitHubResolutionCache_ExpiredNotLoaded(t *testing.T) {
dir := t.TempDir()
cache, err := NewGitHubResolutionCache(dir, 1*time.Millisecond)
if err != nil {
t.Fatalf("NewGitHubResolutionCache: %v", err)
}

skill := ResolvedSkill{Name: "expired-skill", URI: "gh://o/r/s@main"}
cache.Put("gh://o/r/s@main", skill)

time.Sleep(5 * time.Millisecond)

// Reload — expired entries should not be loaded
cache2, err := NewGitHubResolutionCache(dir, 5*time.Minute)
if err != nil {
t.Fatalf("NewGitHubResolutionCache (reload): %v", err)
}

_, ok := cache2.Get("gh://o/r/s@main")
if ok {
t.Fatal("expected expired entry to not be loaded, got hit")
}
}
Loading
Loading