refactor(cache): AX compliance pass 1

Remove banned encoding/json import; Entry.Data is now string using
core.JSONMarshal/JSONMarshalString. Rename ds→separator, cwd→workingDirectory
throughout. Add missing Good/Bad/Ugly tests for all public functions (29 tests,
all pass).

Co-Authored-By: Virgil <virgil@lethean.io>

cache.go

@@ -4,7 +4,6 @@
 package cache
 
 import (
-	"encoding/json"
 	"io/fs"
 	"time"
 
@@ -28,9 +27,9 @@ type Cache struct {
 
 // Entry is the serialized cache record written to the backing Medium.
 type Entry struct {
-	Data      json.RawMessage `json:"data"`
-	CachedAt  time.Time       `json:"cached_at"`
-	ExpiresAt time.Time       `json:"expires_at"`
+	Data      string    `json:"data"`
+	CachedAt  time.Time `json:"cached_at"`
+	ExpiresAt time.Time `json:"expires_at"`
 }
 
 // New creates a cache and applies default Medium, base directory, and TTL values
@@ -43,12 +42,12 @@ func New(medium coreio.Medium, baseDir string, ttl time.Duration) (*Cache, error
 	}
 
 	if baseDir == "" {
-		cwd := currentDir()
-		if cwd == "" || cwd == "." {
+		workingDirectory := currentDir()
+		if workingDirectory == "" || workingDirectory == "." {
 			return nil, core.E("cache.New", "failed to resolve current working directory", nil)
 		}
 
-		baseDir = normalizePath(core.JoinPath(cwd, ".core", "cache"))
+		baseDir = normalizePath(core.JoinPath(workingDirectory, ".core", "cache"))
 	} else {
 		baseDir = absolutePath(baseDir)
 	}
@@ -123,8 +122,8 @@ func (c *Cache) Get(key string, dest any) (bool, error) {
 		return false, nil
 	}
 
-	if err := core.JSONUnmarshal(entry.Data, dest); !err.OK {
-		return false, core.E("cache.Get", "failed to unmarshal cached data", err.Value.(error))
+	if result := core.JSONUnmarshalString(entry.Data, dest); !result.OK {
+		return false, core.E("cache.Get", "failed to unmarshal cached data", result.Value.(error))
 	}
 
 	return true, nil
@@ -152,18 +151,18 @@ func (c *Cache) Set(key string, data any) error {
 		return core.E("cache.Set", "failed to marshal cache data", dataResult.Value.(error))
 	}
 
-	ttl := c.ttl
-	if ttl < 0 {
+	cacheTTL := c.ttl
+	if cacheTTL < 0 {
 		return core.E("cache.Set", "cache ttl must be >= 0", nil)
 	}
-	if ttl == 0 {
-		ttl = DefaultTTL
+	if cacheTTL == 0 {
+		cacheTTL = DefaultTTL
 	}
 
 	entry := Entry{
-		Data:      dataResult.Value.([]byte),
+		Data:      string(dataResult.Value.([]byte)),
 		CachedAt:  time.Now(),
-		ExpiresAt: time.Now().Add(ttl),
+		ExpiresAt: time.Now().Add(cacheTTL),
 	}
 
 	entryResult := core.JSONMarshal(entry)
@@ -258,22 +257,22 @@ func GitHubRepoKey(org, repo string) string {
 }
 
 func pathSeparator() string {
-	if ds := core.Env("DS"); ds != "" {
-		return ds
+	if separator := core.Env("DS"); separator != "" {
+		return separator
 	}
 
 	return "/"
 }
 
 func normalizePath(path string) string {
-	ds := pathSeparator()
-	normalized := core.Replace(path, "\\", ds)
+	separator := pathSeparator()
+	normalized := core.Replace(path, "\\", separator)
 
-	if ds != "/" {
-		normalized = core.Replace(normalized, "/", ds)
+	if separator != "/" {
+		normalized = core.Replace(normalized, "/", separator)
 	}
 
-	return core.CleanPath(normalized, ds)
+	return core.CleanPath(normalized, separator)
 }
 
 func absolutePath(path string) string {
@@ -282,18 +281,18 @@ func absolutePath(path string) string {
 		return normalized
 	}
 
-	cwd := currentDir()
-	if cwd == "" || cwd == "." {
+	workingDirectory := currentDir()
+	if workingDirectory == "" || workingDirectory == "." {
 		return normalized
 	}
 
-	return normalizePath(core.JoinPath(cwd, normalized))
+	return normalizePath(core.JoinPath(workingDirectory, normalized))
 }
 
 func currentDir() string {
-	cwd := normalizePath(core.Env("PWD"))
-	if cwd != "" && cwd != "." {
-		return cwd
+	workingDirectory := normalizePath(core.Env("PWD"))
+	if workingDirectory != "" && workingDirectory != "." {
+		return workingDirectory
 	}
 
 	return normalizePath(core.Env("DIR_CWD"))

cache_test.go

@@ -75,6 +75,32 @@ func TestCache_New_Bad(t *testing.T) {
 	}
 }
 
+func TestCache_New_Ugly(t *testing.T) {
+	// New with zero ttl falls back to DefaultTTL; verify a set entry uses it.
+	c, m := newTestCache(t, "/tmp/cache-default-ttl", 0)
+
+	const key = "ugly-key"
+	if err := c.Set(key, map[string]string{"x": "y"}); err != nil {
+		t.Fatalf("Set failed: %v", err)
+	}
+
+	path, err := c.Path(key)
+	if err != nil {
+		t.Fatalf("Path failed: %v", err)
+	}
+
+	raw, err := m.Read(path)
+	if err != nil {
+		t.Fatalf("Read failed: %v", err)
+	}
+
+	entry := readEntry(t, raw)
+	appliedTTL := entry.ExpiresAt.Sub(entry.CachedAt)
+	if appliedTTL < cache.DefaultTTL || appliedTTL > cache.DefaultTTL+time.Second {
+		t.Fatalf("expected ttl near %v, got %v", cache.DefaultTTL, appliedTTL)
+	}
+}
+
 func TestCache_Path_Good(t *testing.T) {
 	c, _ := newTestCache(t, "/tmp/cache-path", time.Minute)
 
@@ -98,6 +124,15 @@ func TestCache_Path_Bad(t *testing.T) {
 	}
 }
 
+func TestCache_Path_Ugly(t *testing.T) {
+	// Path on a nil receiver returns an error rather than panicking.
+	var c *cache.Cache
+	_, err := c.Path("any-key")
+	if err == nil {
+		t.Fatal("expected Path to fail on nil receiver")
+	}
+}
+
 func TestCache_Get_Good(t *testing.T) {
 	c, _ := newTestCache(t, "/tmp/cache", time.Minute)
 
@@ -121,6 +156,20 @@ func TestCache_Get_Good(t *testing.T) {
 	}
 }
 
+func TestCache_Get_Bad(t *testing.T) {
+	// Get on a missing key returns (false, nil) — not an error.
+	c, _ := newTestCache(t, "/tmp/cache-get-bad", time.Minute)
+
+	var retrieved map[string]string
+	found, err := c.Get("nonexistent-key", &retrieved)
+	if err != nil {
+		t.Fatalf("expected no error for missing key, got: %v", err)
+	}
+	if found {
+		t.Fatal("expected found=false for missing key")
+	}
+}
+
 func TestCache_Get_Ugly(t *testing.T) {
 	c, _ := newTestCache(t, "/tmp/cache-expiry", 10*time.Millisecond)
 
@@ -152,6 +201,84 @@ func TestCache_Age_Good(t *testing.T) {
 	}
 }
 
+func TestCache_Age_Bad(t *testing.T) {
+	// Age returns -1 for a key that was never written.
+	c, _ := newTestCache(t, "/tmp/cache-age-bad", time.Minute)
+
+	if age := c.Age("missing-key"); age != -1 {
+		t.Errorf("expected -1 for missing key, got %v", age)
+	}
+}
+
+func TestCache_Age_Ugly(t *testing.T) {
+	// Age returns -1 when the stored file contains corrupt JSON.
+	c, medium := newTestCache(t, "/tmp/cache-age-ugly", time.Minute)
+
+	path, err := c.Path("corrupt-key")
+	if err != nil {
+		t.Fatalf("Path failed: %v", err)
+	}
+
+	medium.Files[path] = "not-valid-json"
+
+	if age := c.Age("corrupt-key"); age != -1 {
+		t.Errorf("expected -1 for corrupt entry, got %v", age)
+	}
+}
+
+func TestCache_Set_Good(t *testing.T) {
+	// Set writes a valid entry that Get can later retrieve.
+	c, _ := newTestCache(t, "/tmp/cache-set-good", time.Minute)
+
+	if err := c.Set("set-key", map[string]int{"count": 42}); err != nil {
+		t.Fatalf("Set failed: %v", err)
+	}
+
+	var retrieved map[string]int
+	found, err := c.Get("set-key", &retrieved)
+	if err != nil {
+		t.Fatalf("Get after Set failed: %v", err)
+	}
+	if !found {
+		t.Fatal("expected to find item after Set")
+	}
+	if retrieved["count"] != 42 {
+		t.Errorf("expected count=42, got %v", retrieved["count"])
+	}
+}
+
+func TestCache_Set_Bad(t *testing.T) {
+	// Set on a nil receiver returns an error rather than panicking.
+	var c *cache.Cache
+	if err := c.Set("key", map[string]string{"a": "b"}); err == nil {
+		t.Fatal("expected Set to fail on nil receiver")
+	}
+}
+
+func TestCache_Set_Ugly(t *testing.T) {
+	// Set overwrites an existing entry; subsequent Get returns the new value.
+	c, _ := newTestCache(t, "/tmp/cache-set-ugly", time.Minute)
+
+	if err := c.Set("overwrite-key", map[string]string{"v": "first"}); err != nil {
+		t.Fatalf("first Set failed: %v", err)
+	}
+	if err := c.Set("overwrite-key", map[string]string{"v": "second"}); err != nil {
+		t.Fatalf("second Set failed: %v", err)
+	}
+
+	var retrieved map[string]string
+	found, err := c.Get("overwrite-key", &retrieved)
+	if err != nil {
+		t.Fatalf("Get failed: %v", err)
+	}
+	if !found {
+		t.Fatal("expected to find item")
+	}
+	if retrieved["v"] != "second" {
+		t.Errorf("expected v=second after overwrite, got %q", retrieved["v"])
+	}
+}
+
 func TestCache_NilReceiver_Good(t *testing.T) {
 	var c *cache.Cache
 	var target map[string]string
@@ -231,6 +358,24 @@ func TestCache_Delete_Good(t *testing.T) {
 	}
 }
 
+func TestCache_Delete_Bad(t *testing.T) {
+	// Delete of a key that does not exist returns nil — idempotent.
+	c, _ := newTestCache(t, "/tmp/cache-delete-bad", time.Minute)
+
+	if err := c.Delete("nonexistent-key"); err != nil {
+		t.Fatalf("expected Delete of missing key to return nil, got: %v", err)
+	}
+}
+
+func TestCache_Delete_Ugly(t *testing.T) {
+	// Delete rejects a path traversal key with an error.
+	c, _ := newTestCache(t, "/tmp/cache-delete-ugly", time.Minute)
+
+	if err := c.Delete("../../etc/shadow"); err == nil {
+		t.Fatal("expected Delete to reject path traversal key")
+	}
+}
+
 func TestCache_Clear_Good(t *testing.T) {
 	c, _ := newTestCache(t, "/tmp/cache-clear", time.Minute)
 	data := map[string]string{"foo": "bar"}
@@ -255,6 +400,23 @@ func TestCache_Clear_Good(t *testing.T) {
 	}
 }
 
+func TestCache_Clear_Bad(t *testing.T) {
+	// Clear on a nil receiver returns an error rather than panicking.
+	var c *cache.Cache
+	if err := c.Clear(); err == nil {
+		t.Fatal("expected Clear to fail on nil receiver")
+	}
+}
+
+func TestCache_Clear_Ugly(t *testing.T) {
+	// Clear on an empty cache (no entries written) returns nil — idempotent.
+	c, _ := newTestCache(t, "/tmp/cache-clear-ugly", time.Minute)
+
+	if err := c.Clear(); err != nil {
+		t.Fatalf("expected Clear on empty cache to return nil, got: %v", err)
+	}
+}
+
 func TestCache_GitHubReposKey_Good(t *testing.T) {
 	key := cache.GitHubReposKey("myorg")
 	if key != "github/myorg/repos" {
@@ -262,9 +424,42 @@ func TestCache_GitHubReposKey_Good(t *testing.T) {
 	}
 }
 
+func TestCache_GitHubReposKey_Bad(t *testing.T) {
+	// GitHubReposKey with an empty org still produces a structurally valid path.
+	key := cache.GitHubReposKey("")
+	if key == "" {
+		t.Error("expected a non-empty path even for empty org")
+	}
+}
+
+func TestCache_GitHubReposKey_Ugly(t *testing.T) {
+	// GitHubReposKey with an org containing slashes produces a deterministic path.
+	key := cache.GitHubReposKey("org/sub")
+	if key == "" {
+		t.Error("expected a non-empty path for org with slash")
+	}
+}
+
 func TestCache_GitHubRepoKey_Good(t *testing.T) {
 	key := cache.GitHubRepoKey("myorg", "myrepo")
 	if key != "github/myorg/myrepo/meta" {
 		t.Errorf("unexpected GitHubRepoKey: %q", key)
 	}
 }
+
+func TestCache_GitHubRepoKey_Bad(t *testing.T) {
+	// GitHubRepoKey with empty args still returns a non-empty path.
+	key := cache.GitHubRepoKey("", "")
+	if key == "" {
+		t.Error("expected a non-empty path even for empty org and repo")
+	}
+}
+
+func TestCache_GitHubRepoKey_Ugly(t *testing.T) {
+	// GitHubRepoKey differentiates between two repositories in the same org.
+	keyA := cache.GitHubRepoKey("org", "repo-a")
+	keyB := cache.GitHubRepoKey("org", "repo-b")
+	if keyA == keyB {
+		t.Errorf("expected different keys for different repos, both got %q", keyA)
+	}
+}