feat(editor): async post-save diagnostics on chat-diff save path (L1, #1375)

src/core/task/Task.ts

@@ -3725,6 +3725,15 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		// could still build tools and call `createMessage()`.
 		this.requestDispose()
 
+		// Stop post-save diagnostics tails that are still waiting on their delay. A
+		// disposed task cannot receive their say() emit, and without this the timer (and
+		// the provider + diagnostics snapshot it holds) survives the teardown.
+		try {
+			this.diffViewProvider.cancelPostSaveDiagnosticsTails()
+		} catch (error) {
+			console.error("Error cancelling post-save diagnostics tails:", error)
+		}
+
 		// Cancel any in-progress HTTP request
 		try {
 			this.cancelCurrentRequest()

src/core/task/__tests__/Task.spec.ts

@@ -6152,6 +6152,44 @@ describe("Cline", () => {
 			expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Timed out"))
 		})
 
+		it("cancels post-save diagnostics tails when the task is disposed", async () => {
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "test task",
+				startTask: false,
+			})
+			const cancelSpy = vi
+				.spyOn(task.diffViewProvider, "cancelPostSaveDiagnosticsTails")
+				.mockImplementation(() => {})
+			// disposeOnce is private; bracket notation is the repo's convention for it.
+			await task["disposeOnce"]()
+			expect(cancelSpy).toHaveBeenCalledTimes(1)
+		})
+
+		it("continues disposal when cancelling the post-save diagnostics tails throws", async () => {
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "test task",
+				startTask: false,
+			})
+			vi.spyOn(task.diffViewProvider, "cancelPostSaveDiagnosticsTails").mockImplementation(() => {
+				throw new Error("cancel boom")
+			})
+			const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
+			// The step after the cancel in the same teardown: if the throw escaped, this would
+			// never run and the HTTP request would keep streaming into a dead task.
+			const cancelRequestSpy = vi.spyOn(task, "cancelCurrentRequest").mockImplementation(() => {})
+			// disposeOnce is private; bracket notation is the repo's convention for it.
+			await task["disposeOnce"]()
+			expect(cancelRequestSpy).toHaveBeenCalled()
+			expect(errorSpy).toHaveBeenCalledWith(
+				expect.stringContaining("Error cancelling post-save diagnostics tails:"),
+				expect.any(Error),
+			)
+		})
+
 		it("refuses to send a request when the task is disposed during the bounded metadata wait", async () => {
 			// Disposal alone — no cancel button, no abortTask — must make the
 			// task observe cancellation: disposeOnce sets the abort state

src/integrations/editor/DiffViewProvider.ts

@@ -46,6 +46,14 @@
 	private streamedLines: string[] = []
 	private preDiagnostics: [vscode.Uri, vscode.Diagnostic[]][] = []
 	private preEditScrollLine: number | undefined
+	// One controller per post-save diagnostics tail that is still waiting, so task
+	// disposal can cancel the wait instead of leaving a timer (and this provider and
+	// the pre-save diagnostics snapshot it closes over) running past the teardown.
+	private readonly postSaveTails = new Set<AbortController>()
+	// Latched once Task disposal has cancelled the tails. A save that was already awaiting
+	// its file operations can still reach the tail start after that point, and a tail that
+	// begins after disposal would emit into a dead task and keep its providers alive.
+	private tailsDisposed = false
 	// Tracks whether the user activated the target file's editor tab during the
 	// diff session. When the file was not already open before the edit, we only
 	// keep it open afterward if the user explicitly interacted with it.
@@ -1152,8 +1160,11 @@
 	}> {
 		const absolutePath = path.resolve(this.cwd, relPath)
 
-		// Get diagnostics before editing the file
-		this.preDiagnostics = vscode.languages.getDiagnostics()
+		// Get diagnostics before editing the file. Capture the snapshot locally:
+		// overlapping saveDirectly calls (multi-file edits) must not let a later
+		// call overwrite this one's baseline before its diagnostics tail runs.
+		const preDiagnostics = vscode.languages.getDiagnostics()
+		this.preDiagnostics = preDiagnostics
 
 		// Write the content directly to the file
 		await createDirectoriesForFile(absolutePath)
@@ -1176,52 +1187,155 @@
 				await doc.save()
 			}
 
-			// Force a small delay to ensure diagnostics are triggered
-			await new Promise((resolve) => setTimeout(resolve, 100))
+			// The 100 ms diagnostics-settle wait is carried by the
+			// emitPostSaveDiagnostics tail (inMemoryDocument) instead of here:
+			// blocking the save path delayed every openFile=false save even when
+			// diagnostics were disabled or the write delay was 0.
 		}
 
-		let newProblemsMessage = ""
-
+		// L1 (A2): resolve without awaiting the LSP diagnostics settle. The
+		// diagnostics check becomes a fire-and-forget tail that emits any new
+		// problems via the existing "error" ClineSay type; the returned
+		// newProblemsMessage is therefore always undefined.
 		if (diagnosticsEnabled) {
-			// Add configurable delay to allow linters time to process
-			const safeDelayMs = Math.max(0, writeDelayMs)
+			// The method's outer try/catch guarantees it never rejects, so the
+			// fire-and-forget call needs no .catch wrapper.
+			void this.emitPostSaveDiagnostics(relPath, writeDelayMs, preDiagnostics, !openFile)
+		}
+
+		// Store the results for formatFileWriteResponse
+		this.newProblemsMessage = undefined
+		this.userEdits = undefined
+		this.relPath = relPath
+		this.newContent = content
+
+		return {
+			newProblemsMessage: undefined,
+			userEdits: undefined,
+			finalContent: content,
+		}
+	}
+
+	// L1 (A2): fire-and-forget post-save diagnostics. After the write delay,
+	// collects new Error-severity problems and emits them via the existing
+	// "error" ClineSay type (only Error-severity diagnostics reach this branch;
+	// "error" carries no task-failure semantics in core). Abort-safe: say()
+	// rejects when the task is aborted, so the whole body sits inside a
+	// try/catch that degrades to a console.warn — the tail can never reject.
+	// The wait itself is registered in postSaveTails so Task disposal can cancel
+	// it: an unregistered delay keeps the timer, this provider and the pre-save
+	// diagnostics snapshot alive past disposal, and the tail then does stale
+	// diagnostics work against a task that is already gone.
+	private async emitPostSaveDiagnostics(
+		relPath: string,
+		writeDelayMs: number,
+		preDiagnostics: [vscode.Uri, vscode.Diagnostic[]][],
+		inMemoryDocument = false,
+	): Promise<void> {
+		if (this.tailsDisposed) {
+			// Disposal already ran: emitting now would call say() on a disposed task.
+			return
+		}
 
+		const controller = new AbortController()
+		this.postSaveTails.add(controller)
+		try {
+			// Add configurable delay to allow linters time to process. When the
+			// document was opened in memory (openFile=false), the tail also
+			// carries the 100 ms diagnostics-settle wait that used to block
+			// saveDirectly. The signal is the disposal hook: delay() rejects with
+			// AbortError once the task is gone, which is the tail's expected end.
+			const safeDelayMs = Math.max(0, writeDelayMs) + (inMemoryDocument ? 100 : 0)
 			try {
-				await delay(safeDelayMs)
+				await delay(safeDelayMs, { signal: controller.signal })
 			} catch (error) {
-				console.warn(`Failed to apply write delay: ${error}`)
+				if (controller.signal.aborted) {
+					return
+				}
+				throw error
+			}
+			// A cancellation can also land between the wait resolving and the work
+			// starting; either way nothing is queried or emitted after it.
+			if (controller.signal.aborted) {
+				return
 			}
 
-			const postDiagnostics = vscode.languages.getDiagnostics()
+			// Filter to the saved file: saveDirectly resolves before this tail
+			// completes, so in a multi-file write sequence (e.g. apply_patch)
+			// a later file's problems must not be attributed to this relPath.
+			const savedFilePath = path.resolve(this.cwd, relPath)
+			// arePathsEqual: case-insensitive on Windows, where a relPath whose
+			// casing differs from the diagnostic URI is still the same file.
+			const postDiagnostics = vscode.languages
+				.getDiagnostics()
+				.filter(([uri]) => arePathsEqual(uri.fsPath, savedFilePath))
 
 			// Get diagnostic settings from state
 			const task = this.taskRef.deref()
 			const state = await task?.providerRef.deref()?.getState()
 			const includeDiagnosticMessages = state?.includeDiagnosticMessages ?? true
 			const maxDiagnosticMessages = state?.maxDiagnosticMessages ?? 50
 
+			// The settings read is an await too, so disposal can land inside it and the
+			// await still returns normally. Stopping here keeps a gone task from paying for
+			// the formatting pass before the emit guard below.
+			if (controller.signal.aborted) {
+				return
+			}
+
 			const newProblems = await diagnosticsToProblemsString(
-				getNewDiagnostics(this.preDiagnostics, postDiagnostics),
+				getNewDiagnostics(preDiagnostics, postDiagnostics),
 				[vscode.DiagnosticSeverity.Error],
 				this.cwd,
 				includeDiagnosticMessages,
 				maxDiagnosticMessages,
 			)
 
-			newProblemsMessage =
-				newProblems.length > 0 ? `\n\nNew problems detected after saving the file:\n${newProblems}` : ""
-		}
+			// Formatting is awaited too, so a cancellation can land inside it. The emit below
+			// persists an error row into the task, so it must not start once the caller is gone:
+			// say() would otherwise be dropped or land in a task the user already left.
+			if (controller.signal.aborted) {
+				return
+			}
 
-		// Store the results for formatFileWriteResponse
-		this.newProblemsMessage = newProblemsMessage
-		this.userEdits = undefined
-		this.relPath = relPath
-		this.newContent = content
+			if (newProblems.length > 0) {
+				// This tail runs writeDelayMs after saveDirectly resolved, so the task has usually
+				// moved on and may be sitting in ask() waiting for the user - the next tool's
+				// approval, completion_result. say() bumps lastMessageTs unless the message is
+				// non-interactive, and ask()'s pWaitFor reads a moved lastMessageTs as
+				// "superseded": the pending ask would throw AskIgnoredError even though the user
+				// never answered. These diagnostics are informational, so they must not be able to
+				// cancel an ask; the "error" channel and the text stay exactly as they were.
+				await task?.say(
+					"error",
+					`New problems detected after saving file: ${relPath}\n\n${newProblems}`,
+					undefined /* images */,
+					undefined /* partial */,
+					undefined /* checkpoint */,
+					undefined /* progressStatus */,
+					{ isNonInteractive: true } /* options */,
+				)
+			}
+		} catch (error) {
+			// Abort-safe: never let a post-save diagnostic emit become an
+			// unhandled rejection (say() rejects when the task is aborted).
+			console.warn(`Post-save diagnostics emit failed: ${error}`)
+		} finally {
+			this.postSaveTails.delete(controller)
+		}
+	}
 
-		return {
-			newProblemsMessage,
-			userEdits: undefined,
-			finalContent: content,
+	/**
+	 * Cancel post-save diagnostics tails that are still waiting. Called when the
+	 * owning task is disposed. Deliberately NOT called from reset(): reset follows a
+	 * successful write, and the tail that write started still has to report the
+	 * problems it is waiting for.
+	 */
+	public cancelPostSaveDiagnosticsTails(): void {
+		this.tailsDisposed = true
+		for (const controller of this.postSaveTails) {
+			controller.abort()
 		}
+		this.postSaveTails.clear()
 	}
 }