fix(appkit): honor TTL in CacheManager.getOrExecute (#326)

Signed-off-by: Tim Hoare <t.hoare@codat.io>
Signed-off-by: TimHoare <t.hoare@codat.io>
Co-authored-by: Mario Cadenas <17888484+MarioCadenas@users.noreply.github.com>

Author: Tim-Hoare

packages/appkit/src/cache/index.ts

@@ -1,6 +1,6 @@
 import { createHash } from "node:crypto";
 import { ApiError, WorkspaceClient } from "@databricks/sdk-experimental";
-import type { CacheConfig, CacheStorage } from "shared";
+import type { CacheConfig, CacheEntry, CacheStorage } from "shared";
 import { createLakebasePool } from "../connectors/lakebase";
 import { AppKitError, ExecutionError, InitializationError } from "../errors";
 import { createLogger } from "../logging/logger";
@@ -202,8 +202,7 @@ export class CacheManager {
       },
       async (span) => {
         try {
-          // check if the value is in the cache
-          const cached = await this.storage.get<T>(cacheKey);
+          const cached = await this.getValid<T>(cacheKey);
           if (cached !== null) {
             span.setAttribute("cache.hit", true);
             span.setStatus({ code: SpanStatusCode.OK });
@@ -216,7 +215,7 @@ export class CacheManager {
               cache_key: cacheKey,
             });
 
-            return cached.value as T;
+            return cached.value;
           }
 
           // check if the value is being processed by another request
@@ -308,14 +307,26 @@ export class CacheManager {
     // probabilistic cleanup trigger
     this.maybeCleanup();
 
+    const entry = await this.getValid<T>(key);
+    return entry?.value ?? null;
+  }
+
+  /**
+   * Get a cached entry only if it has not expired.
+   * Returns null on miss or expired (and deletes the expired entry).
+   *
+   * Storage implementations return entries unconditionally — expiry handling
+   * lives at the CacheManager layer.
+   */
+  private async getValid<T>(key: string): Promise<CacheEntry<T> | null> {
     const entry = await this.storage.get<T>(key);
     if (!entry) return null;
 
     if (Date.now() > entry.expiry) {
       await this.storage.delete(key);
       return null;
     }
-    return entry.value as T;
+    return entry;
   }
 
   /** Probabilistically trigger cleanup of expired entries (fire-and-forget) */
@@ -386,14 +397,8 @@ export class CacheManager {
   async has(key: string): Promise<boolean> {
     if (!this.config.enabled) return false;
 
-    const entry = await this.storage.get(key);
-    if (!entry) return false;
-
-    if (Date.now() > entry.expiry) {
-      await this.storage.delete(key);
-      return false;
-    }
-    return true;
+    const entry = await this.getValid(key);
+    return entry !== null;
   }
 
   /**

packages/appkit/src/cache/tests/cache-manager.test.ts

@@ -354,6 +354,30 @@ describe("CacheManager", () => {
       expect(result1).toBe("user1-data");
       expect(result2).toBe("user2-data");
     });
+
+    test("should re-execute function when cached entry has expired", async () => {
+      const cache = await CacheManager.getInstance({
+        storage: createMockStorage(),
+      });
+      let calls = 0;
+      const fn = vi.fn().mockImplementation(async () => `result-${++calls}`);
+
+      // First call - populates cache with a 1ms TTL
+      const r1 = await cache.getOrExecute(["key"], fn, "user1", {
+        ttl: 0.001,
+      });
+      expect(r1).toBe("result-1");
+
+      // Wait for expiry
+      await new Promise((resolve) => setTimeout(resolve, 10));
+
+      // Second call - cached entry is past its expiry, fn must run again
+      const r2 = await cache.getOrExecute(["key"], fn, "user1", {
+        ttl: 0.001,
+      });
+      expect(r2).toBe("result-2");
+      expect(fn).toHaveBeenCalledTimes(2);
+    });
   });
 
   describe("disabled cache", () => {