ADFA-4128: qb 11 review fixes — install rotation window, flag-off intent, test gaps

Important 1 (daemon idle-timeout/Metaspace tuner un-gated): kept un-gated by
design — the 384m Metaspace floor fixes real OOM-killed builds and the tiered
idle timeouts keep low-RAM devices from losing the IDE to lmkd; GradleBuildTuner
now states this in its KDoc. Ships flag-off; needs Bryan sign-off in the PR body.

Important 2 (generateSources narrowing un-gated): judged a genuine all-users
improvement, not QB-specific — the old code ran a Gradle generateSources after
EVERY save-all (and after any XML save in SaveFileAction), a per-save build tax;
flag-off the deferral degenerates to the same immediate call, so the narrowing
is the only behavior change. Known trade (manifest-only edits leave generated
Manifest/R intermediates stale until the next resource save or build) now
stated at both call sites. Ships flag-off; needs sign-off in the PR body.

Important 3 (install dropped on rotation, flag on): installApk's async path now
re-arms AwaitingInstall (BuildViewModel.reArmInstall, fires only from Idle)
from the coroutine's drop path, so a configuration change during the APK-manifest
parse makes the recreated activity's collector retry the install instead of
silently losing a successful build. Covered by BuildViewModelInstallReArmTest.

Test gap (zip-slip guard): extraction loop extracted to
QuickBuildArtifactStager.extractDaemonZip(InputStream, File); the guard is
watched going red by QuickBuildArtifactStagerTest (a ../ entry throws and
nothing lands outside the daemon dir).

Test gap (InstallationEventFlow mapping): InstallationEventFlowTest pins the
PackageInstaller status mapping, including the ABORTED-vs-FAILURE branch order
and the no-extras / no-status paths.

Test gap (service-side output capture): suppress/capture/drain routing
extracted from GradleBuildService.logOutput into InternalBuildOutputCapture;
bounded tail, drain-clears, throwing progress listener and editor-listener
routing pinned by InternalBuildOutputCaptureTest.

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

Author: fryanpan

app/src/main/java/com/itsaky/androidide/actions/file/SaveFileAction.kt

@@ -99,6 +99,10 @@ class SaveFileAction(
 			val saveResult = result.result
 			// Only a resource save can change R, so only it warrants the Gradle generateSources run
 			// (Java R.jar freshness + ViewBinding accessors - see SaveResult.resourceXmlSaved).
+			// Deliberately un-gated (experiments flag off included): previously ANY XML save
+			// triggered this, so skipping it on non-resource XML is a save-latency win for every
+			// user. Known trade: a manifest-only edit no longer refreshes the generated Manifest/R
+			// intermediates until the next resource save or build.
 			// Routed through the deferral: immediate with no Quick Build session, parked and
 			// coalesced until the session pipeline settles with one (see GenerateSourcesDeferral).
 			if (saveResult.resourceXmlSaved) {

app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt

@@ -1002,6 +1002,10 @@ open class EditorHandlerActivity :
 
 		// Only a resource save can change R, so only it warrants the Gradle generateSources run
 		// (Java R.jar freshness + ViewBinding accessors - see SaveResult.resourceXmlSaved).
+		// Deliberately un-gated (experiments flag off included): previously this ran after EVERY
+		// save here, so skipping it on Kotlin/Java and non-resource saves is a save-latency win
+		// for every user. Known trade: a manifest-only edit no longer refreshes the generated
+		// Manifest/R intermediates until the next resource save or build.
 		// Routed through the deferral: immediate with no Quick Build session, parked and
 		// coalesced until the session pipeline settles with one (see GenerateSourcesDeferral).
 		if (processResources && result.resourceXmlSaved) {

app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt

@@ -535,37 +535,50 @@ abstract class ProjectHandlerActivity : BaseEditorActivity() {
 		}
 		val answerAtTap = buildViewModel.consumeClobberAnswerAtTap()
 		lifecycleScope.launch {
-			// Reading the APK's manifest is disk work, and on emulated storage that is not free.
-			val apkApplicationId = withContext(Dispatchers.IO) { apkApplicationId(state.apkFile) }
-			if (isDestroyed || isFinishing) {
-				return@launch
-			}
-			val now =
-				quickBuildClobberConfirmation(apkApplicationId, clobberCheck::standardRunNeedsConfirm)
-			val onProceed = {
-				// The Quick Build session's installed baseline is about to be replaced; stop it.
-				// Keyed off the re-check rather than off whether a dialog was shown: a tap that
-				// already confirmed this exact clobber skips the dialog but still clobbers.
-				if (now != QuickBuildClobberConfirmation.NotNeeded) {
-					quickBuildSessionManager()?.restartSession()
+			// installationAttempted() has already reset the build state, so an activity destroyed
+			// (rotation) during the IO parse below cancels this coroutine and would silently drop
+			// the whole install - a successful build with no install and no message. Until the
+			// install (or its confirm dialog) is actually dispatched, the drop path re-arms
+			// AwaitingInstall in the surviving ViewModel so the recreated activity retries.
+			var dispatched = false
+			try {
+				// Reading the APK's manifest is disk work, and on emulated storage that is not free.
+				val apkApplicationId = withContext(Dispatchers.IO) { apkApplicationId(state.apkFile) }
+				if (isDestroyed || isFinishing) {
+					return@launch
 				}
-				doInstallApk(state)
-			}
-			when (val decision = installTimeClobberConfirmation(answerAtTap, now)) {
-				QuickBuildClobberConfirmation.NotNeeded -> {
-					onProceed()
+				dispatched = true
+				val now =
+					quickBuildClobberConfirmation(apkApplicationId, clobberCheck::standardRunNeedsConfirm)
+				val onProceed = {
+					// The Quick Build session's installed baseline is about to be replaced; stop it.
+					// Keyed off the re-check rather than off whether a dialog was shown: a tap that
+					// already confirmed this exact clobber skips the dialog but still clobbers.
+					if (now != QuickBuildClobberConfirmation.NotNeeded) {
+						quickBuildSessionManager()?.restartSession()
+					}
+					doInstallApk(state)
 				}
+				when (val decision = installTimeClobberConfirmation(answerAtTap, now)) {
+					QuickBuildClobberConfirmation.NotNeeded -> {
+						onProceed()
+					}
 
-				QuickBuildClobberConfirmation.NeededForUnknownAppId -> {
-					confirmUnknownOccupantSwitch(onProceed)
-				}
+					QuickBuildClobberConfirmation.NeededForUnknownAppId -> {
+						confirmUnknownOccupantSwitch(onProceed)
+					}
 
-				is QuickBuildClobberConfirmation.Needed -> {
-					confirmBuildTypeSwitch(
-						getString(string.quick_build_switch_to_standard_title),
-						getString(string.quick_build_switch_to_standard_message, decision.applicationId),
-						onProceed,
-					)
+					is QuickBuildClobberConfirmation.Needed -> {
+						confirmBuildTypeSwitch(
+							getString(string.quick_build_switch_to_standard_title),
+							getString(string.quick_build_switch_to_standard_message, decision.applicationId),
+							onProceed,
+						)
+					}
+				}
+			} finally {
+				if (!dispatched) {
+					buildViewModel.reArmInstall(state)
 				}
 			}
 		}

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

@@ -6,6 +6,7 @@ import org.slf4j.LoggerFactory
 import java.io.File
 import java.io.FileNotFoundException
 import java.io.IOException
+import java.io.InputStream
 import java.util.zip.ZipInputStream
 
 /**
@@ -52,8 +53,25 @@ object QuickBuildArtifactStager {
 		}
 		Environment.mkdirIfNotExists(daemonDir)
 
+		val count = extractDaemonZip(context.assets.open(ASSET_DAEMON_ZIP).buffered(), daemonDir)
+		log.info("Staged {} daemon files into {}", count, daemonDir)
+	}
+
+	/**
+	 * Unpacks the daemon zip from [input] into [daemonDir]. Internal so the JVM test can watch
+	 * the zip-slip guard go red without an Android [Context].
+	 *
+	 * @return the number of files extracted.
+	 * @throws IOException on a zip entry escaping [daemonDir].
+	 * @throws FileNotFoundException when the zip contains no files.
+	 */
+	@Throws(IOException::class)
+	internal fun extractDaemonZip(
+		input: InputStream,
+		daemonDir: File,
+	): Int {
 		val canonicalRoot = daemonDir.canonicalFile
-		ZipInputStream(context.assets.open(ASSET_DAEMON_ZIP).buffered()).use { zip ->
+		ZipInputStream(input).use { zip ->
 			var entry = zip.nextEntry
 			var count = 0
 			while (entry != null) {
@@ -75,7 +93,7 @@ object QuickBuildArtifactStager {
 			if (count == 0) {
 				throw FileNotFoundException("Daemon zip contained no files")
 			}
-			log.info("Staged {} daemon files into {}", count, daemonDir)
+			return count
 		}
 	}
 }

app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt

@@ -117,11 +117,11 @@ class GradleBuildService :
 		private set
 
 	/**
-	 * Gradle output captured while the editor's listener is suppressed, oldest line first. Bounded
-	 * by [MAX_INTERNAL_OUTPUT_LINES]; guarded by itself, since it is written from the tooling
-	 * API's thread and drained from the caller's.
+	 * Gradle output captured while the editor's listener is suppressed, oldest line first.
+	 * Bounded by [MAX_INTERNAL_OUTPUT_LINES]; the routing/capture/drain logic lives in
+	 * [InternalBuildOutputCapture] so it is JVM-testable without this service.
 	 */
-	private val internalBuildOutput = ArrayDeque<String>()
+	private val internalBuildOutput = InternalBuildOutputCapture(MAX_INTERNAL_OUTPUT_LINES)
 
 	/**
 	 * Whether an INTERNAL build is running - a build the user never asked for that goes through the
@@ -134,7 +134,7 @@ class GradleBuildService :
 		InternalBuildBracket(
 			// Outermost internal build: drop any tail a previous one left unread, so a failure
 			// report quotes this build and not the last one.
-			onFirstAcquire = { synchronized(internalBuildOutput) { internalBuildOutput.clear() } },
+			onFirstAcquire = { internalBuildOutput.clear() },
 			// postValue, not setValue: the bracket releases on the tooling API's thread.
 			onHeldChanged = { held -> _internalBuildInProgress.postValue(held) },
 		)
@@ -453,27 +453,10 @@ class GradleBuildService :
 	}
 
 	override fun logOutput(line: String) {
-		val listener = editorListener()
-		if (listener != null) {
-			listener.onOutput(line)
-			return
-		}
-		// Suppressed because an internal build is running. Keep a bounded tail anyway: if that
-		// build FAILS this is the only copy of Gradle's reason, since the tooling API's own
-		// failure is a bare enum. See takeInternalBuildOutput.
-		synchronized(internalBuildOutput) {
-			if (internalBuildOutput.size >= MAX_INTERNAL_OUTPUT_LINES) {
-				internalBuildOutput.removeFirst()
-			}
-			internalBuildOutput.addLast(line)
-		}
-		internalBuildProgress?.let { report ->
-			try {
-				report(line)
-			} catch (e: Exception) {
-				log.warn("Internal build progress listener threw", e)
-			}
-		}
+		// When the editor's listener is suppressed (an internal build is running), a bounded
+		// tail is kept anyway: if that build FAILS it is the only copy of Gradle's reason,
+		// since the tooling API's own failure is a bare enum. See takeInternalBuildOutput.
+		internalBuildOutput.onLine(line, editorListener(), internalBuildProgress)
 	}
 
 	/**
@@ -484,12 +467,7 @@ class GradleBuildService :
 	 *
 	 * @return the captured lines, oldest first; empty when nothing was captured.
 	 */
-	fun takeInternalBuildOutput(): List<String> =
-		synchronized(internalBuildOutput) {
-			val captured = internalBuildOutput.toList()
-			internalBuildOutput.clear()
-			captured
-		}
+	fun takeInternalBuildOutput(): List<String> = internalBuildOutput.drain()
 
 	override fun prepareBuild(buildInfo: BuildInfo): CompletableFuture<ClientGradleBuildConfig> =
 		CompletableFuture.supplyAsync {

app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildTuner.kt

@@ -10,7 +10,15 @@ import com.itsaky.androidide.tooling.api.messages.BuildId
 import com.itsaky.androidide.tooling.api.messages.GradleBuildParams
 import org.slf4j.LoggerFactory
 
-/** @author Akash Yadav */
+/**
+ * Applies to EVERY Gradle build, Quick Build experiments flag on or off - deliberately NOT
+ * gated behind FeatureFlags.isExperimentsEnabled: the 384m Metaspace floor fixes builds that
+ * previously died in OutOfMemoryError: Metaspace (see BalancedStrategy.GRADLE_METASPACE_MB),
+ * and the tiered daemon idle timeouts are what keep the IDE itself (and, flag on, the
+ * quick-build daemon) resident on low-RAM devices after the user stops building.
+ *
+ * @author Akash Yadav
+ */
 object GradleBuildTuner {
 	private val logger = LoggerFactory.getLogger(GradleBuildTuner::class.java)
 

app/src/main/java/com/itsaky/androidide/services/builder/InternalBuildOutputCapture.kt

@@ -0,0 +1,73 @@
+package com.itsaky.androidide.services.builder
+
+import org.slf4j.LoggerFactory
+
+/**
+ * Routes one Gradle output line while an internal build may be suppressing the editor's build
+ * UI (see [GradleBuildService.logOutput]): lines go to the editor's listener when it is
+ * listening, otherwise into a bounded tail plus the internal progress listener.
+ *
+ * The tail exists because a suppressed build that FAILS has no other copy of Gradle's reason -
+ * the tooling API's own failure is a bare enum. Guarded by the deque itself: written from the
+ * tooling API's thread, drained from the caller's.
+ *
+ * @param maxLines how much tail to keep; oldest lines are dropped beyond it. Gradle puts the
+ *   cause at the END of the stream, so a tail is the right shape.
+ */
+class InternalBuildOutputCapture(
+	private val maxLines: Int,
+) {
+	private val lines = ArrayDeque<String>()
+
+	/**
+	 * Routes one Gradle output line.
+	 *
+	 * @param editorListener the editor's build listener, or null while it is suppressed.
+	 * @param progressListener where suppressed lines are additionally reported; it cannot veto
+	 *   the capture - a throwing listener is logged and the line is kept.
+	 */
+	fun onLine(
+		line: String,
+		editorListener: GradleBuildService.EventListener?,
+		progressListener: ((String) -> Unit)?,
+	) {
+		if (editorListener != null) {
+			editorListener.onOutput(line)
+			return
+		}
+		synchronized(lines) {
+			if (lines.size >= maxLines) {
+				lines.removeFirst()
+			}
+			lines.addLast(line)
+		}
+		progressListener?.let { report ->
+			try {
+				report(line)
+			} catch (e: Exception) {
+				log.warn("Internal build progress listener threw", e)
+			}
+		}
+	}
+
+	/**
+	 * Takes and clears the captured lines, oldest first; empty when nothing was captured.
+	 * Draining rather than reading, so one failure's report can never be quoted against the
+	 * next build.
+	 */
+	fun drain(): List<String> =
+		synchronized(lines) {
+			val captured = lines.toList()
+			lines.clear()
+			captured
+		}
+
+	/** Drops any tail a previous internal build left unread. */
+	fun clear() {
+		synchronized(lines) { lines.clear() }
+	}
+
+	companion object {
+		private val log = LoggerFactory.getLogger(InternalBuildOutputCapture::class.java)
+	}
+}

app/src/main/java/com/itsaky/androidide/viewmodel/BuildViewModel.kt

@@ -169,6 +169,21 @@ class BuildViewModel : ViewModel() {
 		}
 	}
 
+	/**
+	 * Re-arms [BuildState.AwaitingInstall] when an install dispatch was dropped before anything
+	 * user-visible happened (ADFA-4128): the flag-on install path parses the APK on IO after
+	 * [installationAttempted] has already reset the state, so a configuration change mid-parse
+	 * cancels the dispatch and would otherwise turn a successful build into no install and no
+	 * message. This ViewModel outlives the activity, so the recreated activity's collector sees
+	 * the re-armed state and retries. Only fires from [BuildState.Idle], so it cannot stomp a
+	 * build the user started in the meantime.
+	 */
+	fun reArmInstall(state: BuildState.AwaitingInstall) {
+		if (_buildState.value is BuildState.Idle) {
+			_buildState.value = state
+		}
+	}
+
 	/** Call this after the error has been shown once, so a lifecycle replay does not re-flash it. */
 	fun errorDisplayed() {
 		if (_buildState.value is BuildState.Error) {