feat(tools): guarded write CAS core with per-path FIFO chain (S4a, #1375)

knip.json

@@ -30,6 +30,7 @@
 				"!utils/WorkspacePathResolver.ts!",
 				"!test-utils/**!",
 				"!utils/versionToken.ts!",
+				"!core/tools/guardedWrite.ts!",
 				"!core/tools/helpers/toolResultFormatting.ts!"
 			],
 			"ignoreDependencies": [

src/core/config/__tests__/importExport.spec.ts

@@ -1557,10 +1557,14 @@ describe("importExport", () => {
 			expect(mockContextProxy.export).toHaveBeenCalled()
 			expect(fs.mkdir).toHaveBeenCalledWith("/mock/path", { recursive: true })
 
-			expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/zoo-code-settings.json", {
-				providerProfiles: mockProviderProfiles,
-				globalSettings: mockGlobalSettings,
-			})
+			expect(safeWriteJson).toHaveBeenCalledWith(
+				"/mock/path/zoo-code-settings.json",
+				{
+					providerProfiles: mockProviderProfiles,
+					globalSettings: mockGlobalSettings,
+				},
+				{ refuseSymlinkTarget: true },
+			)
 		})
 
 		it("should include globalSettings when allowedMaxRequests is null", async () => {
@@ -1589,10 +1593,14 @@ describe("importExport", () => {
 				contextProxy: mockContextProxy,
 			})
 
-			expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/zoo-code-settings.json", {
-				providerProfiles: mockProviderProfiles,
-				globalSettings: mockGlobalSettings,
-			})
+			expect(safeWriteJson).toHaveBeenCalledWith(
+				"/mock/path/zoo-code-settings.json",
+				{
+					providerProfiles: mockProviderProfiles,
+					globalSettings: mockGlobalSettings,
+				},
+				{ refuseSymlinkTarget: true },
+			)
 		})
 
 		it("should handle errors during the export process", async () => {
@@ -1712,10 +1720,14 @@ describe("importExport", () => {
 					contextProxy: mockContextProxy,
 				})
 
-				expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/zoo-code-settings.json", {
-					providerProfiles: mockProviderProfiles,
-					globalSettings: mockGlobalSettings,
-				})
+				expect(safeWriteJson).toHaveBeenCalledWith(
+					"/mock/path/zoo-code-settings.json",
+					{
+						providerProfiles: mockProviderProfiles,
+						globalSettings: mockGlobalSettings,
+					},
+					{ refuseSymlinkTarget: true },
+				)
 			})
 
 			it("should export model dimension for OpenAI Compatible provider", async () => {
@@ -1865,10 +1877,14 @@ describe("importExport", () => {
 				})
 
 				// Should not throw an error and should preserve original settings
-				expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/zoo-code-settings.json", {
-					providerProfiles: mockProviderProfiles,
-					globalSettings: mockGlobalSettings, // Should remain unchanged
-				})
+				expect(safeWriteJson).toHaveBeenCalledWith(
+					"/mock/path/zoo-code-settings.json",
+					{
+						providerProfiles: mockProviderProfiles,
+						globalSettings: mockGlobalSettings, // Should remain unchanged
+					},
+					{ refuseSymlinkTarget: true },
+				)
 			})
 
 			it("should maintain backward compatibility with existing exports", async () => {
@@ -1908,10 +1924,14 @@ describe("importExport", () => {
 				})
 
 				// Should not modify settings for non-openai-compatible providers
-				expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/zoo-code-settings.json", {
-					providerProfiles: mockProviderProfiles,
-					globalSettings: mockGlobalSettings, // Should remain unchanged
-				})
+				expect(safeWriteJson).toHaveBeenCalledWith(
+					"/mock/path/zoo-code-settings.json",
+					{
+						providerProfiles: mockProviderProfiles,
+						globalSettings: mockGlobalSettings, // Should remain unchanged
+					},
+					{ refuseSymlinkTarget: true },
+				)
 			})
 
 			it("should handle missing current provider gracefully", async () => {
@@ -1953,10 +1973,14 @@ describe("importExport", () => {
 				})
 
 				// Should not throw an error and should preserve original settings
-				expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/zoo-code-settings.json", {
-					providerProfiles: mockProviderProfiles,
-					globalSettings: mockGlobalSettings, // Should remain unchanged
-				})
+				expect(safeWriteJson).toHaveBeenCalledWith(
+					"/mock/path/zoo-code-settings.json",
+					{
+						providerProfiles: mockProviderProfiles,
+						globalSettings: mockGlobalSettings, // Should remain unchanged
+					},
+					{ refuseSymlinkTarget: true },
+				)
 			})
 		})
 
@@ -2190,10 +2214,14 @@ describe("importExport", () => {
 			})
 
 			// Step 4: Verify the exported data includes the model dimension
-			expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/test-settings.json", {
-				providerProfiles: mockProviderProfiles,
-				globalSettings: mockGlobalSettings,
-			})
+			expect(safeWriteJson).toHaveBeenCalledWith(
+				"/mock/path/test-settings.json",
+				{
+					providerProfiles: mockProviderProfiles,
+					globalSettings: mockGlobalSettings,
+				},
+				{ refuseSymlinkTarget: true },
+			)
 
 			// Step 5: Get the exported data for import test
 			const exportedData = (safeWriteJson as Mock).mock.calls[0][1]

src/core/config/importExport.ts

@@ -346,7 +346,7 @@ export const exportSettings = async ({ providerSettingsManager, contextProxy }:
 
 		const dirname = path.dirname(uri.fsPath)
 		await fs.mkdir(dirname, { recursive: true })
-		await safeWriteJson(uri.fsPath, { providerProfiles, globalSettings })
+		await safeWriteJson(uri.fsPath, { providerProfiles, globalSettings }, { refuseSymlinkTarget: true })
 	} catch (e) {
 		console.error("Failed to export settings:", e)
 		// Don't re-throw - the UI will handle showing error messages

src/core/task/Task.ts

@@ -107,6 +107,7 @@ import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector"
 import { restoreTodoListForTask } from "../tools/UpdateTodoListTool"
 import { FileContextTracker } from "../context-tracking/FileContextTracker"
 import { RooIgnoreController } from "../ignore/RooIgnoreController"
+import { ObservationRegistry } from "./observationRegistry"
 import { RooProtectedController } from "../protect/RooProtectedController"
 import { type AssistantMessageContent, presentAssistantMessage } from "../assistant-message"
 import { type RequestPolicySnapshot } from "../prompts/tools/effective-tool-policy"
@@ -298,6 +299,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	readonly parentTask: Task | undefined = undefined
 	readonly taskNumber: number
 	readonly workspacePath: string
+	readonly observationRegistry = new ObservationRegistry()
 
 	/**
 	 * The mode associated with this task. Persisted across sessions
@@ -1450,6 +1452,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	/** Cancels the current persistence generation before creating the next assistant-turn boundary. */
 	private resetAssistantMessagePersistence(): void {
 		this.cancelAssistantMessagePersistence()
+
 		this.assistantMessagePersistencePromise = new Promise<AssistantMessagePersistenceResult>((resolve) => {
 			this.resolveAssistantMessagePersistence = resolve
 		})
@@ -3631,6 +3634,12 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`)
 		this.cancelAssistantMessagePersistence()
 
+		// Drop the per-task file observations. The registry holds an entry per absolute
+		// path the task read or wrote (version token + timestamp); nothing can consume them
+		// after disposal, and a long-lived extension host would otherwise keep every path a
+		// finished task touched alive.
+		this.observationRegistry.close()
+
 		// Release handler-owned host resources (e.g. VS Code configuration listeners).
 		// This is the fallback for leases that never settle; later lease releases
 		// find nothing retired and so cannot dispose twice.

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

@@ -1440,6 +1440,71 @@ describe("Cline", () => {
 		})
 	})
 
+	describe("observation registry lifecycle (S4a, epic #1375)", () => {
+		it("gives each Task its own observation registry", () => {
+			const firstTask = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "first observation task",
+				startTask: false,
+			})
+			const secondTask = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "second observation task",
+				startTask: false,
+			})
+
+			// The guarded-write contract assumes an observation in one task never validates
+			// a write issued by another task.
+			expect(firstTask.observationRegistry).not.toBe(secondTask.observationRegistry)
+			firstTask.observationRegistry.observe("/workspace/a.ts", "v-a")
+			expect(firstTask.observationRegistry.get("/workspace/a.ts")?.version).toBe("v-a")
+			expect(secondTask.observationRegistry.get("/workspace/a.ts")).toBeUndefined()
+		})
+
+		it("clears the observation registry when the task is disposed", async () => {
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "disposed observation task",
+				startTask: false,
+			})
+			task.observationRegistry.observe("/workspace/a.ts", "v-a")
+			task.observationRegistry.observe("/workspace/b.ts", "v-b")
+
+			await task.dispose()
+
+			// A disposed task cannot serve another guarded write, so its observed paths
+			// (version token + timestamp each) must not stay reachable for the host lifetime.
+			expect(task.observationRegistry.get("/workspace/a.ts")).toBeUndefined()
+			expect(task.observationRegistry.get("/workspace/b.ts")).toBeUndefined()
+			// Closing is what makes a later observation refuse, so it is asserted rather than
+			// left to the test name: clearing the map alone would look identical from here.
+			expect(task.observationRegistry.isClosed).toBe(true)
+		})
+
+		it("refuses an observation recorded after the task was disposed", async () => {
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "late observation task",
+				startTask: false,
+			})
+
+			await task.dispose()
+
+			// A read that was already in flight when the task was disposed can finish late and
+			// call observe(). Recording then would repopulate a registry no task owns and hand a
+			// version token to a guarded write that will never happen.
+			task.observationRegistry.observe("/workspace/late.ts", "v-late")
+
+			expect(task.observationRegistry.get("/workspace/late.ts")).toBeUndefined()
+			expect(task.observationRegistry.size).toBe(0)
+			expect(task.observationRegistry.isClosed).toBe(true)
+		})
+	})
+
 	describe("constructor", () => {
 		it.each([{ apiConfigName: "parent-local-profile" }, { apiConfigName: undefined }])(
 			"uses an explicit delegated-child context without shared state or startup persistence",

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

@@ -0,0 +1,90 @@
+import { describe, it, expect, vi } from "vitest"
+
+import { ObservationRegistry } from "../observationRegistry"
+
+describe("ObservationRegistry", () => {
+	it("observe → get returns the recorded version and observedAt", () => {
+		const reg = new ObservationRegistry()
+		reg.observe("/a/b/c.ts", "1:2:300:4000000000:5000000000")
+
+		const obs = reg.get("/a/b/c.ts")
+		expect(obs).toBeDefined()
+		expect(obs!.version).toBe("1:2:300:4000000000:5000000000")
+		expect(typeof obs!.observedAt).toBe("number")
+	})
+
+	it("re-observe replaces the entry with a fresh observedAt", () => {
+		vi.useFakeTimers()
+		const reg = new ObservationRegistry()
+		reg.observe("/a/b/c.ts", "v1")
+		const first = reg.get("/a/b/c.ts")!
+		expect(first.version).toBe("v1")
+
+		vi.advanceTimersByTime(50)
+		reg.observe("/a/b/c.ts", "v2")
+		const second = reg.get("/a/b/c.ts")!
+		expect(second.version).toBe("v2")
+		expect(second.observedAt).toBeGreaterThan(first.observedAt)
+
+		vi.useRealTimers()
+	})
+
+	it("has returns true for observed paths, false otherwise", () => {
+		const reg = new ObservationRegistry()
+		reg.observe("/x.ts", "t1")
+		expect(reg.has("/x.ts")).toBe(true)
+		expect(reg.has("/y.ts")).toBe(false)
+	})
+
+	it("size reflects the number of observed entries", () => {
+		const reg = new ObservationRegistry()
+		expect(reg.size).toBe(0)
+		reg.observe("/a.ts", "t1")
+		reg.observe("/b.ts", "t2")
+		expect(reg.size).toBe(2)
+	})
+
+	it("clear removes all entries and resets size to 0", () => {
+		const reg = new ObservationRegistry()
+		reg.observe("/a.ts", "t1")
+		reg.observe("/b.ts", "t2")
+		reg.clear()
+		expect(reg.size).toBe(0)
+		expect(reg.get("/a.ts")).toBeUndefined()
+		expect(reg.has("/b.ts")).toBe(false)
+	})
+
+	it("get on empty registry returns undefined", () => {
+		const reg = new ObservationRegistry()
+		expect(reg.get("/any.ts")).toBeUndefined()
+	})
+
+	it("separate instances are independent — observing in one does not appear in the other", () => {
+		const regA = new ObservationRegistry()
+		const regB = new ObservationRegistry()
+		regA.observe("/shared.ts", "v1")
+		expect(regA.get("/shared.ts")).toBeDefined()
+		expect(regB.get("/shared.ts")).toBeUndefined()
+		regB.observe("/shared.ts", "v2")
+		expect(regA.get("/shared.ts")!.version).toBe("v1")
+		expect(regB.get("/shared.ts")!.version).toBe("v2")
+	})
+})
+
+describe("close() - disposal is terminal", () => {
+	it("drops every observation and refuses later ones", () => {
+		const registry = new ObservationRegistry()
+		registry.observe("/workspace/a.ts", "v-a")
+		expect(registry.size).toBe(1)
+
+		registry.close()
+
+		expect(registry.size).toBe(0)
+		expect(registry.isClosed).toBe(true)
+
+		// A read that was in flight when the task was disposed must not repopulate it.
+		registry.observe("/workspace/late.ts", "v-late")
+		expect(registry.get("/workspace/late.ts")).toBeUndefined()
+		expect(registry.size).toBe(0)
+	})
+})