ADFA-5231: Refuse edits computed against a joined stale pin

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.kt

@@ -24,6 +24,7 @@ import com.itsaky.androidide.lsp.models.TextEdit
 import com.itsaky.androidide.resources.R
 import com.itsaky.androidide.utils.applyLongPressRecursively
 import com.itsaky.androidide.utils.flashError
+import com.itsaky.androidide.utils.flashInfo
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import java.nio.file.Path
@@ -58,10 +59,10 @@ class AddImportAction : BaseKotlinCodeAction() {
 		}
 	}
 
-	override suspend fun execAction(data: ActionData): Map<String, List<TextEdit>> {
+	override suspend fun execAction(data: ActionData): Any {
 		val (_, extra) =
 			data.findDiagnosticExtra<DiagnosticAction.ResolveReference>()
-				?: return emptyMap()
+				?: return ImportCandidates.Found(emptyMap())
 
 		val (env, action) = extra
 		val nioPath = data.requireFile().toPath()
@@ -83,7 +84,7 @@ class AddImportAction : BaseKotlinCodeAction() {
 		env: AbstractCompilationEnvironment,
 		nioPath: Path,
 		referenceName: String,
-	): Map<String, List<TextEdit>> {
+	): ImportCandidates {
 		/*
 		 * Resolved before the file is pinned, not inside the pin: this is an unbounded SQLite scan that
 		 * never reads the file, and a pin held across it freezes live-PSI refresh for the path - every
@@ -97,14 +98,31 @@ class AddImportAction : BaseKotlinCodeAction() {
 				.toList()
 
 		if (classifiers.isEmpty()) {
-			return emptyMap()
+			return ImportCandidates.Found(emptyMap())
 		}
 
 		return env.ktSymbolIndex.withLiveKtFile(nioPath) { live ->
-			live.read { ktFile ->
-				classifiers.associate { it.fqName to insertImport(ktFile, it.fqName) }
+			if (live.isStale) {
+				// Joining another feature's scope hands over text older than the buffer, so the import
+				// insertion point computed from it would land in the wrong place.
+				logger.debug("skipping import candidates for {}: pinned text is behind the buffer", nioPath)
+				return@withLiveKtFile ImportCandidates.FileChanged
 			}
-		} ?: emptyMap()
+
+			val candidates =
+				live.read { ktFile ->
+					classifiers.associate { it.fqName to insertImport(ktFile, it.fqName) }
+				}
+
+			if (live.isStale) {
+				// Resolving the file and taking the read lock can both block long enough for the user to
+				// type, and nothing between here and performCodeAction re-checks the insertion point.
+				logger.debug("dropping import candidates for {}: buffer moved while computing", nioPath)
+				return@withLiveKtFile ImportCandidates.FileChanged
+			}
+
+			ImportCandidates.Found(candidates)
+		} ?: ImportCandidates.Found(emptyMap())
 	}
 
 	override fun postExec(
@@ -113,14 +131,17 @@ class AddImportAction : BaseKotlinCodeAction() {
 	) {
 		super.postExec(data, result)
 
-		if (result !is Map<*, *>) {
+		if (result is ImportCandidates.FileChanged) {
+			flashInfo(R.string.msg_import_file_changed)
 			return
 		}
 
-		@Suppress("UNCHECKED_CAST")
-		result as Map<String, List<TextEdit>>
+		if (result !is ImportCandidates.Found) {
+			return
+		}
 
-		if (result.isEmpty()) {
+		val candidates = result.edits
+		if (candidates.isEmpty()) {
 			logger.warn("No classifiers to import.")
 			flashError(R.string.msg_no_imports_found)
 			return
@@ -136,7 +157,7 @@ class AddImportAction : BaseKotlinCodeAction() {
 		val file = data.requireFile()
 		val nioPath = file.toPath()
 		val actions =
-			result
+			candidates
 				.map { (fqName, edits) ->
 					CodeActionItem(
 						title = fqName,
@@ -211,3 +232,20 @@ class AddImportAction : BaseKotlinCodeAction() {
 		)
 	}
 }
+
+/**
+ * The outcome of resolving import candidates.
+ *
+ * The two are distinct at the UI: [Found] with no entries means the reference names nothing
+ * importable, while [FileChanged] means candidates were found and then discarded because the buffer
+ * moved out from under the offsets they were measured against.
+ */
+internal sealed interface ImportCandidates {
+	/** The import edits for each candidate, keyed by fully-qualified name. */
+	data class Found(
+		val edits: Map<String, List<TextEdit>>,
+	) : ImportCandidates
+
+	/** The buffer moved while the candidates were being computed, so the edits were dropped. */
+	data object FileChanged : ImportCandidates
+}

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.kt

@@ -72,27 +72,46 @@ class ImplementMembersAction : BaseKotlinCodeAction() {
 		cancelChecker: ICancelChecker,
 	): List<TextEdit> =
 		runCatching {
-			// A user-invoked command: AnalysisPriority.COMMAND, retried once if keystroke-driven work
-			// preempts it (ADR 0011). Without the retry a preemption fell into the getOrElse below and the
-			// action silently inserted nothing. The file is re-pinned per attempt because the preemptor
-			// also refreshed the live PSI.
+			/*
+			 * A user-invoked command: AnalysisPriority.COMMAND, retried once if keystroke-driven work
+			 * preempts it (ADR 0011). Without the retry a preemption fell into the getOrElse below and the
+			 * action silently inserted nothing. The file is re-pinned per attempt because the preemptor
+			 * also refreshed the live PSI.
+			 */
 			retryingOnPreemption(cancelChecker, "Implement members for $nioPath") { checker ->
 				env.ktSymbolIndex.withLiveKtFile(nioPath) { live ->
-					live.read { ktFile ->
-						val classOrObject = findEnclosingClassOrObject(ktFile, offset) ?: return@read emptyList()
-						live.analyzing(AnalysisPriority.COMMAND, checker) {
-							val classSymbol = classOrObject.symbol as? KaClassSymbol ?: return@analyzing emptyList()
-							if (!isImplementable(classSymbol)) return@analyzing emptyList()
-
-							val classIndent = classIndentOf(ktFile, classOrObject)
-							val unit = detectIndentUnit(ktFile.text)
-							val memberIndent = memberIndentOf(ktFile, classOrObject, classIndent, unit)
-							val stubs = membersToImplement(classSymbol).mapNotNull { renderOverrideStub(it, memberIndent, unit) }
-							if (stubs.isEmpty()) return@analyzing emptyList()
-
-							buildInsertionEdit(ktFile, classOrObject, stubs, classIndent)
+					if (live.isStale) {
+						// Joining another feature's scope hands over text older than the buffer, so both the
+						// caret offset and the computed insertion point would land in the wrong place.
+						logger.debug("skipping implement-members for {}: pinned text is behind the buffer", nioPath)
+						return@withLiveKtFile emptyList()
+					}
+
+					val edits =
+						live.read { ktFile ->
+							val classOrObject = findEnclosingClassOrObject(ktFile, offset) ?: return@read emptyList()
+							live.analyzing(AnalysisPriority.COMMAND, checker) {
+								val classSymbol = classOrObject.symbol as? KaClassSymbol ?: return@analyzing emptyList()
+								if (!isImplementable(classSymbol)) return@analyzing emptyList()
+
+								val classIndent = classIndentOf(ktFile, classOrObject)
+								val unit = detectIndentUnit(ktFile.text)
+								val memberIndent = memberIndentOf(ktFile, classOrObject, classIndent, unit)
+								val stubs = membersToImplement(classSymbol).mapNotNull { renderOverrideStub(it, memberIndent, unit) }
+								if (stubs.isEmpty()) return@analyzing emptyList()
+
+								buildInsertionEdit(ktFile, classOrObject, stubs, classIndent)
+							}
 						}
+
+					if (live.isStale) {
+						// The analysis above is slow enough for the user to type through, and nothing between
+						// here and performCodeAction re-checks the offsets these edits were measured against.
+						logger.debug("dropping implement-members edits for {}: buffer moved while computing", nioPath)
+						return@withLiveKtFile emptyList()
 					}
+
+					edits
 				} ?: emptyList()
 			}
 		}.getOrElse { e ->

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.kt

@@ -11,6 +11,7 @@ import com.itsaky.androidide.actions.requireFile
 import com.itsaky.androidide.idetooltips.TooltipManager
 import com.itsaky.androidide.idetooltips.TooltipTag
 import com.itsaky.androidide.lsp.api.ILanguageClient
+import com.itsaky.androidide.lsp.kotlin.compiler.AbstractCompilationEnvironment
 import com.itsaky.androidide.lsp.kotlin.diagnostic.DiagnosticAction
 import com.itsaky.androidide.lsp.kotlin.utils.NullSafetyKind
 import com.itsaky.androidide.lsp.kotlin.utils.NullSafetyVariant
@@ -25,6 +26,7 @@ import com.itsaky.androidide.utils.applyLongPressRecursively
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
+import java.nio.file.Path
 
 /**
  * Offers null-safety quick fixes on an UNSAFE_CALL diagnostic (`receiver.selector` where `receiver`
@@ -68,24 +70,56 @@ class NullSafetyAction : BaseKotlinCodeAction() {
 
 			// Off the main thread: acquiring the pin resolves the file first, which can block on a refresh.
 			withContext(Dispatchers.IO) {
-				extra.compilationEnv.ktSymbolIndex.withLiveKtFile(nioPath) { live ->
-					live.read { ktFile ->
-						val qe =
-							findNullableMemberAccess(
-								ktFile,
-								diagnostic.range.start.requireIndex(),
-								diagnostic.range.end.requireIndex(),
-							) ?: return@read emptyList()
-						nullSafetyVariants(qe)
-					}
-				} ?: emptyList()
+				computeNullSafetyVariants(
+					extra.compilationEnv,
+					nioPath,
+					diagnostic.range.start.requireIndex(),
+					diagnostic.range.end.requireIndex(),
+				)
 			}
 		}.getOrElse { e ->
 			if (e is CancellationException) throw e
 			logger.warn("Failed to compute null-safety fixes", e)
 			emptyList()
 		}
 
+	/**
+	 * The null-safety rewrites for the nullable member access spanning [startOffset] to [endOffset].
+	 *
+	 * Blocking: pinning the file resolves it first, so callers must stay off the main thread
+	 * ([execAction] wraps it in [Dispatchers.IO]). Returns an empty list when the span names no
+	 * nullable access, and when the pinned text is behind the buffer.
+	 */
+	internal fun computeNullSafetyVariants(
+		env: AbstractCompilationEnvironment,
+		nioPath: Path,
+		startOffset: Int,
+		endOffset: Int,
+	): List<NullSafetyVariant> =
+		env.ktSymbolIndex.withLiveKtFile(nioPath) { live ->
+			if (live.isStale) {
+				// Joining another feature's scope hands over text older than the buffer, and these variants
+				// carry raw PSI offsets that nothing downstream re-checks against the document.
+				logger.debug("skipping null-safety fixes for {}: pinned text is behind the buffer", nioPath)
+				return@withLiveKtFile emptyList()
+			}
+
+			val variants =
+				live.read { ktFile ->
+					val qe = findNullableMemberAccess(ktFile, startOffset, endOffset) ?: return@read emptyList()
+					nullSafetyVariants(qe)
+				}
+
+			if (live.isStale) {
+				// Resolving the file and taking the read lock can both block long enough for the user to
+				// type, and these variants carry raw PSI offsets that nothing downstream re-checks.
+				logger.debug("dropping null-safety fixes for {}: buffer moved while computing", nioPath)
+				return@withLiveKtFile emptyList()
+			}
+
+			variants
+		} ?: emptyList()
+
 	override fun postExec(
 		data: ActionData,
 		result: Any,

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.kt

@@ -59,20 +59,39 @@ class OrganizeImportsAction : BaseKotlinCodeAction() {
 		cancelChecker: ICancelChecker,
 	): List<TextEdit> =
 		runCatching {
-			// A user-invoked command: AnalysisPriority.COMMAND, retried once if keystroke-driven work
-			// preempts it (ADR 0011). Without the retry a preemption fell into the getOrElse below and
-			// organize-imports silently did nothing. The file is re-pinned per attempt because the
-			// preemptor also refreshed the live PSI.
+			/*
+			 * A user-invoked command: AnalysisPriority.COMMAND, retried once if keystroke-driven work
+			 * preempts it (ADR 0011). Without the retry a preemption fell into the getOrElse below and
+			 * organize-imports silently did nothing. The file is re-pinned per attempt because the
+			 * preemptor also refreshed the live PSI.
+			 */
 			retryingOnPreemption(cancelChecker, "Organize imports for $nioPath") { checker ->
 				env.ktSymbolIndex.withLiveKtFile(nioPath) { live ->
-					live.read { ktFile ->
-						if (ktFile.importDirectives.isEmpty()) return@read emptyList()
-						val usage = live.analyzing(AnalysisPriority.COMMAND, checker) { collectImportUsage(it) }
-						val newText = organizedImportBlock(ktFile, usage) ?: return@read emptyList()
-						val range = ktFile.importList?.textRange?.toRange(ktFile) ?: return@read emptyList()
-						if (range == Range.NONE) return@read emptyList()
-						listOf(TextEdit(range, newText))
+					if (live.isStale) {
+						// Joining another feature's scope hands over text older than the buffer, and the
+						// import-list range computed from it would replace the wrong span.
+						logger.debug("skipping organize-imports for {}: pinned text is behind the buffer", nioPath)
+						return@withLiveKtFile emptyList()
 					}
+
+					val edits =
+						live.read { ktFile ->
+							if (ktFile.importDirectives.isEmpty()) return@read emptyList()
+							val usage = live.analyzing(AnalysisPriority.COMMAND, checker) { collectImportUsage(it) }
+							val newText = organizedImportBlock(ktFile, usage) ?: return@read emptyList()
+							val range = ktFile.importList?.textRange?.toRange(ktFile) ?: return@read emptyList()
+							if (range == Range.NONE) return@read emptyList()
+							listOf(TextEdit(range, newText))
+						}
+
+					if (live.isStale) {
+						// The analysis above is slow enough for the user to type through, and nothing between
+						// here and performCodeAction re-checks the range these edits were measured against.
+						logger.debug("dropping organize-imports edits for {}: buffer moved while computing", nioPath)
+						return@withLiveKtFile emptyList()
+					}
+
+					edits
 				} ?: emptyList()
 			}
 		}.getOrElse { e ->

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/KtSymbolIndex.kt

@@ -336,6 +336,13 @@ internal class KtSymbolIndex(
 	 * Never call this while holding `project.read` - it deadlocks. Acquire the scope first, then use
 	 * [LiveKtFile.read] / [LiveKtFile.analyzing] inside it, which take the read lock for you.
 	 *
+	 * The pin is process-wide, not per-caller: while any scope on [path] is open, *every* request for
+	 * that path joins it and sees the same instance and the same text, including requests from unrelated
+	 * features. So a scope's duration is a staleness window for everyone else - a caller that joins a
+	 * long-running scope can get text older than the buffer the user is looking at. Any site whose
+	 * output is an edit, or that indexes into the text with coordinates from its own request, must
+	 * therefore check [LiveKtFile.isStale] and degrade rather than compute against frozen text.
+	 *
 	 * Known gap: the instance is resolved *before* the pin is installed, so a request arriving in that
 	 * window sees no pin and can launch a refresh that completes inside this scope, firing
 	 * `registerInMemoryFile` and a FIR modification event underneath it. Instance identity still holds -
@@ -523,9 +530,11 @@ internal class KtSymbolIndex(
 		pins[path]?.let { return it.file }
 
 		if (FileManager.isActive(path)) {
-			// Peek, never block: getKtFile runs under project.read inside Analysis-API services, so a
-			// blocking getCurrentKtFile().get() (its refresh needs project.write) would deadlock. A miss
-			// falls through to the disk instance; the edit already scheduled a refresh for next time.
+			/*
+			 * Peek, never block: getKtFile runs under project.read inside Analysis-API services, so a
+			 * blocking refresh (which needs project.write) would deadlock. A miss falls through to the disk
+			 * instance; the edit already scheduled a refresh for next time.
+			 */
 			getCurrentKtFileIfPresent(path)?.let { return it }
 		}