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

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

@@ -101,9 +101,26 @@ class AddImportAction : BaseKotlinCodeAction() {
 		}
 
 		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 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 emptyMap()
+			}
+
+			candidates
 		} ?: emptyMap()
 	}
 

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 }
 		}
 

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/completion/KotlinCompletions.kt

@@ -26,6 +26,7 @@ import com.itsaky.androidide.lsp.models.MatchLevel
 import com.itsaky.androidide.preferences.utils.indentationString
 import com.itsaky.androidide.progress.ICancelChecker
 import com.itsaky.androidide.progress.ProgressManager
+import com.itsaky.androidide.projects.FileManager
 import io.github.rosemoe.sora.lang.completion.CompletionCancelledException
 import org.appdevforall.codeonthego.indexing.jvm.JvmClassInfo
 import org.appdevforall.codeonthego.indexing.jvm.JvmFunctionInfo
@@ -133,6 +134,37 @@ internal fun codeComplete(params: CompletionParams): CompletionResult {
 	}
 }
 
+/** The buffer a completion request was measured against, paired with its offset into it. */
+internal data class CompletionRequestBuffer(
+	val text: String,
+	val offset: Int,
+)
+
+/**
+ * The live buffer for [params] and the request's offset into it, or `null` if the offset is past its
+ * end.
+ *
+ * Deliberately the document rather than the pinned [LiveKtFile]: the pin is process-wide, so a joined
+ * scope hands over another feature's frozen text while [CompletionParams.position] was measured
+ * against the buffer. Taking both from the buffer keeps them on one version. Refusing on a stale pin
+ * instead would be worse than useless - the refusal returns before `analyzingVariant`, so an
+ * INTERACTIVE request never reaches [AnalysisScheduler] and stops preempting the older completion
+ * whose pin it joined, leaving that older one to publish items for a caret the user has moved past.
+ *
+ * A `null` means the buffer moved between the editor measuring the offset and this read, so the
+ * request describes text that no longer exists. Clamping the offset into range instead would compute
+ * items for an unrelated context and insert them at the user's real caret.
+ */
+internal fun completionRequestBuffer(params: CompletionParams): CompletionRequestBuffer? {
+	val text = FileManager.getDocumentContents(params.file)
+	val offset = params.position.requireIndex()
+	if (offset > text.length) {
+		logger.debug("skipping completion for {}: request offset is past the live buffer", params.file)
+		return null
+	}
+	return CompletionRequestBuffer(text, offset)
+}
+
 /**
  * Runs at the highest [AnalysisPriority.INTERACTIVE]: preempts in-progress diagnostics/indexing and
  * is never preempted by lower-priority work, but is superseded (cancelled and discarded) by a newer
@@ -142,11 +174,10 @@ context(env: CompilationEnvironment)
 internal fun doComplete(params: CompletionParams): CompletionResult {
 	val result =
 		env.ktSymbolIndex.withLiveKtFile(params.file) { live ->
-			// Completion still parses its own placeholder variant (text differs), anchored to the
-			// current file.
-			val originalText = live.read { it.text }
-			val requestPosition = params.position
-			val completionOffset = requestPosition.requireIndex()
+			// Completion still parses its own placeholder variant (text differs), anchored by the pin to
+			// the one instance every door answers with for the path.
+			val (originalText, completionOffset) =
+				completionRequestBuffer(params) ?: return@withLiveKtFile CompletionResult.EMPTY
 			val prefix = params.requirePrefix()
 			val partial = partialIdentifier(prefix)
 
@@ -162,9 +193,11 @@ internal fun doComplete(params: CompletionParams): CompletionResult {
 
 			abortIfCancelled()
 
-			// Use the request-scoped checker on params, not the global Lookup: Lookup holds one ICancelChecker
-			// updated per request, so with concurrent completions an older request could read a newer request's
-			// checker and never observe its own cancellation. Fall back to Lookup only for a NOOP checker (tests).
+			/*
+			 * Use the request-scoped checker on params, not the global Lookup: Lookup holds one ICancelChecker
+			 * updated per request, so with concurrent completions an older request could read a newer request's
+			 * checker and never observe its own cancellation. Fall back to Lookup only for a NOOP checker (tests).
+			 */
 			val delegate =
 				params.cancelChecker.takeUnless { it === ICancelChecker.NOOP }
 					?: Lookup.getDefault().lookup(ICancelChecker::class.java)

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/diagnostic/KotlinDiagnosticProvider.kt

@@ -66,9 +66,11 @@ private fun doAnalyze(
 	file: Path,
 	cancelChecker: ICancelChecker,
 ): DiagnosticResult {
-	// Diagnostics yield to completion but preempt indexing. The wrapped checker turns a scheduler
-	// preemption into an AnalysisPreemptedException, which CompilationEnvironment's fileAnalyzer catches
-	// to re-schedule this run once the higher-priority work finishes.
+	/*
+	 * Diagnostics yield to completion but preempt indexing. The wrapped checker turns a scheduler
+	 * preemption into an AnalysisPreemptedException, which CompilationEnvironment's fileAnalyzer catches
+	 * to re-schedule this run once the higher-priority work finishes.
+	 */
 	val checker = ScheduledCancelChecker(cancelChecker)
 
 	var superseded = false
@@ -91,9 +93,11 @@ private fun doAnalyze(
 								)
 							}
 
-						// analyzeMaybeDangling installs a CancelCheckerProgressIndicator, so this is cancellable
-						// mid-`analyze`: it aborts at the compiler's internal checkCanceled() once `checker` reports
-						// preemption/cancellation. (Previously this analysis was not cancellable at all.)
+						/*
+						 * analyzeMaybeDangling installs a CancelCheckerProgressIndicator, so this is cancellable
+						 * mid-`analyze`: it aborts at the compiler's internal checkCanceled() once `checker` reports
+						 * preemption/cancellation. (Previously this analysis was not cancellable at all.)
+						 */
 						ktFile
 							.collectDiagnostics(KaDiagnosticCheckerFilter.EXTENDED_AND_COMMON_CHECKERS)
 							.forEach { diagnostic ->