Revert "Revert "Move responsibility of disk watching for active editor into KclManager, don't watch while writing (#10305)""

This reverts commit a66c669.

Author: franknoirot

e2e/playwright/debug-pane.spec.ts

@@ -52,6 +52,7 @@ test.describe('Debug pane', { tag: '@desktop' }, () => {
         await page.keyboard.press('ArrowDown')
       }
     })
+    let lastSegmentText = await segment.innerText()
     // TODO: if you type all the code at once without delay (or paste it in)
     // the initial segment artifact ID is different. This appears to be niche bug
     // that is being sidestepped in this test until https://github.com/KittyCAD/modeling-app/issues/9609 is addressed.
@@ -60,20 +61,23 @@ test.describe('Debug pane', { tag: '@desktop' }, () => {
       // Wait for keyboard input debounce and updated artifact graph.
       await page.waitForTimeout(1000)
     })
-    // Extract the artifact IDs from the debug artifact graph.
-    const initialSegmentIds = await segment.innerText({ timeout: 5_000 })
+    await expect(segment).not.toHaveText(lastSegmentText)
     // The artifact ID should include a UUID.
-    expect(initialSegmentIds).toMatch(
+    const uuidRegexp =
       /[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}/
-    )
-    await test.step('Enter a comment', async () => {
+    await expect(segment).toHaveText(uuidRegexp)
+    const uuid = (await segment.innerText()).match(uuidRegexp)!
+
+    await test.step('Enter another line', async () => {
+      lastSegmentText = await segment.innerText()
       await page.keyboard.type('\n|> line(end = [2, 2])', { delay: 10 })
       // Wait for keyboard input debounce and updated artifact graph.
       await page.waitForTimeout(1000)
     })
-    const newSegmentIds = await segment.innerText()
-    // Strip off the closing bracket.
-    const initialIds = initialSegmentIds.slice(0, initialSegmentIds.length - 1)
-    expect(newSegmentIds.slice(0, initialIds.length)).toEqual(initialIds)
+
+    // Expect the artifact IDs to be changed (by adding another),
+    await expect(segment).not.toHaveText(lastSegmentText)
+    // but still contain the stable first ID.
+    await expect(segment).toContainText(uuid)
   })
 })

src/components/CommandBar/CommandBarSelectionMixedInput.spec.tsx

@@ -55,10 +55,11 @@ describe('CommandBarSelectionMixedInput', () => {
   describe('clearSelectionFirst behavior', () => {
     it('should send clear selection command when clearSelectionFirst is true', async () => {
       const app = App.getDefaultSystems()
-      const executingEditor = new KclManager({
+      const executingEditor = new KclManager('some-path', {
         commandBar: app.commands.actor,
         settings: app.settings.actor,
         wasmInstancePromise: app.wasmPromise,
+        projectPath: 'some-project',
       })
       const mockModelingSend = vi.spyOn(
         executingEditor.engineCommandManager,
@@ -85,10 +86,11 @@ describe('CommandBarSelectionMixedInput', () => {
 
     it('should NOT send clear selection command when clearSelectionFirst is false', async () => {
       const app = App.getDefaultSystems()
-      const executingEditor = new KclManager({
+      const executingEditor = new KclManager('some-path', {
         commandBar: app.commands.actor,
         settings: app.settings.actor,
         wasmInstancePromise: app.wasmPromise,
+        projectPath: 'some-project',
       })
       const mockModelingSend = vi.spyOn(
         executingEditor.engineCommandManager,
@@ -112,10 +114,11 @@ describe('CommandBarSelectionMixedInput', () => {
 
     it('should NOT send clear selection command when clearSelectionFirst is undefined', async () => {
       const app = App.getDefaultSystems()
-      const executingEditor = new KclManager({
+      const executingEditor = new KclManager('some-path', {
         commandBar: app.commands.actor,
         settings: app.settings.actor,
         wasmInstancePromise: app.wasmPromise,
+        projectPath: 'some-project',
       })
       const mockModelingSend = vi.spyOn(
         executingEditor.engineCommandManager,
@@ -139,10 +142,11 @@ describe('CommandBarSelectionMixedInput', () => {
 
     it('should send clear selection command only once on mount', async () => {
       const app = App.getDefaultSystems()
-      const executingEditor = new KclManager({
+      const executingEditor = new KclManager('some-path', {
         commandBar: app.commands.actor,
         settings: app.settings.actor,
         wasmInstancePromise: app.wasmPromise,
+        projectPath: 'some-project',
       })
       const mockModelingSend = vi.spyOn(
         executingEditor.engineCommandManager,
@@ -181,10 +185,11 @@ describe('CommandBarSelectionMixedInput', () => {
 
     it('should set hasClearedSelection state after clearing', async () => {
       const app = App.getDefaultSystems()
-      const executingEditor = new KclManager({
+      const executingEditor = new KclManager('some-path', {
         commandBar: app.commands.actor,
         settings: app.settings.actor,
         wasmInstancePromise: app.wasmPromise,
+        projectPath: 'some-project',
       })
       const mockModelingSend = vi.spyOn(
         executingEditor.engineCommandManager,

src/components/RouteProvider.tsx

@@ -1,10 +1,7 @@
-import { isCodeTheSame } from '@src/lib/codeEditor'
 import fsZds from '@src/lib/fs-zds'
 import type { ReactNode } from 'react'
 import { createContext, useEffect, useState } from 'react'
 import { useLocation, useNavigate, useNavigation } from 'react-router-dom'
-import toast from 'react-hot-toast'
-
 import { useAuthNavigation } from '@src/hooks/useAuthNavigation'
 import { useFileSystemWatcher } from '@src/hooks/useFileSystemWatcher'
 import { getAppSettingsFilePath } from '@src/lib/desktop'
@@ -75,35 +72,10 @@ export function RouteProvider({ children }: { children: ReactNode }) {
       // is very high in the context tree, higher than mlEphant's.
       if (kclManager.mlEphantManagerMachineBulkManipulatingFileSystem) return
 
-      const isCurrentFile = loadedFile?.path === path
-      if (isCurrentFile) {
-        if (window.electron) {
-          // Your current file is changed, read it from disk and write it into the code manager and execute the AST,
-          // unless the change was initiated by us (the currently running instance).
-          const code = await window.electron.readFile(path, {
-            encoding: 'utf-8',
-          })
-
-          const lastWrittenCode = kclManager.lastWrite?.code
-          if (!lastWrittenCode || !isCodeTheSame(lastWrittenCode, code)) {
-            const isInSketchMode =
-              kclManager.modelingState?.matches('Sketch') ||
-              kclManager.modelingState?.matches('sketchSolveMode')
-
-            // Nothing written out yet by ourselves, or it's not the same as the current file content
-            // -> this must be an external change -> re-execute.
-            kclManager.updateCodeEditor(code, {
-              shouldExecute: !isInSketchMode,
-              shouldResetCamera: !isInSketchMode,
-              // We explicitly do not write to the file here since we are loading from
-              // the file system and not the editor.
-              shouldWriteToDisk: false,
-            })
-
-            toast('Reloading file from disk', { icon: '📁' })
-          }
-        }
-      } else {
+      // We only react on files other than the currently-executing one here
+      // because the currently-executing one is handled with its own watcher in
+      // KclManager. In future, all files and folders will watch themselves.
+      if (loadedFile?.path !== path) {
         const fileNameWithExtension = getStringAfterLastSeparator(path)
         // Is the file from the change event type imported into the currently opened file
         const isImportedInCurrentFile = kclManager.ast.body.some(

src/lang/KclManager.ts

@@ -42,7 +42,7 @@ import {
 } from '@src/lib/settings/settingsUtils'
 
 import { err, reportRejection } from '@src/lib/trap'
-import { deferredCallback } from '@src/lib/utils'
+import { deferredCallback, uuidv4 } from '@src/lib/utils'
 import { ConnectionManager } from '@src/network/connectionManager'
 import { EngineDebugger } from '@src/lib/debugger'
 import type {
@@ -131,6 +131,7 @@ import type { FileEntry, Project } from '@src/lib/project'
 import { getStringAfterLastSeparator } from '@src/lib/paths'
 import type { SettingsActorType } from '@src/machines/settingsMachine'
 import type { CommandBarActorType } from '@src/machines/commandBarMachine'
+import { isCodeTheSame } from '@src/lib/codeEditor'
 import { getResolvedTheme } from '@src/lib/theme'
 
 interface ExecuteArgs {
@@ -155,6 +156,7 @@ interface SystemDeps {
   wasmInstancePromise: Promise<ModuleType>
   settings: SettingsActorType
   commandBar: CommandBarActorType
+  projectPath: string
 }
 
 export enum KclManagerEvents {
@@ -234,42 +236,47 @@ export class ZDSProject {
     if (newPath === null) {
       return
     }
-    const foundPathSignal = this.findEditorPathSignal(newPath)
+    const foundPathSignal = this.findEditor(newPath)
     if (!foundPathSignal) {
       return
     }
-    const found = this.editors.get(foundPathSignal)
+    const found = foundPathSignal[1]
     if (found) {
       // TODO: Reconfigure the editor to be an executing one
     }
-    this.#executingPath.value = foundPathSignal
+    this.#executingPath.value = foundPathSignal[0]
   }
-  findEditorPathSignal(path: string) {
-    return this.editors.keys().find((p) => p.value === path)
+  findEditor(path: string) {
+    return this.editors.entries().find(([p]) => p.value === path)
   }
 
   // Saving some keystrokes
-  private get = this.editors.get.bind(this.editors)
   private set = this.editors.set.bind(this.editors)
 
   // TODO: Remove providedEditor, replace with options about if the editor is the executing one
   // once the app can handle not having a KclManager.
   openEditor(path: string, providedEditor?: KclManager) {
-    const foundPathSignal = this.findEditorPathSignal(path)
-    const found = foundPathSignal ? this.get(foundPathSignal) : undefined
+    const foundEditor = this.findEditor(path)
+    const found = foundEditor?.[1]
     if (found) {
       console.warn(`Attempted to overwrite editor with path "${path}"`)
       return found
     }
 
     const newEditor =
       providedEditor ??
-      new KclManager({
+      new KclManager(path, {
         wasmInstancePromise: this.app.wasmPromise,
         commandBar: this.app.commands.actor,
         settings: this.app.settings.actor,
+        get projectPath() {
+          return this.path
+        },
       })
 
+    if (providedEditor) {
+      providedEditor.path = path
+    }
     // Initialize the editor theme
     // Subsequent changes are listened for within app.onSettingsUpdate()
     // TODO: Disassemble onSettingsUpdate, subscribe to changes from subsystems
@@ -285,15 +292,19 @@ export class ZDSProject {
   }
 
   closeEditor(path: string) {
-    const foundPathSignal = this.findEditorPathSignal(path)
+    const foundPathSignal = this.findEditor(path)
     if (!foundPathSignal) {
       console.warn(`Attempted to close nonexistent editor with path "${path}"`)
       return
     }
-    this.editors.delete(foundPathSignal)
+    foundPathSignal[1].close()
+    this.editors.delete(foundPathSignal[0])
   }
 
   closeAllEditors() {
+    for (const editor of this.editors.values()) {
+      editor.close()
+    }
     this.editors.clear()
   }
 }
@@ -428,6 +439,44 @@ export class KclManager extends EventTarget {
 
   // INTERNAL BOOKKEEPING STATE
 
+  private fileWatcherKey = uuidv4()
+  /**
+   * Watching the file system for updates and reacting to them.
+   * TODO: We don't watch for deletions here, should we?
+   */
+  private onFileWatchEvent = (_eventType: string, path: string) => {
+    // TODO: We can remove this once we make it impossible to have
+    // a KclManager without a ZDSProject.
+    if (path !== this.path || !this.systemDeps.projectPath) {
+      return
+    }
+    // Your current file is changed, read it from disk and write it into the code manager and execute the AST,
+    // unless the change was initiated by us (the currently running instance).
+    window.electron
+      ?.readFile(path, {
+        encoding: 'utf-8',
+      })
+      .then((code) => {
+        const isInSketchMode =
+          this.modelingState?.matches('Sketch') ||
+          this.modelingState?.matches('sketchSolveMode')
+
+        if (!isCodeTheSame(code, this.code)) {
+          // Nothing written out yet by ourselves, or it's not the same as the current file content
+          // -> this must be an external change -> re-execute.
+          this.updateCodeEditor(code, {
+            shouldExecute: !isInSketchMode,
+            shouldResetCamera: !isInSketchMode,
+            // We explicitly do not write to the file here since we are loading from
+            // the file system and not the editor.
+            shouldWriteToDisk: false,
+          })
+
+          toast('Reloading file from disk', { icon: '📁' })
+        }
+      })
+      .catch(reportRejection)
+  }
   private _wasmInitFailed = signal<boolean | undefined>(undefined)
   private _astParseFailed = false
   private _switchedFiles = false
@@ -450,11 +499,11 @@ export class KclManager extends EventTarget {
     undefined
   public writeCausedByAppCheckedInFileTreeFileSystemWatcher = false
   public mlEphantManagerMachineBulkManipulatingFileSystem = false
-  // The last code written by the app, used to compare against external changes to the current file
-  public lastWrite: {
-    code: string // last code written by ZDS
-    time: number // Unix epoch time in milliseconds
-  } | null = null
+  /**
+    Indicator Promise that is pending while a live write is happening.
+    If this value isn't `null`, don't watch for file system writes it was probably us!
+   */
+  public writingPromise = signal<Promise<unknown> | null>(null)
   public isBufferMode = false
   sceneInfraBaseUnitMultiplierSetter: (unit: BaseUnit) => void = () => {}
   /** Values merged in from former EditorManager and CodeManager classes */
@@ -751,7 +800,7 @@ export class KclManager extends EventTarget {
         console.error('Error when updating Rust state after user edit:', error)
       }
     },
-    300
+    1000
   )
 
   private createEditorExtensions() {
@@ -826,6 +875,18 @@ export class KclManager extends EventTarget {
         this._wasmInitFailed.value = true
         reportRejection(e)
       })
+
+    // Register a file watcher for this file.
+    window.electron?.watchFileOn(
+      path,
+      this.fileWatcherKey,
+      this.onFileWatchEvent
+    )
+  }
+
+  /** Clean up listeners, watchers, etc */
+  public close() {
+    window.electron?.watchFileOff(this.path, this.fileWatcherKey)
   }
 
   clearAst() {
@@ -1854,7 +1915,6 @@ export class KclManager extends EventTarget {
   updateCurrentFilePath(path: string) {
     if (this._currentFilePath !== path) {
       this._currentFilePath = path
-      this.lastWrite = null
     }
   }
   get currentFileName() {
@@ -1954,17 +2014,14 @@ export class KclManager extends EventTarget {
       // writes.
       clearTimeout(this.timeoutWriter)
       return new Promise((resolve, reject) => {
-        this.lastWrite = {
-          code: newCode ?? '',
-          time: Date.now(),
-        }
         this.timeoutWriter = setTimeout(() => {
           if (!path) {
             return reject(new Error('currentFilePath not set'))
           }
           // Wait one event loop to give a chance for params to be set
           // Save the file to disk
           this.writeCausedByAppCheckedInFileTreeFileSystemWatcher = true
+          window.electron?.watchFileOff(this.path, this.fileWatcherKey)
           fsZds
             .writeFile(path, new TextEncoder().encode(newCode))
             .then(resolve)

src/lib/app.ts

@@ -342,10 +342,11 @@ export class App implements AppSubsystems {
    * Build the world!
    */
   buildSingletons() {
-    const kclManager = new KclManager({
+    const kclManager = new KclManager('', {
       settings: this.settings.actor,
       wasmInstancePromise: this.wasmPromise,
       commandBar: this.commands.actor,
+      projectPath: '',
     })
 
     if (typeof window !== 'undefined') {

src/preload.ts

@@ -79,7 +79,7 @@ const watchFileOn = (
   if (!watchers) {
     watchers = new Map()
   }
-  const watcher = chokidar.watch(path, { depth: 1 })
+  const watcher = chokidar.watch(path, { depth: 1, ignoreInitial: true })
   watcher.on('all', callback)
   watchers.set(key, { watcher, callback })
   fsWatchListeners.set(path, watchers)