ADFA-4931: stop a re-stage from truncating the distribution's own jars

Code review found that linkDistJars could destroy the on-device Gradle
distribution. createSymbolicLink refuses an existing path, the broad catch
swallowed that, and the copy fallback then opened the stale link for write --
which truncates the distribution's jar, not the daemon's copy. Every Gradle
build on the device breaks, and the stamp records it as success.

Deleting the target before linking removes the class of failure. The rest is
the same review's smaller findings:

- link failures leave no stamp, now covered by a test that goes red when the
  stamp is written first
- names read from dist-linked-jars.txt must be bare file names, the guard
  extractDaemonZip already applies twelve lines up
- a missing list and an empty one now say which one happened
- the copy fallback is reached by injecting the link operation, so the test
  named for a symlink-less filesystem tests one
- the daemon zip's exclusion is scoped to the classpath it is about
- two docs bullets said the daemon re-extracts every provision; it re-extracts
  when the installed APK changed

Tested: 304 app quickbuild tests green, spotlessCheck green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GwtUCGy8fKaSzu2uuUEUPY

Author: fryanpan

app/build.gradle.kts

@@ -8,7 +8,6 @@ import org.adfa.constants.GRADLE_API_NAME_JAR_BR
 import org.adfa.constants.GRADLE_API_NAME_JAR_ZIP
 import org.adfa.constants.GRADLE_DISTRIBUTION_ARCHIVE_NAME
 import org.adfa.constants.GRADLE_DISTRIBUTION_NAME
-import org.adfa.constants.GRADLE_DISTRIBUTION_VERSION
 import org.adfa.constants.KOTLIN_VERSION
 import org.gradle.nativeplatform.platform.internal.DefaultNativePlatform
 import org.json.JSONObject
@@ -626,11 +625,10 @@ val quickBuildDaemonZip =
 		from(daemonProject.tasks.named("daemonJar"))
 		// The daemon jar's manifest Class-Path names these by file name; they must sit
 		// next to the jar after extraction.
-		from(daemonProject.configurations.named("runtimeClasspath"))
 		// ADFA-4931: except the ones the distribution already carries; the stager links those
-		// in at provision time.
+		// in at provision time. Scoped to this spec so it cannot filter the other from()s.
 		val linked = quickBuildDistLinkedJars.toSet()
-		exclude { it.name in linked }
+		from(daemonProject.configurations.named("runtimeClasspath")) { exclude { it.name in linked } }
 		from(quickBuildDistLinkedJarList)
 		// Compose compiler plugin, version-matched to the daemon's compiler; the stable
 		// name is the contract EnvironmentQuickBuildPaths.composeCompilerPlugin reads.

app/src/main/java/com/itsaky/androidide/quickbuild/QuickBuildArtifactStager.kt

@@ -10,6 +10,7 @@ import java.io.FileNotFoundException
 import java.io.IOException
 import java.io.InputStream
 import java.nio.file.Files
+import java.nio.file.Path
 import java.util.zip.ZipInputStream
 
 /**
@@ -148,13 +149,15 @@ object QuickBuildArtifactStager {
 	 * expects them.
 	 *
 	 * @param gradleDists exists so tests can point at a fake distribution.
+	 * @param createLink exists so tests can force the copy fallback.
 	 * @throws FileNotFoundException if a jar or the list is missing, which fails loudly here
 	 *   rather than as a NoClassDefFoundError during the user's first compile.
 	 */
 	@Throws(IOException::class)
 	internal fun linkDistJars(
 		daemonDir: File,
 		gradleDists: File = Environment.GRADLE_DISTS,
+		createLink: (Path, Path) -> Unit = { link, existing -> Files.createSymbolicLink(link, existing) },
 	) {
 		val distLib = File(File(gradleDists, GRADLE_DISTRIBUTION_NAME), "lib")
 		for (jarName in readLinkedJarNames(daemonDir)) {
@@ -167,31 +170,50 @@ object QuickBuildArtifactStager {
 			}
 
 			val target = File(daemonDir, jarName)
+			// Clear the path first: a link an earlier staging left would make the copy fallback
+			// follow it and truncate the distribution's own jar.
+			Files.deleteIfExists(target.toPath())
 			try {
-				Files.createSymbolicLink(target.toPath(), source.toPath())
-			} catch (e: Exception) {
-				// A filesystem without symlinks still needs these bytes, so copy instead.
-				log.warn("Symlink {} -> {} failed ({}), copying", target, source, e.toString())
-				source.inputStream().use { input -> target.outputStream().use { input.copyTo(it) } }
+				createLink(target.toPath(), source.toPath())
+			} catch (e: IOException) {
+				copyInstead(source, target, e)
+			} catch (e: UnsupportedOperationException) {
+				copyInstead(source, target, e)
 			}
 		}
 	}
 
+	/** Copies [source] to [target] when the filesystem refused a link, so the bytes still arrive. */
+	private fun copyInstead(
+		source: File,
+		target: File,
+		cause: Exception,
+	) {
+		log.warn("Symlink {} -> {} failed ({}), copying", target, source, cause.toString())
+		source.inputStream().use { input -> target.outputStream().use { input.copyTo(it) } }
+	}
+
 	/** The jar names the build left out of the zip for the distribution to supply. */
 	private fun readLinkedJarNames(daemonDir: File): List<String> {
 		val list = File(daemonDir, LINKED_JARS_LIST)
-		val names =
-			if (list.isFile) {
-				list.readLines().map(String::trim).filter(String::isNotEmpty)
-			} else {
-				emptyList()
-			}
+		if (!list.isFile) {
+			throw FileNotFoundException(
+				"$LINKED_JARS_LIST is missing from $daemonDir, so the build never wrote it. It " +
+					"lists the jars Quick Build links from the Gradle distribution.",
+			)
+		}
+		val names = list.readLines().map(String::trim).filter(String::isNotEmpty)
 		if (names.isEmpty()) {
 			throw FileNotFoundException(
 				"$LINKED_JARS_LIST at $list names no jars. It lists the jars Quick Build links " +
 					"from the Gradle distribution rather than shipping in the APK.",
 			)
 		}
+		// Each name is used as a path on both sides of the link, so the same boundary
+		// extractDaemonZip guards applies here: anything but a bare file name escapes.
+		names.firstOrNull { File(it).name != it }?.let { name ->
+			throw IOException("Refusing linked jar name that is not a bare file name: $name")
+		}
 		return names
 	}
 }

app/src/test/java/com/itsaky/androidide/quickbuild/QuickBuildArtifactStagerDistLinkTest.kt

@@ -8,6 +8,7 @@ import org.junit.Test
 import org.junit.rules.TemporaryFolder
 import java.io.File
 import java.io.FileNotFoundException
+import java.io.IOException
 import java.nio.file.Files
 
 /**
@@ -84,18 +85,73 @@ class QuickBuildArtifactStagerDistLinkTest {
 	@Test
 	fun `a jar that cannot be linked is copied instead, so the daemon still has the bytes`() {
 		val daemonDir = daemonDirListing()
-		// However a link is refused, the current bytes still have to end up at this path.
-		val occupied = File(daemonDir, jarNames.first()).apply { writeText("stale-bytes") }
+		val refused = jarNames.first()
 
-		QuickBuildArtifactStager.linkDistJars(daemonDir, gradleDistsWith())
+		// Stands in for a filesystem that has no symlinks, which throws rather than returning.
+		QuickBuildArtifactStager.linkDistJars(daemonDir, gradleDistsWith()) { link, existing ->
+			if (link.fileName.toString() == refused) throw UnsupportedOperationException("no symlinks here")
+			Files.createSymbolicLink(link, existing)
+		}
 
-		assertThat(Files.isSymbolicLink(occupied.toPath())).isFalse()
-		assertThat(occupied.readText()).isEqualTo("${jarNames.first()}-bytes")
+		assertThat(Files.isSymbolicLink(File(daemonDir, refused).toPath())).isFalse()
+		assertThat(File(daemonDir, refused).readText()).isEqualTo("$refused-bytes")
 		// The fallback is per jar, so one refused link must not cost the rest theirs.
 		assertThat(Files.isSymbolicLink(File(daemonDir, jarNames.last()).toPath())).isTrue()
 		assertThat(File(daemonDir, jarNames.last()).readText()).isEqualTo("${jarNames.last()}-bytes")
 	}
 
+	@Test
+	fun `a link refused with an IO error is copied too, not just an unsupported one`() {
+		val daemonDir = daemonDirListing()
+		val refused = jarNames.first()
+
+		// Stands in for a filesystem that supports links but refuses this one, such as a
+		// read-only mount or a denied permission.
+		QuickBuildArtifactStager.linkDistJars(daemonDir, gradleDistsWith()) { link, existing ->
+			if (link.fileName.toString() == refused) throw IOException("permission denied")
+			Files.createSymbolicLink(link, existing)
+		}
+
+		assertThat(Files.isSymbolicLink(File(daemonDir, refused).toPath())).isFalse()
+		assertThat(File(daemonDir, refused).readText()).isEqualTo("$refused-bytes")
+	}
+
+	@Test
+	fun `a listed name that is a path rather than a file name is refused`() {
+		// Resolves to a jar that is really there, so only the guard stops the link being made
+		// one directory above the daemon dir.
+		val escaping = "../lib/${jarNames.first()}"
+		val daemonDir = daemonDirListing(names = listOf(escaping))
+
+		// The list is extracted from the daemon zip, which extractDaemonZip already refuses to
+		// let escape; a name used as a path deserves the same guard.
+		val thrown =
+			runCatching { QuickBuildArtifactStager.linkDistJars(daemonDir, gradleDistsWith()) }
+				.exceptionOrNull()
+
+		assertThat(thrown).isInstanceOf(IOException::class.java)
+		assertThat(thrown!!).hasMessageThat().contains("bare file name")
+		assertThat(thrown).hasMessageThat().contains(escaping)
+		assertThat(stagedNames(daemonDir)).isEmpty()
+	}
+
+	@Test
+	fun `staging twice leaves the distribution's own jars intact`() {
+		val daemonDir = daemonDirListing()
+		val dists = gradleDistsWith()
+		QuickBuildArtifactStager.linkDistJars(daemonDir, dists)
+
+		// Linking onto the previous run's links: the copy fallback would otherwise open them for
+		// write, truncating the distribution itself and breaking every Gradle build on the device.
+		QuickBuildArtifactStager.linkDistJars(daemonDir, dists)
+
+		jarNames.forEach { name ->
+			val inDistribution = File(dists, "$GRADLE_DISTRIBUTION_NAME/lib/$name")
+			assertThat(inDistribution.readText()).isEqualTo("$name-bytes")
+			assertThat(File(daemonDir, name).readText()).isEqualTo("$name-bytes")
+		}
+	}
+
 	@Test
 	fun `missing distribution fails loudly and names the path a human has to fix`() {
 		val daemonDir = daemonDirListing()
@@ -116,7 +172,8 @@ class QuickBuildArtifactStagerDistLinkTest {
 	fun `one listed jar missing from the distribution fails the whole staging`() {
 		val daemonDir = daemonDirListing()
 
-		// A partial link is worse than none: the daemon starts and dies on the missing class.
+		// The throw is what protects the caller here: the stamp is left unwritten, so the next
+		// staging wipes the jars this one already linked.
 		val thrown =
 			runCatching {
 				QuickBuildArtifactStager.linkDistJars(daemonDir, gradleDistsWith(jarNames.take(1)))

app/src/test/java/com/itsaky/androidide/quickbuild/QuickBuildArtifactStagerTest.kt

@@ -202,6 +202,25 @@ class QuickBuildArtifactStagerTest {
 		assertThat(jar.exists()).isTrue()
 	}
 
+	@Test
+	fun `a failed link leaves no stamp so the next stage retries`() {
+		val daemonDir = File(tmp.newFolder("home-link-fail"), "daemon")
+		val jar = File(daemonDir, "quickbuild-daemon.jar")
+		val emptyDists = tmp.newFolder("gradle-dists-empty")
+
+		// Stamping a failed link would serve a daemon whose classpath names jars that are not
+		// there, and the early return would never stage again.
+		val thrown =
+			runCatching {
+				QuickBuildArtifactStager.stageDaemonIfNeeded("7:1000", daemonDir, jar, emptyDists) { daemonZip() }
+			}.exceptionOrNull()
+		assertThat(thrown).isInstanceOf(FileNotFoundException::class.java)
+		assertThat(File(daemonDir, QuickBuildArtifactStager.DAEMON_STAMP_FILE).exists()).isFalse()
+
+		val ran = QuickBuildArtifactStager.stageDaemonIfNeeded("7:1000", daemonDir, jar, gradleDists) { daemonZip() }
+		assertThat(ran).isTrue()
+	}
+
 	@Test
 	fun `a failed extraction leaves no stamp so the next stage retries`() {
 		val daemonDir = File(tmp.newFolder("home"), "daemon")

quickbuild/docs/debugging.md

@@ -179,7 +179,7 @@ to CoGo.
 | Path | What | How to read it |
 | --- | --- | --- |
 | `files/home/.cg/quickbuild/quickbuild-runtime.aar` | staged runtime AAR, re-staged every provision | `run-as com.itsaky.androidide` |
-| `files/home/.cg/quickbuild/daemon/` | daemon jar, its full runtime classpath, `compose-compiler-plugin.jar`; deleted and re-extracted every provision. The jars named in `dist-linked-jars.txt`, the Kotlin compiler among them, are symlinks into the Gradle distribution's `lib/` rather than extracted files - `ls -l` here, since a broken one surfaces as a `NoClassDefFoundError` on the first compile | `run-as com.itsaky.androidide` |
+| `files/home/.cg/quickbuild/daemon/` | daemon jar, its full runtime classpath, `compose-compiler-plugin.jar`; deleted and re-extracted when the installed APK changed. The jars named in `dist-linked-jars.txt`, the Kotlin compiler among them, are symlinks into the Gradle distribution's `lib/` rather than extracted files - `ls -l` here, since a broken one surfaces as a `NoClassDefFoundError` on the first compile | `run-as com.itsaky.androidide` |
 | `files/home/.cg/quickbuild/bench-events.jsonl` | session event log; bench flag only | `run-as com.itsaky.androidide` |
 | `no_backup/quickbuild-scratch/<name>-<16 hex>/work` | executor payload staging | `run-as com.itsaky.androidide`; **deleted on teardown** |
 | `no_backup/quickbuild-scratch/<name>-<16 hex>/out` | daemon output: classes, dex, relinked resources | as above |

quickbuild/docs/pipeline.md

@@ -356,7 +356,7 @@ flowchart TB
 
 Step 7's rebuild fallback re-enters the same runner through `rebuildProxyApp`.
 
-1. [QuickBuildArtifactStager](../../app/src/main/java/com/itsaky/androidide/quickbuild/QuickBuildArtifactStager.kt) extracts the runtime AAR and the daemon zip from CoGo's APK assets into `<ANDROIDIDE_HOME>/quickbuild/`. The zip leaves out the eight jars the on-device Gradle distribution already carries, the Kotlin compiler among them; the stager reads their names from `dist-linked-jars.txt` and symlinks the distribution's copies in beside the daemon jar, copying instead if the filesystem refuses a symlink. It re-extracts on **every** provision on purpose: a version-keyed marker serves a stale bundle when content changes without a version bump.
+1. [QuickBuildArtifactStager](../../app/src/main/java/com/itsaky/androidide/quickbuild/QuickBuildArtifactStager.kt) extracts the runtime AAR and the daemon zip from CoGo's APK assets into `<ANDROIDIDE_HOME>/quickbuild/`. The zip leaves out the eight jars the on-device Gradle distribution already carries, the Kotlin compiler among them; the stager reads their names from `dist-linked-jars.txt` and symlinks the distribution's copies in beside the daemon jar, copying instead if the filesystem refuses a symlink. It re-extracts when the installed APK changed, which a stamp file beside the daemon jar records.
 2. [GradleQuickBuildProvisioner](../../app/src/main/java/com/itsaky/androidide/quickbuild/GradleQuickBuildProvisioner.kt) runs the proxy app build through CoGo's existing `BuildService.executeTasks`, then parses `setup.json` into [ProxyAppInfo](../core/src/main/java/org/appdevforall/cotg/quickbuild/data/ProxyAppInfo.kt).
 3. [QuickBuildScratch](../core/src/main/java/org/appdevforall/cotg/quickbuild/data/QuickBuildScratch.kt) creates `noBackupFilesDir/quickbuild-scratch/<projectKey>/{work,out}` and enforces the free-space floor. It must stay on app-private storage - see the README's "Build Scratch Lives in Faster Private Storage".
 4. [QuickBuildClobberCheck](../core/src/main/java/org/appdevforall/cotg/quickbuild/service/provision/QuickBuildClobberCheck.kt) plus [RealIdInstall](../core/src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/RealIdInstall.kt) read the installed package's `android:appComponentFactory` to decide whether the tap clobbers a Standard Run install. Stateless: an install or uninstall outside CoGo cannot leave it stale.