ADFA-4128: 0902 review round on the gradle-plugin

Akash's 2 September round on proxy app generation.

- A dependency's own <service> is no longer recorded as a manifest component.
  Its class ships in the base APK dex and no payload redefines it, so a live
  instance cannot go stale - but recorded, it is restart-sensitive and the
  deploy policy restarts the process on every code-bearing deploy. androidx.work
  alone declares three non-final services, which is hot reload never happening
  again, silently. Pinned by a test that fails without the ownership test.
  #1722 (comment)
- The gen-0 proxy javac passes --release 17, the daemon's own target, so a
  Gradle JVM above 17 cannot put two bytecode levels in one payload dex.
  #1722 (comment)
- The payload-classes dir is a directory input, not its path as a string: the
  report reads the tree, so tracking the path alone left it up to date after the
  classes underneath changed.
  #1722 (comment)
- The APK fallback for unreadable build metadata requires exactly one APK and
  names them all when there are more, instead of picking an arbitrary one - the
  same choice selectUniversalApk makes a level up.
  #1722 (comment)
- ComponentInfo's reader-side KDoc catches up with the writer: proxyClass is
  also null for an activity the build could not proxy, and launcher is false for
  a skipped launcher activity.
  #1722 (comment)
- The design doc's alias section names all three copied attributes and says what
  dropping each would cost; the test that pins them is renamed to match.
  #1722 (comment)
  #1722 (comment)
- One home for the by-name skip list: UNPROXIABLE_BY_NAME. Neither doc carries a
  count that can drift from it.
  #1722 (comment)
- A test env that names no repository fails loudly rather than adding none and
  surfacing later as an unresolved dependency.
  #1722 (comment)
- The "no LAUNCHER activity" warning says what it actually means: either the app
  declares none, or it declares one on an <activity-alias>, which is a supported
  pattern with a test of its own.
  #1722 (comment)
- A companion object no longer sits between a KDoc and the function it
  documents.
  #1722 (comment)
- tasks.register in place of the deprecated tasks.create.
  #1722 (comment)
- The parity test's KDoc claims what the test catches - a one-sided source edit,
  both sides run against the daemon's ASM - not production byte parity.
  #1722 (comment)

Not fixed here: the PR description's always-on-files claim and its stale
test-evidence block, and the six @disabled reasons. All three are description
or ticket work, drafted with this round's replies.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

Author: fryanpan

gradle-plugin/src/main/java/com/itsaky/androidide/gradle/COTGSettingsPlugin.kt

@@ -32,9 +32,18 @@ class COTGSettingsPlugin : Plugin<Settings> {
 		val (isTestEnv, mavenLocalRepos) = getTestEnvProps(target.startParameter)
 
 		// The bundled repo lives at a device-only path, so a host test env supplies its own
-		// repos instead - requiring the device path there would fail every host build.
+		// repos instead - requiring the device path there would fail every host build. A test
+		// env that supplies none is a misconfiguration, and adding zero repositories silently
+		// turns it into an unresolved-dependency failure somewhere far from the cause.
 		val allLocalRepos =
-			if (isTestEnv) mavenLocalRepos else listOf(MAVEN_LOCAL_REPOSITORY)
+			if (isTestEnv) {
+				require(mavenLocalRepos.isNotEmpty()) {
+					"$_PROPERTY_IS_TEST_ENV is set but $_PROPERTY_MAVEN_LOCAL_REPOSITORY names no repository"
+				}
+				mavenLocalRepos
+			} else {
+				listOf(MAVEN_LOCAL_REPOSITORY)
+			}
 
 		target.addLocalRepos(allLocalRepos)
 	}

gradle-plugin/src/main/java/com/itsaky/androidide/gradle/QuickBuildPlugin.kt

@@ -301,9 +301,7 @@ class QuickBuildPlugin : Plugin<Project> {
 				task.transformedManifestPath.set(
 					generate.flatMap { it.updatedManifest }.map { it.asFile.absolutePath },
 				)
-				task.payloadClassesPath.set(
-					divert.flatMap { it.payloadClasses }.map { it.asFile.absolutePath },
-				)
+				task.payloadClasses.set(divert.flatMap { it.payloadClasses })
 				// Provider, not a plain value: finalizeDsl (which computes the flag) runs
 				// during configuration, but reading here at task-config time could race it.
 				task.composeEnabled.set(project.provider { composeEnabled() })

gradle-plugin/src/main/java/com/itsaky/androidide/gradle/quickbuild/ComponentProxiabilityResolver.kt

@@ -67,6 +67,18 @@ class ComponentProxiabilityResolver(
 		}
 	}
 
+	/**
+	 * Whether a dependency owns [userClass], as far as the configured classpath can tell.
+	 *
+	 * Absence is not proof of project ownership - [byNameOnly] finds nothing at all, and the
+	 * dependency-artifact view is narrower than the compile classpath - so a caller must treat
+	 * false as "assume the project owns it" and only act on true.
+	 *
+	 * @param userClass the component's implementation class, as a dotted binary name.
+	 * @return true when the class was found on the dependency classpath.
+	 */
+	fun isLibraryOwned(userClass: String): Boolean = libraryClassBytes(userClass) != null
+
 	/**
 	 * [resolve], except that a class the project itself compiled is always proxiable.
 	 *

gradle-plugin/src/main/java/com/itsaky/androidide/gradle/quickbuild/QuickBuildManifestTransformer.kt

@@ -338,8 +338,10 @@ class QuickBuildManifestTransformer(
 	 * @param type the kind to rewrite; its [ComponentType.jsonName] is also the manifest tag.
 	 * @param manifestPackage the manifest's package, for expanding android:name shorthand.
 	 * @param unproxied accumulator for components [skipProxy] rejects; a rejected component is
-	 *   also left out of the returned list, keeping library-owned components (whose classes
-	 *   never travel in the payload) invisible to the deploy policy's restart rule.
+	 *   also left out of the returned list. So is an unproxied component a dependency owns:
+	 *   neither's class travels in the payload, and the deploy policy's restart rule must not
+	 *   see either. A proxied kind is recorded whoever owns the user class, because the
+	 *   generated proxy that extends it does travel.
 	 * @param unsupportedAttribute an android attribute the proxy app cannot host when it is
 	 *   `"true"` (a service's isolatedProcess, a provider's multiprocess), or null for a kind
 	 *   with none.
@@ -377,6 +379,15 @@ class QuickBuildManifestTransformer(
 				// applicationComponent: the runtime resolves this name against the payload
 				// dex, so shorthand left verbatim is fragile.
 				element.setAttributeNS(ANDROID_NS, "android:name", userClass)
+				if (proxiability.isLibraryOwned(userClass)) {
+					// A dependency's own <service> is not recorded, for the same reason as the
+					// components CoGo injects: its class ships in the base APK dex and no
+					// payload ever redefines it, so a live instance of it cannot go stale.
+					// Recorded, it would make the deploy policy restart the process on every
+					// code-bearing deploy - androidx.work alone declares three non-final
+					// services, which is hot reload never happening again, silently.
+					return@mapIndexedNotNull null
+				}
 				return@mapIndexedNotNull ProxiedComponent(
 					type = type,
 					userClass = userClass,

gradle-plugin/src/main/java/com/itsaky/androidide/gradle/quickbuild/QuickBuildTasks.kt

@@ -126,7 +126,13 @@ abstract class QuickBuildGenerateSourcesTask : DefaultTask() {
 			.writeText(QuickBuildJson.manifestInfoJson(info))
 
 		if (result.entryActivity == null) {
-			logger.warn("Quick Build: no LAUNCHER activity found in the merged manifest")
+			// Not necessarily missing: a MAIN/LAUNCHER filter on an <activity-alias> whose
+			// target activity carries none is a supported pattern, and leaves no proxied
+			// entry activity to name. The relaunch then uses the package's default intent.
+			logger.warn(
+				"Quick Build: no LAUNCHER activity to relaunch into; the app either declares " +
+					"none, or declares it on an <activity-alias>",
+			)
 		}
 		result.unproxied.forEach { skipped ->
 			// Lifecycle, not info: someone debugging a stale-code report needs to see a
@@ -480,6 +486,12 @@ abstract class QuickBuildPayloadDexTask : DefaultTask() {
 				listOf(
 					"-proc:none",
 					"-nowarn",
+					// The same release the daemon pins its own javac to
+					// (IncrementalCompiler.JVM_TARGET, a separate artifact). These proxies are
+					// bundled into every payload dex beside daemon-compiled classes, and a
+					// Gradle JVM above 17 would put two bytecode levels in one payload.
+					"--release",
+					"17",
 					"-classpath",
 					classpath.joinToString(File.pathSeparator) { it.absolutePath },
 					"-d",
@@ -549,9 +561,15 @@ abstract class QuickBuildProxyAppReportTask : DefaultTask() {
 	@get:Input
 	abstract val transformedManifestPath: Property<String>
 
-	/** The divert task's payload-classes dir; its jars/ carry R.jar and kin. */
-	@get:Input
-	abstract val payloadClassesPath: Property<String>
+	/**
+	 * The divert task's payload-classes dir; its jars/ carry R.jar and kin.
+	 *
+	 * A directory input, not the path as a string: the report reads the tree itself (the jar
+	 * list, and the class headers the supertype index is built from), so tracking only the
+	 * path leaves the report up to date after the classes underneath it change.
+	 */
+	@get:InputDirectory
+	abstract val payloadClasses: DirectoryProperty
 
 	/** True when the project uses Compose; the daemon then adds its compiler plugin. */
 	@get:Input
@@ -658,18 +676,10 @@ abstract class QuickBuildProxyAppReportTask : DefaultTask() {
 				?.elements
 				?.takeIf { it.isNotEmpty() }
 				?.let { selectUniversalApk(it, apkDirectory.get().asFile) }
-				?: apkDirectory
-					.get()
-					.asFile
-					.walkTopDown()
-					.firstOrNull { it.extension == "apk" }
-					?.absolutePath
-				?: throw GradleException(
-					"Quick Build: no APK found under '${apkDirectory.get().asFile}'",
-				)
+				?: soleApkUnder(apkDirectory.get().asFile)
 
 		val outFile = reportFile.get().asFile.apply { parentFile.mkdirs() }
-		val payloadClassesRoot = File(payloadClassesPath.get())
+		val payloadClassesRoot = payloadClasses.get().asFile
 		val payloadJars =
 			File(payloadClassesRoot, "jars")
 				.listFiles { file -> file.extension == "jar" }
@@ -738,15 +748,39 @@ abstract class QuickBuildProxyAppReportTask : DefaultTask() {
 			?.firstOrNull { it.isFile && it.name == "stableIds.txt" }
 
 	/**
-	 * Collects every pre-compiled `.flat` unit a relink needs to resolve a dependency's
-	 * resources: [mergedResSearchDir]'s closure plus [dependencyResourceDirs]' FILE-based units.
+	 * The one APK under [apkDirectory], for a build whose artifact metadata could not be read.
 	 *
-	 * Sorted for determinism only. The two sets never declare the same resource, and layering the
-	 * relink's own fresh compile on top of both is `Aapt2Link`'s job, not this task's.
+	 * Without the metadata nothing here can tell a split from the universal APK, so the only
+	 * safe answer is that there must be exactly one candidate. Picking one of several was the
+	 * same arbitrary choice [selectUniversalApk] exists to prevent, one level down.
 	 *
-	 * @return absolute paths of every `.flat` unit found, sorted; empty when neither source
-	 *   exists, which the caller logs rather than treating as an error.
+	 * @param apkDirectory the variant's APK output directory.
+	 * @return the single APK's absolute path.
+	 * @throws GradleException when the directory holds no APK, or more than one.
 	 */
+	private fun soleApkUnder(apkDirectory: File): String {
+		val apks = apkDirectory.walkTopDown().filter { it.extension == "apk" }.toList()
+		return when (apks.size) {
+			1 -> {
+				apks.single().absolutePath
+			}
+
+			0 -> {
+				throw GradleException("Quick Build: no APK found under '$apkDirectory'")
+			}
+
+			else -> {
+				throw GradleException(
+					"Quick Build: '$apkDirectory' holds ${apks.size} APKs (" +
+						apks.map { it.name }.sorted().joinToString(", ") +
+						") and their build metadata could not be read, so the universal one " +
+						"cannot be identified; disable APK splits for this variant or use a " +
+						"Standard Run",
+				)
+			}
+		}
+	}
+
 	companion object {
 		/**
 		 * Picks the one APK every device can install out of AGP's built-artifact metadata.
@@ -780,6 +814,16 @@ abstract class QuickBuildProxyAppReportTask : DefaultTask() {
 				)
 	}
 
+	/**
+	 * Collects every pre-compiled `.flat` unit a relink needs to resolve a dependency's
+	 * resources: [mergedResSearchDir]'s closure plus [dependencyResourceDirs]' FILE-based units.
+	 *
+	 * Sorted for determinism only. The two sets never declare the same resource, and layering the
+	 * relink's own fresh compile on top of both is `Aapt2Link`'s job, not this task's.
+	 *
+	 * @return absolute paths of every `.flat` unit found, sorted; empty when neither source
+	 *   exists, which the caller logs rather than treating as an error.
+	 */
 	private fun collectLibraryResourcePaths(): List<String> {
 		val mergedRes =
 			mergedResSearchDir.orNull

gradle-plugin/src/test/java/com/itsaky/androidide/gradle/quickbuild/QuickBuildManifestTransformerTest.kt

@@ -240,7 +240,7 @@ class QuickBuildManifestTransformerTest {
 	}
 
 	@Test
-	fun `the synthetic alias mirrors its target's exported value, never widening it`() {
+	fun `the synthetic alias mirrors its target's exported, permission and enabled, never widening them`() {
 		// The alias is the only manifest entry left under the real class name, so forcing it
 		// false rejects a launch the standard run allows: a pinned shortcut or share target the
 		// app published records the real name, and the launcher is a different uid. Mirroring is
@@ -492,6 +492,56 @@ class QuickBuildManifestTransformerTest {
 			.isEqualTo("com.itsaky.androidide.logsender.LogSenderService")
 	}
 
+	/**
+	 * A transformer whose dependency classpath holds [libraryClasses] as ordinary, non-final
+	 * library classes - the androidx.work shape, where the class is real, extendable, and not
+	 * the project's.
+	 */
+	private fun transformerSeeingLibrary(vararg libraryClasses: String): QuickBuildManifestTransformer {
+		val byName =
+			libraryClasses.associateWith { name ->
+				ClassWriter(0)
+					.apply {
+						visit(Opcodes.V11, Opcodes.ACC_PUBLIC, name.replace('.', '/'), null, "java/lang/Object", null)
+						visitEnd()
+					}.toByteArray()
+			}
+		return QuickBuildManifestTransformer(
+			proxyPackage,
+			factory,
+			proxiability = ComponentProxiabilityResolver { byName[it] },
+		)
+	}
+
+	@Test
+	fun `a dependency's own service is not recorded, so it cannot force a restart`() {
+		// androidx.work declares three non-final services. Their classes ship in the base APK
+		// dex and no payload redefines them, so a live instance cannot go stale - but recorded,
+		// each is a restart-sensitive component and the deploy policy restarts the process on
+		// every code-bearing deploy. Hot reload then never happens again, silently.
+		val transformer = transformerSeeingLibrary("androidx.work.impl.foreground.SystemForegroundService")
+
+		val result =
+			transformer.transform(
+				manifest(
+					launcherActivity +
+						"""
+						<service android:name="androidx.work.impl.foreground.SystemForegroundService" />
+						<service android:name="com.example.app.SyncService" />
+						""".trimIndent(),
+				).byteInputStream(),
+			)
+
+		assertThat(result.components.filter { it.type == ComponentType.SERVICE }.map { it.userClass })
+			.containsExactly("com.example.app.SyncService")
+		// The manifest entry itself stays: the app still declares the library's service.
+		assertThat(componentNames(result, "service"))
+			.containsExactly(
+				"androidx.work.impl.foreground.SystemForegroundService",
+				"com.example.app.SyncService",
+			)
+	}
+
 	@Test
 	fun `services keep their real manifest names so explicit start-service intents still resolve`() {
 		// Android has no <service> alias, so a renamed service silently breaks

gradle-plugin/src/test/java/com/itsaky/androidide/gradle/quickbuild/QuickBuildPayloadTransformTaskTest.kt

@@ -38,7 +38,7 @@ class QuickBuildPayloadTransformTaskTest {
 		// pipeline produced nothing (or a cleaned intermediate) must not fail the divert when
 		// the sibling walkTopDown path already tolerates exactly that absence.
 		val project = ProjectBuilder.builder().withProjectDir(tempDir).build()
-		val task = project.tasks.create("qbDivert", QuickBuildPayloadTransformTask::class.java)
+		val task = project.tasks.register("qbDivert", QuickBuildPayloadTransformTask::class.java).get()
 
 		writeJar(File(tempDir, "present.jar"), "com/example/Foo.class", "com/example/R\$string.class")
 		val presentDir = File(tempDir, "classes").apply { mkdirs() }

quickbuild/README.md

@@ -98,7 +98,7 @@ How `:quickbuild:core` gets a compiled change into the running app - the step th
 
 ### Proxy-App Architecture
 
-What gets proxied: every **manifest-declared, proxiable** activity and provider gets a generated `Proxy<N><Type> extends <user class>` compiled into the APK - `final` library components and three name-resolved ones are skipped (see the exceptions below). Services, receivers, and the `Application` keep the user's class name - explicit intents address services and receivers by that real name and Android has no alias to compensate a rename with, while the `AppComponentFactory` instantiates whatever the manifest names through the payload loader anyway. Runtime-registered receivers are ordinary objects needing nothing.
+What gets proxied: every **manifest-declared, proxiable** activity and provider gets a generated `Proxy<N><Type> extends <user class>` compiled into the APK - `final` library components and the name-resolved ones are skipped (see the exceptions below). Services, receivers, and the `Application` keep the user's class name - explicit intents address services and receivers by that real name and Android has no alias to compensate a rename with, while the `AppComponentFactory` instantiates whatever the manifest names through the payload loader anyway. Runtime-registered receivers are ordinary objects needing nothing.
 
 What the installed proxy app is made of:
 

quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/ComponentInfo.kt

@@ -80,12 +80,14 @@ fun ComponentInfo.isRestartSensitive(): Boolean = kind in RESTART_SENSITIVE_KIND
  *
  * @property kind which manifest tag declared it, which is what decides restart vs recreate.
  * @property className the USER class FQN declared in the source manifest.
- * @property proxyClass the generated proxy FQN carried in the transformed manifest;
- *   null for the Application, service and receiver entries, which keep the user's real
- *   name (services/receivers are addressed by it via explicit intents, and the
- *   appComponentFactory instantiates the manifest name through the payload loader).
- * @property launcher true for the launcher activity - its [proxyClass] is the explicit
- *   relaunch target after a restart-deploy.
+ * @property proxyClass the generated proxy FQN carried in the transformed manifest; null for
+ *   the Application, service and receiver entries, which keep the user's real name
+ *   (services/receivers are addressed by it via explicit intents, and the appComponentFactory
+ *   instantiates the manifest name through the payload loader), and null for an activity the
+ *   build could not proxy, which also keeps its real name.
+ * @property launcher true for the launcher activity when it was proxied - its [proxyClass] is
+ *   then the explicit relaunch target after a restart-deploy. False for a launcher activity the
+ *   build skipped, which has no proxy to relaunch into.
  * @property supertypes the user-side (project-compiled) superclass chain recorded from
  *   class headers at proxy app build time. Carried but currently unread: [DeployPolicy]
  *   keeps no supertype index. Kept for a planned deploy mode that redefines classes in

quickbuild/daemon/src/test/kotlin/org/appdevforall/cotg/quickbuild/daemon/dex/FinalStripperClassOpenerParityTest.kt

@@ -9,16 +9,25 @@ import java.nio.file.Files
 import javax.tools.ToolProvider
 
 /**
- * Pins the daemon's [FinalStripper] to the proxy build's ClassOpener byte for byte.
+ * Catches a one-sided source edit between the daemon's [FinalStripper] and the proxy build's
+ * ClassOpener, which are deliberate duplicates - a shared module would drag a second jar into
+ * the gradle-plugin's flatDir init-script bundle.
  *
- * The two are deliberate duplicates - a shared module would drag a second jar into the
- * gradle-plugin's flatDir init-script bundle - and the dex verifier depends on their outputs
- * agreeing: the gen-0 baseline carries ClassOpener's bytes and every hot recompile carries
- * FinalStripper's. A one-sided edit otherwise surfaces on device as a verifier rejection;
- * this test makes it fail at build time instead.
+ * It does NOT pin the bytes the two produce in production. ASM is `compileOnly` on both sides
+ * and not exported, so this test runs both transforms against the daemon's ASM; in a real build
+ * each side gets its own. What it does pin is that the two sources still agree when handed the
+ * same ASM, which is where a one-sided edit shows up. The dex verifier depends on their outputs
+ * agreeing - the gen-0 baseline carries ClassOpener's bytes and every hot recompile carries
+ * FinalStripper's - and an edit to one alone otherwise surfaces on device as a verifier
+ * rejection.
  */
 class FinalStripperClassOpenerParityTest {
-	/** Compiles one fixture per shape the transform distinguishes; returns all four .class files. */
+	/**
+	 * Compiles one fixture per shape the transform distinguishes.
+	 *
+	 * @return every `.class` produced, sorted by name: three top-level classes, plus the nested
+	 *   fixture's inner class, which javac emits as its own file.
+	 */
 	private fun compileFixtures(): List<File> {
 		val dir = Files.createTempDirectory("stripper-parity").toFile()
 		File(dir, "FinalFixture.java")
@@ -36,7 +45,7 @@ class FinalStripperClassOpenerParityTest {
 	@Test
 	fun `both transforms produce identical bytes over the same classes`() {
 		val classFiles = compileFixtures()
-		// Final, open, final-with-final-method, and the nested pair's two files.
+		// FinalFixture, OpenFixture, and the nested pair's two files (Nested, Nested$Inner).
 		assertThat(classFiles).hasSize(4)
 		for (classFile in classFiles) {
 			val bytes = classFile.readBytes()