ADFA-4128: record the retained set's invalidation where a read-only dir cannot swallow it

purge() emptied meta.json inside a runCatching and then returned the delete's
result, so a store whose metadata write AND recursive delete both failed left
meta.json intact and readable. load() parsed it and handed a superseded
baseline's bytes to a reconnect re-send - the one thing this store exists not to
do. Emptying the metadata and unlinking it need different permissions, which is
why the class treated them as independent, but a read-only meta.json under a
read-only directory refuses both at once.

The invalidation is now recorded beside the directory when neither half took, and
load() refuses a set carrying that marker. The marker needs only the parent, which
retain() must be able to write anyway to rename its staging dir into place, and it
is cleared as soon as a purge removes the set.

Without the fix the new test reports:
  value of: load()
  expected: null
  but was : RetainedPayload(generation=1, metadataJson={"gen":1}, ...)

Also corrects the purge() KDoc, which claimed a failed delete always leaves the
set unreadable (board task t-jwpRC46uvGrH). It now states the one case that is
still only logged - a store whose whole tree has gone read-only, where there is
nowhere left to record anything - and clear()'s warning no longer asserts an
invalidation it cannot guarantee.

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

@@ -54,7 +54,8 @@ internal class RetainedPayloadStore(
 	 * 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. 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.
+	 * so this deploy failing to retain leaves nothing to load rather than the older set - as
+	 * far as [purge] can still record it, which its own contract bounds.
 	 *
 	 * @param generation the generation the confirmed deploy claimed
 	 * @param dexFile the deployed classes, or null when the build moved no code
@@ -105,6 +106,10 @@ internal class RetainedPayloadStore(
 	 *   (missing part, corrupt metadata) - either way the caller falls back to rebuilding
 	 */
 	fun load(): RetainedPayload? {
+		if (invalidMarker().isFile) {
+			log.warn("Retained payload under {} could not be invalidated in place; refusing it", dir)
+			return null
+		}
 		val meta = File(dir, META_NAME)
 		if (!meta.isFile) return null
 		return try {
@@ -129,7 +134,7 @@ internal class RetainedPayloadStore(
 	 */
 	fun clear() {
 		if (!purge()) {
-			log.warn("Could not fully drop the retained payload under {}; it is invalidated but its parts remain", dir)
+			log.warn("Could not fully drop the retained payload under {}; its parts remain", dir)
 		}
 		stagingDir().deleteRecursively()
 	}
@@ -145,17 +150,38 @@ internal class RetainedPayloadStore(
 	 * 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
+	 * Both can fail together, though - a read-only [META_NAME] under a read-only [dir] refuses
+	 * the write and the unlink alike - and then nothing inside [dir] has changed, so [load]
+	 * would parse the superseded metadata and re-send a baseline the session has moved past.
+	 * The marker beside [dir] is the third place to record it: it needs only the parent, which
+	 * [retain] must be able to write anyway to rename its staging dir into place.
+	 *
+	 * @return true when the directory is gone; false when parts of it survive, in which case the
+	 *   caller cannot rename a new set into place. A false still leaves the old set unreadable -
+	 *   by the emptied [META_NAME], or by the marker when that write was refused too - except
+	 *   where neither could be written at all, which is a store whose whole tree has gone
+	 *   read-only and which [clear] can then only log
 	 */
 	private fun purge(): Boolean {
 		val meta = File(dir, META_NAME)
-		if (meta.isFile) {
-			runCatching { meta.writeText("") }
+		val invalidated = !meta.isFile || runCatching { meta.writeText("") }.isSuccess
+		if (!invalidated) {
+			runCatching { invalidMarker().writeText(dir.name) }
 		}
-		return dir.deleteRecursively()
+		if (!dir.deleteRecursively()) return false
+		// The set is gone, so the marker has nothing left to refuse; leaving it would refuse
+		// whatever the next retain puts here.
+		invalidMarker().delete()
+		return true
 	}
 
+	/**
+	 * The marker that makes [load] refuse a retained set [purge] could not invalidate in place.
+	 * It sits beside [dir] rather than inside it, because the case it exists for is [dir] being
+	 * unwritable.
+	 */
+	private fun invalidMarker(): File = File(dir.parentFile, dir.name + ".invalid")
+
 	/**
 	 * One payload part of the retained set.
 	 *

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

@@ -137,6 +137,20 @@ class RetainedPayloadStoreTest {
 		assertThat(store.load()).isNull()
 	}
 
+	@Test
+	fun `a set that can be neither emptied nor deleted is refused rather than re-sent`() {
+		store.retain(1L, artifact("built.dex", "old-dex"), null, null, """{"gen":1}""")
+		assumeTrue(blockMetadataWrite(), "the filesystem ignored the permission change")
+		assumeTrue(blockDeletion(), "the filesystem ignored the permission change")
+
+		store.clear()
+
+		// Both halves of the in-place invalidation are refused here, so nothing inside the
+		// retention directory changed at all. Reading the untouched metadata back would
+		// re-send generation 1 onto a baseline the session has already moved past.
+		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
@@ -151,9 +165,23 @@ class RetainedPayloadStoreTest {
 		return dir.setWritable(false) && !dir.canWrite()
 	}
 
+	/**
+	 * Makes the metadata impossible to rewrite - the other half of the invalidation, which fails
+	 * independently of [blockDeletion] because it turns on the file's own mode rather than the
+	 * directory's.
+	 *
+	 * @return false when the platform ignored the permission change, in which case the caller
+	 *   must skip
+	 */
+	private fun blockMetadataWrite(): Boolean {
+		val meta = File(File(workDir, "last-deployed"), "meta.json")
+		return meta.setWritable(false) && !meta.canWrite()
+	}
+
 	@AfterEach
 	fun restoreRetentionDirPermissions() {
 		// @TempDir cleanup fails on a directory it cannot empty.
 		File(workDir, "last-deployed").setWritable(true)
+		File(File(workDir, "last-deployed"), "meta.json").setWritable(true)
 	}
 }