Skip to content

fix(process-runner): rebuild a cached runner when its workspace moves - #338

Open
kevin9327 wants to merge 1 commit into
lightningpixel:devfrom
kevin9327:fix/process-runner-stale-workspace
Open

kevin9327 wants to merge 1 commit into
lightningpixel:devfrom
kevin9327:fix/process-runner-stale-workspace

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What

After moving the workspace folder in Settings → Storage, workflow process nodes keep writing their output into the previous workspace folder until the app is restarted. The built-in Mesh Optimizer writes to <old workspace>/Workflows/ and Mesh Exporter to <old workspace>/Exports/, and Python process nodes are handed the old workspaceDir too. The workflow resolves a node's scene URL against the current workspace, so the result never appears in the viewer, and if the old folder was moved away it is silently recreated.

Why it triggers

extensions:runProcess reads the current workspaceDir from settings on every call and passes it to getProcessRunner / getPythonProcessRunner. But the runner registry is keyed only by extension id and ignores the arguments once a runner exists:

if (!registry.has(extensionId)) {
  registry.set(extensionId, new ProcessRunner(extDir, entry, workspaceDir, tempDir))
}
return registry.get(extensionId)! as ProcessRunner

The JS runner bakes workspaceDir into its worker's workerData, and the Python runner stores it on the instance and sends it with every spawn, so the workspace of the first run wins. Storage settings apply paths at runtime (api:updatePaths, no restart) and nothing terminates the runners on that path. #136 fixed this shape for extension installs and reloads by terminating runners, but a path change was never covered.

Fix

Remember the arguments each cached runner was built with. When a call's arguments differ, terminate the old runner and build a fresh one. Identical arguments keep returning the same warm runner, so a JS worker is still reused across runs.

Verification

New electron/main/process-runner.test.mjs, esbuild-bundled the same way as copy-runtime.test.mjs. Node stands in for the Python interpreter, so the test needs no Python install.

  • a JS process runner follows the workspace after it moves: fails before, passes after.
  • a Python process runner follows the workspace after it moves: fails before, passes after.
  • unchanged arguments keep reusing the same warm runner: passes before and after. It asserts that the same runner instance comes back and that its worker's module state carries over (run count 1 then 2). That proves the change doesn't widen into respawning a worker on every call.

Fail-before output, with the source reverted and the test kept. It is verbatim except that the local temp/checkout path prefix is replaced with <tmp> / <repo>:

✖ a JS process runner follows the workspace after it moves (111.4905ms)
✖ a Python process runner follows the workspace after it moves (134.8544ms)
✔ unchanged arguments keep reusing the same warm runner (60.8466ms)
ℹ tests 3
ℹ suites 0
ℹ pass 1
ℹ fail 2
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 429.3934

✖ failing tests:

test at electron\main\process-runner.test.mjs:64:1
✖ a JS process runner follows the workspace after it moves (111.4905ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
  + actual - expected
  
  + '<tmp>\\modly-runner-js-WuPMwu\\workspace-old'
  - '<tmp>\\modly-runner-js-WuPMwu\\workspace-new'
  
      at TestContext.<anonymous> (file:///<repo>/electron/main/process-runner.test.mjs:75:12)
      at async Test.run (node:internal/test_runner/test:1313:7)
      at async startSubtestAfterBootstrap (node:internal/test_runner/harness:385:3) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: '<tmp>\\modly-runner-js-WuPMwu\\workspace-old',
    expected: '<tmp>\\modly-runner-js-WuPMwu\\workspace-new',
    operator: 'strictEqual',
    diff: 'simple'
  }

test at electron\main\process-runner.test.mjs:81:1
✖ a Python process runner follows the workspace after it moves (134.8544ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
  + actual - expected
  
  + '<tmp>\\modly-runner-py-ghE9DV\\workspace-old'
  - '<tmp>\\modly-runner-py-ghE9DV\\workspace-new'
  
      at TestContext.<anonymous> (file:///<repo>/electron/main/process-runner.test.mjs:92:12)
      at process.processTicksAndRejections (node:internal/process/task_queues:104:5)
      at async Test.run (node:internal/test_runner/test:1313:7)
      at async Test.processPendingSubtests (node:internal/test_runner/test:897:7) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: '<tmp>\\modly-runner-py-ghE9DV\\workspace-old',
    expected: '<tmp>\\modly-runner-py-ghE9DV\\workspace-new',
    operator: 'strictEqual',
    diff: 'simple'
  }

After the fix:

✔ a JS process runner follows the workspace after it moves (323.1386ms)
✔ a Python process runner follows the workspace after it moves (244.7963ms)
✔ unchanged arguments keep reusing the same warm runner (71.2991ms)
ℹ tests 3
ℹ pass 3
ℹ fail 0

Whole suite (npm run test:node), unmodified dev compared with this branch:

dev this branch
TS tests 37 pass, 0 fail 37 pass, 0 fail
*.test.mjs 135 tests: 130 pass, 5 skipped, 0 fail 138 tests: 133 pass, 5 skipped, 0 fail
npx eslint . 0 problems 0 problems

The 5 skips were already there on dev. No Python touched.

🤖 Generated with Claude Code

Process-extension runners are cached per extension id and the cache
ignored the arguments of every call after the first. The workspace
folder is baked into each runner at construction, so after the
workspace is moved in Settings (which updates paths at runtime, without
a restart) workflow process nodes such as Mesh Optimizer kept writing
their output into the previous workspace, where the viewer can no
longer find it.

Remember the arguments each runner was built with and replace the
runner when they differ; identical arguments keep reusing the warm
worker as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant