ensure we never execute over ourselves (#3419)

* ensure we never execute over ourselves

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fixups

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* cleanup

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* weird logs

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix flake

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* make faster

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>

e2e/playwright/regression-tests.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect } from '@playwright/test'
+import { test, expect, Page } from '@playwright/test'
 
 import { getUtils, setup, tearDown } from './test-utils'
 import { TEST_CODE_TRIGGER_ENGINE_EXPORT_ERROR } from './storageStates'
@@ -346,55 +346,24 @@ const sketch001 = startSketchAt([-0, -0])
     // expect zero errors in guter
     await expect(page.locator('.cm-lint-marker-error')).not.toBeVisible()
 
-    // export the model
-    const exportButton = page.getByTestId('export-pane-button')
-    await expect(exportButton).toBeVisible()
-
-    // Click the export button
-    exportButton.click()
-
-    // Click the stl.
-    const stlOption = page.getByText('glTF')
-    await expect(stlOption).toBeVisible()
-
-    await page.keyboard.press('Enter')
-
-    // Click the checkbox
-    const submitButton = page.getByText('Confirm Export')
-    await expect(submitButton).toBeVisible()
-
-    await page.keyboard.press('Enter')
-
-    // Find the toast.
-    // Look out for the toast message
-    const exportingToastMessage = page.getByText(`Exporting...`)
-    await expect(exportingToastMessage).toBeVisible()
-
     const errorToastMessage = page.getByText(`Error while exporting`)
-
+    const exportingToastMessage = page.getByText(`Exporting...`)
     const engineErrorToastMessage = page.getByText(`Nothing to export`)
-
     const alreadyExportingToastMessage = page.getByText(`Already exporting`)
 
-    // Try exporting again.
+    await clickExportButton(page)
-    // Click the export button
-    exportButton.click()
 
-    // Click the stl.
+    await expect(exportingToastMessage).toBeVisible()
-    await expect(stlOption).toBeVisible()
 
-    await page.keyboard.press('Enter')
+    await clickExportButton(page)
-
-    // Click the checkbox
-    await expect(submitButton).toBeVisible()
-
-    await page.keyboard.press('Enter')
 
     // Find the toast.
     // Look out for the toast message
     await expect(exportingToastMessage).toBeVisible()
     await expect(alreadyExportingToastMessage).toBeVisible()
 
+    await page.waitForTimeout(1000)
+
     // Expect it to succeed.
     await expect(exportingToastMessage).not.toBeVisible()
     await expect(errorToastMessage).not.toBeVisible()
@@ -406,18 +375,7 @@ const sketch001 = startSketchAt([-0, -0])
     await expect(alreadyExportingToastMessage).not.toBeVisible()
 
     // Try exporting again.
-    // Click the export button
+    await clickExportButton(page)
-    exportButton.click()
-
-    // Click the stl.
-    await expect(stlOption).toBeVisible()
-
-    await page.keyboard.press('Enter')
-
-    // Click the checkbox
-    await expect(submitButton).toBeVisible()
-
-    await page.keyboard.press('Enter')
 
     // Find the toast.
     // Look out for the toast message
@@ -432,3 +390,24 @@ const sketch001 = startSketchAt([-0, -0])
     await expect(successToastMessage).toBeVisible()
   })
 })
+
+async function clickExportButton(page: Page) {
+  // export the model
+  const exportButton = page.getByTestId('export-pane-button')
+  await expect(exportButton).toBeVisible()
+
+  // Click the export button
+  exportButton.click()
+
+  // Click the stl.
+  const gltfOption = page.getByText('glTF')
+  await expect(gltfOption).toBeVisible()
+
+  await page.keyboard.press('Enter')
+
+  // Click the checkbox
+  const submitButton = page.getByText('Confirm Export')
+  await expect(submitButton).toBeVisible()
+
+  await page.keyboard.press('Enter')
+}

src/components/ModelingMachineProvider.tsx

@@ -149,8 +149,6 @@ export const ModelingMachineProvider = ({
         },
         'sketch exit execute': ({ store }) => {
           ;(async () => {
-            // blocks entering a sketch until after exit sketch code has run
-            kclManager.isExecuting = true
             sceneInfra.camControls.syncDirection = 'clientToEngine'
 
             await sceneInfra.camControls.snapToPerspectiveBeforeHandingBackControlToEngine()

src/lang/KclSingleton.ts

@@ -19,6 +19,16 @@ import { getNodeFromPath } from './queryAst'
 import { codeManager, editorManager, sceneInfra } from 'lib/singletons'
 import { Diagnostic } from '@codemirror/lint'
 
+interface ExecuteArgs {
+  ast?: Program
+  zoomToFit?: boolean
+  executionId?: number
+  zoomOnRangeAndType?: {
+    range: SourceRange
+    type: string
+  }
+}
+
 export class KclManager {
   private _ast: Program = {
     body: [],
@@ -36,6 +46,7 @@ export class KclManager {
   private _lints: Diagnostic[] = []
   private _kclErrors: KCLError[] = []
   private _isExecuting = false
+  private _executeIsStale: ExecuteArgs | null = null
   private _wasmInitFailed = true
 
   engineCommandManager: EngineCommandManager
@@ -113,9 +124,25 @@ export class KclManager {
   }
   set isExecuting(isExecuting) {
     this._isExecuting = isExecuting
+    // If we have finished executing, but the execute is stale, we should
+    // execute again.
+    if (!isExecuting && this.executeIsStale) {
+      const args = this.executeIsStale
+      this.executeIsStale = null
+      this.executeAst(args)
+    } else {
+    }
     this._isExecutingCallback(isExecuting)
   }
 
+  get executeIsStale() {
+    return this._executeIsStale
+  }
+
+  set executeIsStale(executeIsStale) {
+    this._executeIsStale = executeIsStale
+  }
+
   get wasmInitFailed() {
     return this._wasmInitFailed
   }
@@ -202,16 +229,16 @@ export class KclManager {
   // This NEVER updates the code, if you want to update the code DO NOT add to
   // this function, too many other things that don't want it exist.
   // just call to codeManager from wherever you want in other files.
-  async executeAst(
+  async executeAst(args: ExecuteArgs = {}): Promise<void> {
-    ast: Program = this._ast,
+    if (this.isExecuting) {
-    zoomToFit?: boolean,
+      this.executeIsStale = args
-    executionId?: number,
+      // Exit early if we are already executing.
-    zoomOnRangeAndType?: {
+      return
-      range: SourceRange
-      type: string
     }
-  ): Promise<void> {
+
-    const currentExecutionId = executionId || Date.now()
+    const ast = args.ast || this.ast
+
+    const currentExecutionId = args.executionId || Date.now()
     this._cancelTokens.set(currentExecutionId, false)
 
     this.isExecuting = true
@@ -229,12 +256,12 @@ export class KclManager {
     defaultSelectionFilter(programMemory, this.engineCommandManager)
     await this.engineCommandManager.waitForAllCommands()
 
-    if (zoomToFit) {
+    if (args.zoomToFit) {
       let zoomObjectId: string | undefined = ''
-      if (zoomOnRangeAndType) {
+      if (args.zoomOnRangeAndType) {
         zoomObjectId = this.engineCommandManager?.mapRangeToObjectId(
-          zoomOnRangeAndType.range,
+          args.zoomOnRangeAndType.range,
-          zoomOnRangeAndType.type
+          args.zoomOnRangeAndType.type
         )
       }
 
@@ -259,6 +286,7 @@ export class KclManager {
     }
 
     this.isExecuting = false
+
     // Check the cancellation token for this execution before applying side effects
     if (this._cancelTokens.get(currentExecutionId)) {
       this._cancelTokens.delete(currentExecutionId)
@@ -351,8 +379,7 @@ export class KclManager {
       return
     }
     this.ast = { ...ast }
-    this.isExecuting = true // executeAst sets this to false again
+    return this.executeAst({ zoomToFit })
-    return this.executeAst(ast, zoomToFit)
   }
   format() {
     const originalCode = codeManager.code
@@ -430,12 +457,11 @@ export class KclManager {
       codeManager.updateCodeEditor(newCode)
       // Write the file to disk.
       await codeManager.writeToFile()
-      await this.executeAst(
+      await this.executeAst({
-        astWithUpdatedSource,
+        ast: astWithUpdatedSource,
-        optionalParams?.zoomToFit,
+        zoomToFit: optionalParams?.zoomToFit,
-        undefined,
+        zoomOnRangeAndType: optionalParams?.zoomOnRangeAndType,
-        optionalParams?.zoomOnRangeAndType
+      })
-      )
     } else {
       // When we don't re-execute, we still want to update the program
       // memory with the new ast. So we will hit the mock executor

src/lang/std/artifactGraph.test.ts

@@ -136,7 +136,7 @@ beforeAll(async () => {
             console.error(ast)
             return Promise.reject(ast)
           }
-          await kclManager.executeAst(ast)
+          await kclManager.executeAst({ ast })
 
           cacheToWriteToFileTemp[codeKey] = {
             orderedCommands: engineCommandManager.orderedCommands,