ADFA-4128: invalidate the retained payload when its directory cannot be deleted

retain() and clear() discarded deleteRecursively's result, so a partial delete could
leave a readable meta.json behind - contradicting this store's own "replaced wholesale
by every retain" contract.

Both now go through a private purge() that empties meta.json before the recursive delete
and returns the delete's result. The order is the load-bearing part: unlinking a name
needs write permission on the directory, rewriting a file's bytes needs it only on the
file, so a truncated meta.json still fails load() closed in the case where the delete
cannot proceed at all. load() keys off the metadata alone, so once that is unreadable the
rest of the tree is inert. retain() checks the result into its existing catch; clear()
warns when parts survive.

Not propagated to callers: both are post-deploy bookkeeping on the success path, with the
payload already live in the app. Failing a build for a retention housekeeping error would
turn a cheaper-recovery optimisation into a build failure, and the documented fallback -
the catch-up rebuild - still happens.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

Author: fryanpan

quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/RetainedPayloadStore.kt

@@ -52,7 +52,9 @@ internal class RetainedPayloadStore(
 	 * confirmed the payload, so what is retained is always something known to have run.
 	 *
 	 * The swap goes through a staging dir: a crash at any point leaves either the previous
-	 * set, or nothing - never a half-written mix that [load] could hand to a re-send.
+	 * set, or nothing - never a half-written mix that [load] could hand to a re-send. A
+	 * delete that fails is the same case: [purge] invalidates the old set before removing it,
+	 * so this deploy failing to retain leaves nothing to load rather than the older set.
 	 *
 	 * @param generation the generation the confirmed deploy claimed
 	 * @param dexFile the deployed classes, or null when the build moved no code
@@ -84,7 +86,7 @@ internal class RetainedPayloadStore(
 						addProperty("hasAssets", assetsZip != null)
 					}.toString(),
 			)
-			dir.deleteRecursively()
+			check(purge()) { "could not clear ${dir.absolutePath}" }
 			check(staging.renameTo(dir)) { "could not move staging into ${dir.absolutePath}" }
 		} catch (e: Exception) {
 			staging.deleteRecursively()
@@ -126,10 +128,34 @@ internal class RetainedPayloadStore(
 	 * generation supersedes the retained one but must never be replayed as a hot swap.
 	 */
 	fun clear() {
-		dir.deleteRecursively()
+		if (!purge()) {
+			log.warn("Could not fully drop the retained payload under {}; it is invalidated but its parts remain", dir)
+		}
 		stagingDir().deleteRecursively()
 	}
 
+	/**
+	 * Removes the retained set, invalidating it before its parts go so a failed delete cannot
+	 * leave a readable [META_NAME] behind. [File.deleteRecursively] reports failure by
+	 * returning false and may already have deleted part of the tree, so its result is what
+	 * decides whether the set is gone.
+	 *
+	 * The metadata is emptied rather than unlinked first: unlinking needs write permission on
+	 * [dir] and writing needs it only on the file, so an emptied [META_NAME] still fails [load]
+	 * closed in the case where the recursive delete cannot finish at all. [load] keys off the
+	 * metadata alone, so once it is unreadable the rest of the tree is inert.
+	 *
+	 * @return true when the directory is gone; false when parts of it survive, in which case
+	 *   the set is still unreadable but the caller cannot rename a new one into place
+	 */
+	private fun purge(): Boolean {
+		val meta = File(dir, META_NAME)
+		if (meta.isFile) {
+			runCatching { meta.writeText("") }
+		}
+		return dir.deleteRecursively()
+	}
+
 	/**
 	 * One payload part of the retained set.
 	 *

quickbuild/core/src/test/java/org/appdevforall/cotg/quickbuild/service/deploy/RetainedPayloadStoreTest.kt

@@ -1,6 +1,8 @@
 package org.appdevforall.cotg.quickbuild.service.deploy
 
 import com.google.common.truth.Truth.assertThat
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.Assumptions.assumeTrue
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.io.TempDir
 import java.io.File
@@ -109,4 +111,49 @@ class RetainedPayloadStoreTest {
 
 		assertThat(store.load()).isNull()
 	}
+
+	@Test
+	fun `a retain that cannot delete the old set leaves nothing loadable`() {
+		store.retain(1L, artifact("built.dex", "old-dex"), null, null, """{"gen":1}""")
+		assumeTrue(blockDeletion(), "the filesystem ignored the permission change")
+
+		store.retain(2L, artifact("next.dex", "new-dex"), null, null, """{"gen":2}""")
+
+		// Generation 1's bytes must not outlive a failed generation-2 swap: a re-send would
+		// replay them at a generation the session no longer deploys, leaving the app behind
+		// with nothing left to notice it.
+		assertThat(store.load()).isNull()
+	}
+
+	@Test
+	fun `a clear that cannot delete the set still makes it unloadable`() {
+		store.retain(1L, artifact("built.dex", "old-dex"), null, null, """{"gen":1}""")
+		assumeTrue(blockDeletion(), "the filesystem ignored the permission change")
+
+		store.clear()
+
+		// clear() runs when the baseline changed or a restart deploy landed; either way the
+		// old bytes must never come back as a hot swap.
+		assertThat(store.load()).isNull()
+	}
+
+	/**
+	 * Makes the retention directory's entries impossible to unlink while leaving the entries
+	 * themselves writable - the shape a failed `deleteRecursively` takes, since removing a name
+	 * needs write permission on the directory and rewriting a file's bytes needs it only on the
+	 * file.
+	 *
+	 * @return false when the platform ignored the permission change (a root test runner, or a
+	 *   filesystem without POSIX permissions), in which case the caller must skip
+	 */
+	private fun blockDeletion(): Boolean {
+		val dir = File(workDir, "last-deployed")
+		return dir.setWritable(false) && !dir.canWrite()
+	}
+
+	@AfterEach
+	fun restoreRetentionDirPermissions() {
+		// @TempDir cleanup fails on a directory it cannot empty.
+		File(workDir, "last-deployed").setWritable(true)
+	}
 }