fix(cache): report a denied cache write as a failure (#1370)

@actions/cache catches reservation and upload failures internally and
returns -1 rather than throwing, so the catch block never ran and every
save reported success. A run whose write was denied logged 'Cache saved'
25ms after the platform refused it, then set the state flag that tells
the post-action there is nothing left to do.

Branch on the sentinel at all three call sites. The library cannot
distinguish a policy denial from a benign key collision once it has
swallowed the error, so a collision is now reported as a failure too;
the alternative is continuing to report real failures as success.

Restores the post-action retry, which the false success had been
suppressing.

Author: marcusrbrown

dist/artifact-C-p9WALw.js dist/artifact-DdovPt19.jsdist/artifact-C-p9WALw.js renamed to dist/artifact-DdovPt19.js

@@ -470,6 +470,36 @@ describe('saveCache', () => {
     expect(result).toBe(true)
   })
 
+  it('returns false and warns when save returns the failure sentinel', async () => {
+    // #given storage with content and an adapter that reports an unsuccessful save
+    await fs.mkdir(storagePath, {recursive: true})
+    await fs.writeFile(path.join(storagePath, 'session.db'), 'test data')
+
+    const logger: Logger = {
+      debug: vi.fn(),
+      info: vi.fn(),
+      warning: vi.fn(),
+      error: vi.fn(),
+    }
+    const adapter = createMockCacheAdapter({saveResult: -1})
+
+    // #when saving cache
+    const result = await saveCache({
+      components: testComponents,
+      runId: 98765,
+      logger,
+      storagePath,
+      authPath,
+      cacheAdapter: adapter,
+    })
+
+    // #then the failure sentinel is reported as an unpersisted cache
+    expect(result).toBe(false)
+    expect(logger.warning).toHaveBeenCalledWith('Cache save did not persist', {
+      saveKey: 'opencode-storage-github-owner-repo-main-Linux-98765',
+    })
+  })
+
   it('writes to object store and cache when configured', async () => {
     await fs.mkdir(storagePath, {recursive: true})
     await fs.writeFile(path.join(storagePath, 'session.db'), 'test data')

dist/main.js

@@ -137,6 +137,25 @@ describe('saveDeduplicationMarker', () => {
     expect(saveCache).toHaveBeenCalledWith([testEntityDir], `${DEDUP_CACHE_PREFIX}-owner-repo-pr-42-3003`)
   })
 
+  it('returns false and warns when save returns the failure sentinel', async () => {
+    // #given a marker and an adapter that reports an unsuccessful save
+    const marker = createMarker(3503)
+    const logger = createMockLogger()
+    const cacheAdapter: CacheAdapter = {
+      restoreCache: vi.fn(async () => undefined),
+      saveCache: vi.fn(async () => -1),
+    }
+
+    // #when saving the deduplication marker
+    const result = await saveDeduplicationMarker(testRepo, testEntity, marker, logger, cacheAdapter)
+
+    // #then the failure sentinel is reported as an unpersisted marker
+    expect(result).toBe(false)
+    expect(logger.warning).toHaveBeenCalledWith('Dedup marker cache save did not persist', {
+      saveKey: `${DEDUP_CACHE_PREFIX}-owner-repo-pr-42-3503`,
+    })
+  })
+
   it('writes sentinel file with marker json content', async () => {
     // #given marker and save adapter
     const marker = createMarker(4004)

dist/post.js

@@ -87,7 +87,14 @@ export async function saveDeduplicationMarker(
   try {
     await fs.mkdir(entityDir, {recursive: true})
     await fs.writeFile(sentinelPath, JSON.stringify(marker), 'utf8')
-    await cacheAdapter.saveCache([entityDir], saveKey)
+    const cacheId = await cacheAdapter.saveCache([entityDir], saveKey)
+    // @actions/cache returns -1 for both write failures and reservation collisions. The
+    // adapter exposes no reason, so report the marker as unpersisted rather than claiming success.
+    if (cacheId === -1) {
+      logger.warning('Dedup marker cache save did not persist', {saveKey})
+      return false
+    }
+
     return true
   } catch (error) {
     const message = toErrorMessage(error).toLowerCase()

src/services/cache/cache.test.ts

@@ -113,7 +113,14 @@ export async function saveCache(options: SaveCacheOptions): Promise<boolean> {
       }
     }
 
-    await cacheAdapter.saveCache(cachePaths, saveKey)
+    const cacheId = await cacheAdapter.saveCache(cachePaths, saveKey)
+    // @actions/cache returns -1 for both write failures and reservation collisions. The
+    // adapter exposes no reason, so report the save as unpersisted rather than claiming success.
+    if (cacheId === -1) {
+      logger.warning('Cache save did not persist', {saveKey})
+      return false
+    }
+
     logger.info('Cache saved', {saveKey})
     return true
   } catch (error) {

src/services/cache/dedup.test.ts

@@ -2,7 +2,7 @@ import type {Logger} from '../../shared/logger.js'
 import * as fs from 'node:fs/promises'
 import * as os from 'node:os'
 import * as path from 'node:path'
-import {afterEach, beforeEach, describe, expect, it} from 'vitest'
+import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'
 import {
   buildCachePaths,
   buildToolsCacheKey,
@@ -618,6 +618,39 @@ describe('saveToolsCache', () => {
     expect(result).toBe(true)
   })
 
+  it('returns false and warns when save returns the failure sentinel', async () => {
+    // #given an adapter that reports an unsuccessful save
+    const logger: Logger = {
+      debug: vi.fn(),
+      info: vi.fn(),
+      warning: vi.fn(),
+      error: vi.fn(),
+    }
+    const adapter = createMockToolsCacheAdapter({saveResult: -1})
+
+    // #when saving the tools cache
+    const result = await saveToolsCache({
+      logger,
+      os: 'Linux',
+      bunVersion: '1.3.14',
+      opencodeVersion: '1.0.0',
+      omoVersion: '3.5.5',
+      systematicVersion: '2.1.0',
+      cacheMode: 'enabled',
+      toolCachePath,
+      bunCachePath,
+      omoConfigPath,
+      opencodeCachePath,
+      cacheAdapter: adapter,
+    })
+
+    // #then the failure sentinel is reported as an unpersisted cache
+    expect(result).toBe(false)
+    expect(logger.warning).toHaveBeenCalledWith('Tools cache save did not persist', {
+      saveKey: 'opencode-tools-Linux-enabled-oc-1.0.0-omo-3.5.5-sys-2.1.0-bun-1.3.14',
+    })
+  })
+
   it('saves cache successfully in disabled mode', async () => {
     // #given a cache adapter
     const adapter = createMockToolsCacheAdapter({saveResult: 1024})

src/services/cache/dedup.ts

@@ -170,7 +170,14 @@ export async function saveToolsCache(options: SaveToolsCacheOptions): Promise<bo
   logger.info('Saving tools cache', {saveKey, paths: cachePaths})
 
   try {
-    await cacheAdapter.saveCache([...cachePaths], saveKey)
+    const cacheId = await cacheAdapter.saveCache([...cachePaths], saveKey)
+    // @actions/cache returns -1 for both write failures and reservation collisions. The
+    // adapter exposes no reason, so report the save as unpersisted rather than claiming success.
+    if (cacheId === -1) {
+      logger.warning('Tools cache save did not persist', {saveKey})
+      return false
+    }
+
     logger.info('Tools cache saved', {saveKey})
     return true
   } catch (error) {