Fix package-level mutable slice/map reassignment in virtual_fs.go and actionpins/data.go

pkg/actionpins/actionpins_internal_test.go

@@ -420,10 +420,8 @@ func TestGetContainerPin_ReturnsPinnedImage(t *testing.T) {
 }
 
 func TestGetContainerPin_MCPGatewayVersionsArePinned(t *testing.T) {
-	getActionPins()
-
 	var mcpgImages []string
-	for image := range cachedContainerPins {
+	for image := range getCachedActionPins().containers {
 		if strings.HasPrefix(image, "ghcr.io/github/gh-aw-mcpg:") {
 			mcpgImages = append(mcpgImages, image)
 		}
@@ -467,6 +465,16 @@ func TestGetActionPins_CacheCorrectnessOnRepeatedCalls(t *testing.T) {
 	assert.Equal(t, first, second, "Expected repeated calls to getActionPins() to return equal data (cache correctness)")
 }
 
+func TestGetCachedActionPins_InitializesCache(t *testing.T) {
+	cache := getCachedActionPins()
+
+	require.NotNil(t, cache, "Expected cache accessor to return initialized data")
+	assert.NotEmpty(t, cache.pins, "Expected cached action pins")
+	assert.NotNil(t, cache.byRepo, "Expected cached action pins by repository")
+	assert.NotNil(t, cache.containers, "Expected cached container pins")
+	assert.Same(t, cache, getCachedActionPins(), "Expected repeated cache access to return the same cache")
+}
+
 func TestResolveActionPinDynamically_SkipsForSHAInput(t *testing.T) {
 	t.Parallel()
 	resolver := &countingResolver{}

pkg/actionpins/data.go

@@ -19,14 +19,22 @@ var actionPinsLog = logger.New("actionpins:actionpins")
 //go:embed data/action_pins.json
 var actionPinsJSON []byte
 
+// actionPinsCache bundles the parsed/derived action pin data behind a single
+// pointer so the package-level variable holding it is never a bare slice or
+// map that gets reassigned in place; the pointer itself is written exactly
+// once (guarded by actionPinsOnce) and is treated as read-only thereafter.
+type actionPinsCache struct {
+	pins       []ActionPin
+	byRepo     map[string][]ActionPin
+	containers map[string]ContainerPin
+}
+
 var (
-	cachedActionPins       []ActionPin
-	cachedActionPinsByRepo map[string][]ActionPin
-	cachedContainerPins    map[string]ContainerPin
-	actionPinsOnce         sync.Once
+	cachedPins     *actionPinsCache
+	actionPinsOnce sync.Once
 )
 
-func getActionPins() []ActionPin {
+func getCachedActionPins() *actionPinsCache {
 	actionPinsOnce.Do(func() {
 		actionPinsLog.Print("Unmarshaling action pins from embedded JSON (first call, will be cached)")
 
@@ -42,19 +50,31 @@ func getActionPins() []ActionPin {
 		})
 
 		actionPinsLog.Printf("Successfully unmarshaled and sorted %d action pins from JSON", len(pins))
-		cachedActionPins = pins
 
-		cachedActionPinsByRepo = buildByRepoIndex(pins)
-		actionPinsLog.Printf("Built per-repo action pin index for %d repos", len(cachedActionPinsByRepo))
+		byRepo := buildByRepoIndex(pins)
+		actionPinsLog.Printf("Built per-repo action pin index for %d repos", len(byRepo))
+
+		containers := data.Containers
+		if containers == nil {
+			containers = make(map[string]ContainerPin)
+		}
+		actionPinsLog.Printf("Loaded %d container pins from JSON", len(containers))
 
-		cachedContainerPins = data.Containers
-		if cachedContainerPins == nil {
-			cachedContainerPins = make(map[string]ContainerPin)
+		cachedPins = &actionPinsCache{
+			pins:       pins,
+			byRepo:     byRepo,
+			containers: containers,
 		}
-		actionPinsLog.Printf("Loaded %d container pins from JSON", len(cachedContainerPins))
 	})
 
-	return cachedActionPins
+	if cachedPins == nil {
+		panic("action pins cache was not initialized")
+	}
+	return cachedPins
+}
+
+func getActionPins() []ActionPin {
+	return getCachedActionPins().pins
 }
 
 // loadActionPinsData unmarshals embedded action pin data.
@@ -129,8 +149,7 @@ func buildByRepoIndex(pins []ActionPin) map[string][]ActionPin {
 // GetActionPinsByRepo returns the sorted (version-descending) list of action pins
 // for the given repository. Returns nil if the repo has no pins.
 func GetActionPinsByRepo(repo string) []ActionPin {
-	getActionPins()
-	return cachedActionPinsByRepo[repo]
+	return getCachedActionPins().byRepo[repo]
 }
 
 // GetLatestActionPinByRepo returns the latest ActionPin for a given repository, if any.
@@ -144,7 +163,6 @@ func GetLatestActionPinByRepo(repo string) (ActionPin, bool) {
 
 // GetContainerPin returns a pinned container image by its original image reference.
 func GetContainerPin(image string) (ContainerPin, bool) {
-	getActionPins()
-	pin, ok := cachedContainerPins[image]
+	pin, ok := getCachedActionPins().containers[image]
 	return pin, ok
 }

pkg/parser/virtual_fs.go

@@ -15,10 +15,15 @@ var virtualFsLog = logger.New("parser:virtual_fs")
 
 // builtinVirtualFiles holds embedded built-in files registered at startup.
 // Keys use the "@builtin:" path prefix (e.g. "@builtin:engines/copilot.md").
-// The map is replaced using copy-on-write during registration and then treated
-// as read-only; concurrent reads are safe.
+// Registration swaps a pointer to an immutable snapshot rather than assigning
+// a map directly. The named snapshot type makes this copy-on-write pattern
+// explicit: readers only see fully-populated, read-only snapshots.
+type builtinVirtualFileSnapshot struct {
+	files map[string][]byte
+}
+
 var (
-	builtinVirtualFiles   map[string][]byte
+	builtinVirtualFiles   = &builtinVirtualFileSnapshot{files: map[string][]byte{}}
 	builtinVirtualFilesMu sync.RWMutex
 )
 
@@ -33,24 +38,25 @@ func RegisterBuiltinVirtualFile(path string, content []byte) {
 	}
 	builtinVirtualFilesMu.Lock()
 	defer builtinVirtualFilesMu.Unlock()
-	if existing, ok := builtinVirtualFiles[path]; ok {
+	current := builtinVirtualFiles.files
+	if existing, ok := current[path]; ok {
 		if !bytes.Equal(existing, content) {
 			panic(fmt.Sprintf("RegisterBuiltinVirtualFile: path %q already registered with different content", path))
 		}
 		return // idempotent: same content, no-op
 	}
 	virtualFsLog.Printf("Registering builtin virtual file: %s (%d bytes)", path, len(content))
-	next := make(map[string][]byte, len(builtinVirtualFiles)+1)
-	maps.Copy(next, builtinVirtualFiles)
+	next := make(map[string][]byte, len(current)+1)
+	maps.Copy(next, current)
 	next[path] = bytes.Clone(content)
-	builtinVirtualFiles = next
+	builtinVirtualFiles = &builtinVirtualFileSnapshot{files: next}
 }
 
 // BuiltinVirtualFileExists returns true if the given path is registered as a builtin virtual file.
 func BuiltinVirtualFileExists(path string) bool {
 	builtinVirtualFilesMu.RLock()
 	defer builtinVirtualFilesMu.RUnlock()
-	_, ok := builtinVirtualFiles[path]
+	_, ok := builtinVirtualFiles.files[path]
 	virtualFsLog.Printf("BuiltinVirtualFileExists: path=%s exists=%t", path, ok)
 	return ok
 }
@@ -116,7 +122,7 @@ const BuiltinPathPrefix = "@builtin:"
 var readFileFunc = func(path string) ([]byte, error) {
 	builtinVirtualFilesMu.RLock()
 	defer builtinVirtualFilesMu.RUnlock()
-	content, ok := builtinVirtualFiles[path]
+	content, ok := builtinVirtualFiles.files[path]
 	if ok {
 		return bytes.Clone(content), nil
 	}

pkg/parser/virtual_fs_wasm.go

@@ -38,7 +38,7 @@ func init() {
 		// Check builtin virtual files first (embedded engine .md files etc.)
 		builtinVirtualFilesMu.RLock()
 		defer builtinVirtualFilesMu.RUnlock()
-		builtinContent, builtinOK := builtinVirtualFiles[path]
+		builtinContent, builtinOK := builtinVirtualFiles.files[path]
 		if builtinOK {
 			parserLog.Printf("readFileFunc: resolved builtin virtual file: %s", path)
 			return builtinContent, nil