From 0bd8fcbcbc490018868c59dfcbe1eff08aca6522 Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Tue, 15 Sep 2026 22:17:46 +0900 Subject: [PATCH 1/7] test: reproduce aicshud/WHEEL#1020 (red) Adds failing tests demonstrating that runProject() and stopProject() independently call removeSsh()/removeExecuters()/removeTransferrers() (and runProject() also runDeferredCleanups()), which race each other when a project is stopped mid-run: Dispatcher.pause() resolves start()'s promise (via a synchronous "stop" emit) well before its own, still-in-flight nested job-task cancellation (pjdel etc., which needs the project's SSH connections) actually completes, so runProject()'s teardown can disconnect SSH out from under stopProject()'s still- running cancellation - observed as "ssh instance is not registerd for the project", which aborts onStopProject() before it ever reaches its own explicit "stopped" state write. - dispatcher.js: Dispatcher#start()'s onStop handler should set a new `stoppedExternally` flag before resolving, so callers can tell this settled via an external stop rather than natural completion. - core/projectController.js: runProject() should skip its own removeSsh/removeExecuters/removeTransferrers/runDeferredCleanups when the dispatcher was stopped externally, deferring entirely to stopProject()'s own (already fully-awaited) teardown instead. Confirmed red: 3 failing (server/scratchpad/jobmanager-repro-test-output.log). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/test/app/core/dispatcher.js | 24 ++++++++++++ server/test/app/core/projectController.js | 48 +++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/server/test/app/core/dispatcher.js b/server/test/app/core/dispatcher.js index c0d881fa..488e976c 100644 --- a/server/test/app/core/dispatcher.js +++ b/server/test/app/core/dispatcher.js @@ -1595,6 +1595,30 @@ describe("UT for Dispatcher class", function () { }); }); + //reproduction for aicshud/WHEEL#1020: start()'s promise must record that it settled via an + //external stop (pause()/remove(), e.g. from stopProject()) rather than natural completion, + //so callers (runProject()) can tell not to also run their own teardown - see #1020's + //investigation for why racing that teardown disconnects SSH out from under stopProject()'s + //still-in-flight nested job cancellation. + describe("#start stoppedExternally flag (aicshud/WHEEL#1020)", ()=>{ + it("should set stoppedExternally and resolve when stopped via pause() before any component is dispatched", async ()=>{ + const projectJson = await fs.readJson(path.resolve(projectRootDir, projectJsonFilename)); + const DP = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + const startPromise = DP.start(); + await DP.pause(); + const state = await startPromise; + expect(DP.stoppedExternally).to.be.true; + expect(state).to.equal("finished"); + }); + + it("should NOT set stoppedExternally when the dispatcher finishes naturally", async ()=>{ + const projectJson = await fs.readJson(path.resolve(projectRootDir, projectJsonFilename)); + const DP = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + await DP.start(); + expect(DP.stoppedExternally).to.not.be.true; + }); + }); + describe("#_checkMandatoryInputFilesExist", ()=>{ let task; beforeEach(async ()=>{ diff --git a/server/test/app/core/projectController.js b/server/test/app/core/projectController.js index 09c83b28..b6948af1 100644 --- a/server/test/app/core/projectController.js +++ b/server/test/app/core/projectController.js @@ -20,6 +20,7 @@ import chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); import { _internal, runProject, stopProject, cleanProject, updateProjectState } from "../../../app/core/projectController.js"; +import Dispatcher from "../../../app/core/dispatcher.js"; //test data const testDirRoot = "WHEEL_TEST_TMP"; @@ -1142,6 +1143,53 @@ describe("project Controller UT", function () { expect(result).to.be.an("error"); expect(result.message).to.include("project is already running"); }); + + //reproduction for aicshud/WHEEL#1020: when a Dispatcher settles because it was stopped + //externally (stopProject() -> Dispatcher.remove() -> pause(), which resolves start()'s + //promise as soon as it emits "stop" - well before its own, still-in-flight nested + //cancellation of remote job tasks is done using the project's SSH connections), + //runProject() must not also tear down removeSsh/removeExecuters/removeTransferrers here: + //stopProject() already owns that once its own await on rootDispatcher.remove() finishes, + //and racing it by doing it again here can disconnect SSH out from under that still-running + //cancellation (observed as "ssh instance is not registerd for the project"). + describe("when the dispatcher was stopped externally", ()=>{ + let removeSshStub, removeExecutersStub, removeTransferrersStub, runDeferredCleanupsStub; + beforeEach(()=>{ + sinon.stub(Dispatcher.prototype, "start").callsFake(function () { + this.stoppedExternally = true; + return Promise.resolve("stopped"); + }); + removeSshStub = sinon.stub(_internal, "removeSsh"); + removeExecutersStub = sinon.stub(_internal, "removeExecuters"); + removeTransferrersStub = sinon.stub(_internal, "removeTransferrers"); + runDeferredCleanupsStub = sinon.stub(_internal, "runDeferredCleanups").resolves(); + }); + it("should not call removeSsh/removeExecuters/removeTransferrers/runDeferredCleanups", async ()=>{ + await runProject(projectRootDir); + sinon.assert.notCalled(removeSshStub); + sinon.assert.notCalled(removeExecutersStub); + sinon.assert.notCalled(removeTransferrersStub); + sinon.assert.notCalled(runDeferredCleanupsStub); + }); + }); + + describe("when the dispatcher finishes naturally (not stopped externally)", ()=>{ + let removeSshStub, removeExecutersStub, removeTransferrersStub, runDeferredCleanupsStub; + beforeEach(()=>{ + sinon.stub(Dispatcher.prototype, "start").resolves("finished"); + removeSshStub = sinon.stub(_internal, "removeSsh"); + removeExecutersStub = sinon.stub(_internal, "removeExecuters"); + removeTransferrersStub = sinon.stub(_internal, "removeTransferrers"); + runDeferredCleanupsStub = sinon.stub(_internal, "runDeferredCleanups").resolves(); + }); + it("should still call removeSsh/removeExecuters/removeTransferrers/runDeferredCleanups", async ()=>{ + await runProject(projectRootDir); + sinon.assert.calledOnceWithExactly(removeSshStub, projectRootDir); + sinon.assert.calledOnceWithExactly(removeExecutersStub, projectRootDir); + sinon.assert.calledOnceWithExactly(removeTransferrersStub, projectRootDir); + sinon.assert.calledOnceWithExactly(runDeferredCleanupsStub, projectRootDir); + }); + }); }); describe("#stopProject", ()=>{ const projectRootDir = "/test/project"; From dc6d0a7deeb265bfa3558ae740f0d431336bf174 Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Tue, 15 Sep 2026 22:24:43 +0900 Subject: [PATCH 2/7] fix: don't race stopProject()'s teardown with runProject()'s own (aicshud/WHEEL#1020) Dispatcher#start()'s onStop handler now sets `stoppedExternally = true` before resolving, recording that this settlement came from an external stop (pause()/remove(), as triggered by stopProject()) rather than the dispatcher's own natural completion. pause() emits "stop" synchronously before awaiting its own nested job-task cancellation (pjdel etc., which needs the project's SSH connections), so start()'s promise - and therefore runProject()'s await on it - can resolve well before that cancellation actually finishes. runProject() now skips its own removeSsh/removeExecuters/ removeTransferrers/runDeferredCleanups teardown when rootDispatcher.stoppedExternally is set, instead deferring entirely to stopProject()'s own teardown (which only runs those once its own await on rootDispatcher.remove() - including all nested pause() calls - has fully finished). Previously, runProject()'s teardown could win the race and disconnect SSH out from under stopProject()'s still-running nested cancellation, surfacing as "ssh instance is not registerd for the project" and aborting onStopProject() before it ever reached its own explicit "stopped" state write. Full server test suite: 1656 passing, 0 failing, 15 pending, including both new #1020 reproduction tests. Test output: server/scratchpad/jobmanager-repro-test-output.log Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/app/core/dispatcher.js | 10 ++++++++++ server/app/core/projectController.js | 21 ++++++++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/server/app/core/dispatcher.js b/server/app/core/dispatcher.js index 9d8a5046..9f851d13 100644 --- a/server/app/core/dispatcher.js +++ b/server/app/core/dispatcher.js @@ -518,6 +518,16 @@ class Dispatcher extends EventEmitter { //start()'s promise with the current outcome instead of leaving it pending forever - //otherwise the caller (runProject()) hangs indefinitely and never reaches its own //state update/cleanup, which in turn leaves the project stuck instead of concluding. + // + //record that this was an external stop (aicshud/WHEEL#1020): pause()/remove() (which + //triggered this) is still busy recursively canceling nested job tasks - using the + //project's SSH connections - well after this resolves, since pause() emits "stop" + //synchronously before awaiting that cancellation. The caller (runProject()) must not + //also run its own SSH/executer/transferrer teardown once this resolves - that races + //the still-in-flight cancellation and can disconnect SSH out from under it. Whoever + //called pause()/remove() (stopProject()) already owns that teardown once its own + //await on remove() finishes. + this.stoppedExternally = true; resolve(this._getState()); }; this.once("done", this.onDone); diff --git a/server/app/core/projectController.js b/server/app/core/projectController.js index 144b35f7..ee6ab5b0 100644 --- a/server/app/core/projectController.js +++ b/server/app/core/projectController.js @@ -23,7 +23,11 @@ const _internal = { eventEmitters, gitClean, gitResetHEAD, - setProjectState + setProjectState, + removeSsh, + removeExecuters, + removeTransferrers, + runDeferredCleanups }; /** @@ -123,10 +127,17 @@ async function runProject(projectRootDir) { await updateProjectState(projectRootDir, rootWF.state, projectJson); await writeComponentJson(projectRootDir, projectRootDir, rootWF, true); _internal.rootDispatchers.delete(projectRootDir); - await runDeferredCleanups(projectRootDir); - removeExecuters(projectRootDir); - removeTransferrers(projectRootDir); - removeSsh(projectRootDir); + //if the dispatcher settled via an external stop (stopProject() -> Dispatcher.remove() -> + //pause()), that caller already owns this teardown once its own await on remove() finishes - + //doing it again here races it, since start() now resolves as soon as pause() emits "stop", + //well before its still-in-flight nested job cancellation is done using these same SSH + //connections (aicshud/WHEEL#1020). + if (!rootDispatcher.stoppedExternally) { + await _internal.runDeferredCleanups(projectRootDir); + _internal.removeExecuters(projectRootDir); + _internal.removeTransferrers(projectRootDir); + _internal.removeSsh(projectRootDir); + } return rootWF.state; } From 3114e1e6be4efd0a73f10b3bd34832fb5db034c5 Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Tue, 15 Sep 2026 23:58:29 +0900 Subject: [PATCH 3/7] fix: stop runDispatcher()'s own removeSsh from racing stopProject() too (aicshud/WHEEL#1020) The previous #1020 commit (f2996dc7) only guarded core/projectController.js's runProject() against redundantly tearing down SSH/executers/transferrers when the dispatcher was stopped externally. It missed a third, independent call site: handlers/projectController.js's runDispatcher() - the function that actually calls runProject() - had its own unconditional removeSsh(projectRootDir) in a shared `finally` block that runs on every exit path (success, external stop, or error), completely bypassing runProject()'s stoppedExternally guard. Since runProject() now returns almost immediately once "stop" is emitted, this outer finally block's removeSsh could still fire while stopProject()'s own, separate (concurrent, fire-and-forget) invocation was still mid-flight cancelling nested job tasks over SSH - reproducing the exact same "ssh instance is not registerd for the project" error the first commit was meant to fix. stopProject() only ever settles a Dispatcher's start() via the "stop" event (never "error"), so runProject() never throws when a stop (user-initiated or task-failure-triggered) is in progress - meaning this outer removeSsh call is only ever needed as a safety net for the genuine, rare case where runProject() itself threw before reaching its own cleanup (e.g. rootDispatcher.start() rejecting via the dispatcher's "error" event). Moved it (and removeAllJWTServerPassphrase) from the shared `finally` into the `catch` block, where it can't race a concurrent stopProject() at all - runProject() now unconditionally owns cleanup on every non-throwing exit. Verified on Fugaku with debug instrumentation (temporary, removed before this commit): confirmed removeSsh was called exactly once, from stopProject()'s own (correct) call site, with no "ssh instance is not registerd" error, and the project's persisted state ended up correctly as "stopped". Repeated 3 consecutive clean->run->stop cycles with no recurrence (this was a race condition, hence the repeat). Full server test suite: 1656 passing, 0 failing, 15 pending. Test output: server/scratchpad/jobmanager-repro-test-output.log Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/app/handlers/projectController.js | 11 ++++++++-- server/test/app/handlers/projectController.js | 22 ++++++++++--------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/server/app/handlers/projectController.js b/server/app/handlers/projectController.js index 3a70c0aa..45c79a39 100644 --- a/server/app/handlers/projectController.js +++ b/server/app/handlers/projectController.js @@ -393,6 +393,15 @@ async function runDispatcher(clientID, projectRootDir, ack) { } catch (err) { notifyUser(projectRootDir, "fatal error occurred while parsing workflow:", err); await updateProjectState(projectRootDir, "failed"); + //runProject() (core/projectController.js) owns removeSsh/removeExecuters/ + //removeTransferrers on every non-throwing exit (natural completion or external stop, + //aicshud/WHEEL#1020) - only do it here as a safety net for the case where runProject() + //threw before ever reaching its own cleanup (e.g. rootDispatcher.start() itself + //rejected). Doing this unconditionally in a shared `finally` below used to always fire + //on every exit path, including the external-stop one - racing stopProject()'s own + //still-in-flight teardown of the very same SSH connections. + removeSsh(projectRootDir); + removeAllJWTServerPassphrase(projectRootDir); ack(err); } finally { //make sure any in-flight abort-on-failure work has fully settled before this handler @@ -401,8 +410,6 @@ async function runDispatcher(clientID, projectRootDir, ack) { emitAll(projectRootDir, "projectJson", await getProjectJson(projectRootDir)); await _internal.sendWorkflow(ack, projectRootDir); _internal.eventEmitters.delete(projectRootDir); - removeSsh(projectRootDir); - removeAllJWTServerPassphrase(projectRootDir); } return; } diff --git a/server/test/app/handlers/projectController.js b/server/test/app/handlers/projectController.js index eab97e80..73945ea0 100644 --- a/server/test/app/handlers/projectController.js +++ b/server/test/app/handlers/projectController.js @@ -39,16 +39,18 @@ const scriptPwd = `${scriptHeader}\n${pwdCmd}`; /** * Wait until a project's dispatch has fully wound down after its ack has already * fired. onRunProject/onContinueProject fire-and-forget the actual dispatch - * (runDispatcher), whose finally block calls ack (via sendWorkflow) *before* its - * own remaining cleanup (eventEmitters.delete/removeSsh/removeAllJWTServerPassphrase) - * runs, and dispatcher itself never awaits individual task execution (by design, - * so a canceled task isn't left blocked forever inside it) - so a test that only - * awaits the ack can still race the next test's beforeEach (which removes/ - * recreates projectRootDir) against trailing dispatcher/task cleanup work, - * intermittently crashing with ENOENT on stale .git/objects files. Poll for the - * eventEmitter runDispatcher registers for the run (and clears right before its - * own trailing cleanup) as a best-effort signal, then add a short fixed grace - * period on top for any other fire-and-forgotten work outside that scope. + * (runDispatcher), whose finally block calls ack (via sendWorkflow) *before* + * eventEmitters.delete runs (removeSsh/removeAllJWTServerPassphrase only run there + * too if runProject() itself threw before reaching its own cleanup - see + * aicshud/WHEEL#1020 - otherwise runProject() already handled them), and dispatcher + * itself never awaits individual task execution (by design, so a canceled task + * isn't left blocked forever inside it) - so a test that only awaits the ack can + * still race the next test's beforeEach (which removes/recreates projectRootDir) + * against trailing dispatcher/task cleanup work, intermittently crashing with + * ENOENT on stale .git/objects files. Poll for the eventEmitter runDispatcher + * registers for the run (and clears right before its own trailing cleanup) as a + * best-effort signal, then add a short fixed grace period on top for any other + * fire-and-forgotten work outside that scope. * @param {string} projectRootDir - project's root path * @param {number} timeoutMs - give up polling and move on to the grace period after this long */ From 4a175cffead997d266919af1e88c3b7be26868a8 Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Wed, 16 Sep 2026 01:00:14 +0900 Subject: [PATCH 4/7] test: reproduce aicshud/WHEEL#1021 (red) Adds a failing test demonstrating that stopProject() discards pending deferred remote cleanups (files preserved as remote-symlink targets for a not-yet-executed downstream task) via clearDeferredCleanups(). This permanently leaks them: a resumed run never re-registers the entry (finished components are skipped on restart), and cleanProject only touches local git state, never the remote host - so once an entry is cleared here, nothing ever deletes the corresponding remote files. Confirmed red: registering a deferred-cleanup entry, calling stopProject(), then calling runDeferredCleanups() again finds nothing to process (getSsh never called), because stopProject() already wiped the registry. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/test/app/core/projectController.js | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/server/test/app/core/projectController.js b/server/test/app/core/projectController.js index b6948af1..ad3bfc32 100644 --- a/server/test/app/core/projectController.js +++ b/server/test/app/core/projectController.js @@ -21,6 +21,7 @@ chai.use(chaiAsPromised); import { _internal, runProject, stopProject, cleanProject, updateProjectState } from "../../../app/core/projectController.js"; import Dispatcher from "../../../app/core/dispatcher.js"; +import { runDeferredCleanups, _internal as transferrerInternal } from "../../../app/core/transferrer.js"; //test data const testDirRoot = "WHEEL_TEST_TMP"; @@ -1207,6 +1208,29 @@ describe("project Controller UT", function () { sinon.assert.calledOnce(mockDispatcher.remove); expect(_internal.rootDispatchers.has(projectRootDir)).to.be.false; }); + //reproduction for aicshud/WHEEL#1021: stopProject() must not discard pending deferred + //remote cleanups (files preserved as remote-symlink targets for a not-yet-executed + //downstream task) - doing so leaks them permanently, since a resumed run never + //re-registers them (finished components are skipped) and cleanProject only touches + //local git state, never the remote host. + it("should NOT discard entries registered in the deferred-cleanup registry", async ()=>{ + const getSshStub = sinon.stub(transferrerInternal, "getSsh").returns({ exec: sinon.stub().resolves() }); + transferrerInternal.addDeferredCleanup(projectRootDir, { + remoteWorkingDir: "/remote/work/dir", + remotehostID: "dummyHostID", + symlinkTargetNames: ["result.txt"] + }); + + try { + await stopProject(projectRootDir); + //if the entry survived stopProject(), a later runDeferredCleanups() (e.g. after a + //resume eventually finishes naturally) must still find and process it. + await runDeferredCleanups(projectRootDir); + sinon.assert.calledOnce(getSshStub); + } finally { + getSshStub.restore(); + } + }); it("should handle the case where the dispatcher does not exist", async ()=>{ _internal.rootDispatchers.delete(projectRootDir); await stopProject(projectRootDir); From 36883a6a4b9140b836022e7ca65dfb1ce13a2456 Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Wed, 16 Sep 2026 01:11:22 +0900 Subject: [PATCH 5/7] fix(projectController): stop discarding pending deferred remote cleanups on stop stopProject() called clearDeferredCleanups(), which silently discards any entries in the deferred-cleanup registry without deleting the remote files they describe. Those entries exist precisely because a finished task's output was delivered downstream via a remote symlink whose target must be preserved for a not-yet-executed consumer; discarding the entry on stop means that consumer's target file leaks on the remote host forever, since no later resume-to-completion or cleanProject can ever re-register it. Simply stop calling clearDeferredCleanups() in stopProject() and leave the registry untouched, so a later natural completion (after 0+ resumes) can still process it via runDeferredCleanups(). Running the deferred cleanup immediately at stop time was considered and rejected: at stop time we cannot safely tell whether the not-yet-executed downstream consumer has already run, so deleting immediately risks removing a file it still needs. Ref: aicshud/WHEEL#1021 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/app/core/projectController.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/app/core/projectController.js b/server/app/core/projectController.js index ee6ab5b0..1522d439 100644 --- a/server/app/core/projectController.js +++ b/server/app/core/projectController.js @@ -9,7 +9,7 @@ import { gitResetHEAD, gitClean } from "../core/gitOperator2.js"; import { removeSsh } from "./sshManager.js"; import { removeExecuters } from "./executerManager.js"; import { removeTransferrers } from "./transferManager.js"; -import { runDeferredCleanups, clearDeferredCleanups } from "./transferrer.js"; +import { runDeferredCleanups } from "./transferrer.js"; import { defaultCleanupRemoteRoot, projectJsonFilename, componentJsonFilename } from "../db/db.js"; import { setProjectState } from "../core/projectJsonFileOperator.js"; import { writeComponentJson } from "./componentJsonIO.js"; @@ -89,7 +89,11 @@ async function stopProject(projectRootDir, reasonState) { await rootDispatcher.remove(); _internal.rootDispatchers.delete(projectRootDir); } - clearDeferredCleanups(projectRootDir); + //deliberately do NOT discard pending deferred remote cleanups here (aicshud/WHEEL#1021): + //an entry means a not-yet-executed downstream task may still need the preserved + //remote-symlink target file on resume, and there is no way to safely tell from here + //whether that consumer has already run. Leave the registry as-is so a later natural + //completion (after 0+ resumes) can still process it via runDeferredCleanups(). removeExecuters(projectRootDir); removeTransferrers(projectRootDir); removeSsh(projectRootDir); From 5d1f2f468a62906b601ef56deaf3c8596f0fd9e8 Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Wed, 16 Sep 2026 01:29:03 +0900 Subject: [PATCH 6/7] test: reproduce aicshud/WHEEL#1022 (red) runDispatcher()'s catch block (handlers/projectController.js) is the last chance to release SSH connections/executers/transferrers when runProject() throws before ever reaching its own cleanup (e.g. rootDispatcher.start() itself rejecting, not a task failure). It only ever called removeSsh(), never removeExecuters()/removeTransferrers() - leaking stale executer/ transferrer map entries that a subsequent run attempt could reuse. Ref: aicshud/WHEEL#1022 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/test/app/handlers/projectController.js | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/server/test/app/handlers/projectController.js b/server/test/app/handlers/projectController.js index 73945ea0..b816b14a 100644 --- a/server/test/app/handlers/projectController.js +++ b/server/test/app/handlers/projectController.js @@ -22,6 +22,7 @@ import allowedOperations from "../../../../common/allowedOperations.js"; //helper functions import { _internal as coreProjectControllerInternal } from "../../../app/core/projectController.js"; +import Dispatcher from "../../../app/core/dispatcher.js"; import { createNewProject } from "../../../app/core/projectOperations.js"; import { createNewComponent } from "../../../app/core/componentOperations.js"; import { addLink } from "../../../app/core/componentLinks.js"; @@ -477,4 +478,26 @@ describe("project Controller handler UT", function () { await drainProjectDispatch(projectRootDir); }); }); + + describe("[reproduction] issue aicshud/WHEEL#1022 - removeExecuters/removeTransferrers are not released on runDispatcher's fatal-error path", ()=>{ + it("should call removeExecuters/removeTransferrers as well as removeSsh when runProject() throws before ever reaching its own cleanup", async ()=>{ + //simulate rootDispatcher.start() itself rejecting (e.g. a bug in the dispatch loop + //itself, not a task failure) - this is the one path where runProject() throws before + //ever reaching its own removeSsh/removeExecuters/removeTransferrers teardown, so + //runDispatcher()'s own catch block is the last chance to release these resources. + sinon.stub(Dispatcher.prototype, "start").rejects(new Error("dummy dispatch failure")); + const removeExecutersStub = sinon.stub(coreProjectControllerInternal, "removeExecuters"); + const removeTransferrersStub = sinon.stub(coreProjectControllerInternal, "removeTransferrers"); + const removeSshStub = sinon.stub(coreProjectControllerInternal, "removeSsh"); + + await new Promise((resolve)=>{ + _internal.onRunProject("test-client-id", projectRootDir, resolve); + }); + await drainProjectDispatch(projectRootDir); + + sinon.assert.calledOnceWithExactly(removeSshStub, projectRootDir); + sinon.assert.calledOnceWithExactly(removeExecutersStub, projectRootDir); + sinon.assert.calledOnceWithExactly(removeTransferrersStub, projectRootDir); + }); + }); }); From 5716210ac043f0a31a5f846e1b9fd3e82b24364a Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Wed, 16 Sep 2026 01:29:14 +0900 Subject: [PATCH 7/7] refactor(projectController): consolidate the removeSsh/removeExecuters/removeTransferrers cleanup set Every path that tears down a project run's SSH connections, executers, and transferrers (natural completion, an external stop, and a fatal dispatch error) used to hand-roll its own subset of these three calls, spread across core/projectController.js's stopProject()/runProject() and handlers/projectController.js's runDispatcher(). This is how runDispatcher()'s catch block came to only call removeSsh() and silently skip removeExecuters()/removeTransferrers() (aicshud/WHEEL#1022) - a genuine implementation gap, not an intentional omission, since both functions are pure in-memory Map cleanups with no side effects that make them unsafe to call unconditionally. Introduce releaseRuntimeResources(projectRootDir) in core/projectController.js as the single shared implementation of this 3-point cleanup set, and call it from all three sites: - stopProject() - runProject()'s guarded tail (only on natural completion, unchanged from aicshud/WHEEL#1020) - runDispatcher()'s catch block, now also releasing executers/transferrers This is a pure consolidation of already-reviewed cleanup logic (no new runtime behavior beyond closing the #1022 gap above) - deferred cleanups (runDeferredCleanups) and JWT passphrase cleanup are deliberately left out of this shared function, since they are not called uniformly from all three sites and have their own separate semantics. Ref: aicshud/WHEEL#1022 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/app/core/projectController.js | 27 ++++++++++++++++++------ server/app/handlers/projectController.js | 20 ++++++++++-------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/server/app/core/projectController.js b/server/app/core/projectController.js index 1522d439..8529466a 100644 --- a/server/app/core/projectController.js +++ b/server/app/core/projectController.js @@ -30,6 +30,23 @@ const _internal = { runDeferredCleanups }; +/** + * release the runtime resources (SSH connections, executers, transferrers) a project run + * accumulated - the 3-point cleanup set shared by every path that tears down a run + * (natural completion, an external stop, or a fatal dispatch error), consolidated here so + * every call site releases the same three things instead of each hand-rolling its own + * subset (aicshud/WHEEL#1022 - removeExecuters/removeTransferrers used to be missing from + * runDispatcher()'s fatal-error path, leaking stale executer/transferrer map entries that a + * subsequent run attempt could otherwise reuse). + * @param {string} projectRootDir - project's root path + */ +function releaseRuntimeResources(projectRootDir) { + _internal.removeExecuters(projectRootDir); + _internal.removeTransferrers(projectRootDir); + _internal.removeSsh(projectRootDir); +} +_internal.releaseRuntimeResources = releaseRuntimeResources; + /** * @event projectStateChanged * @type {object} - updated projectJson @@ -94,9 +111,7 @@ async function stopProject(projectRootDir, reasonState) { //remote-symlink target file on resume, and there is no way to safely tell from here //whether that consumer has already run. Leave the registry as-is so a later natural //completion (after 0+ resumes) can still process it via runDeferredCleanups(). - removeExecuters(projectRootDir); - removeTransferrers(projectRootDir); - removeSsh(projectRootDir); + _internal.releaseRuntimeResources(projectRootDir); //project state must be updated by onStopProject() } @@ -138,11 +153,9 @@ async function runProject(projectRootDir) { //connections (aicshud/WHEEL#1020). if (!rootDispatcher.stoppedExternally) { await _internal.runDeferredCleanups(projectRootDir); - _internal.removeExecuters(projectRootDir); - _internal.removeTransferrers(projectRootDir); - _internal.removeSsh(projectRootDir); + _internal.releaseRuntimeResources(projectRootDir); } return rootWF.state; } -export { cleanProject, runProject, stopProject, updateProjectState, _internal }; +export { cleanProject, runProject, stopProject, updateProjectState, releaseRuntimeResources, _internal }; diff --git a/server/app/handlers/projectController.js b/server/app/handlers/projectController.js index 45c79a39..03e7c5db 100644 --- a/server/app/handlers/projectController.js +++ b/server/app/handlers/projectController.js @@ -21,7 +21,7 @@ import { checkRemoteStoragePathWritePermission } from "../core/checkRemoteStorag import { getProjectJson, getProjectState, setProjectState, updateProjectDescription, updateProjectROStatus } from "../core/projectJsonFileOperator.js"; import { createSsh, removeSsh, askPassword } from "../core/sshManager.js"; import { setJWTServerPassphrase, removeAllJWTServerPassphrase } from "../core/jwtServerPassphraseManager.js"; -import { runProject, cleanProject, stopProject } from "../core/projectController.js"; +import { runProject, cleanProject, stopProject, releaseRuntimeResources } from "../core/projectController.js"; import { isValidOutputFilename } from "../lib/utility.js"; import { checkWritePermissions, parentDirs, eventEmitters } from "../core/global.js"; import { sendWorkflow, sendProjectJson, sendTaskStateList, sendResultsFileDir, sendComponentTree } from "./senders.js"; @@ -393,14 +393,16 @@ async function runDispatcher(clientID, projectRootDir, ack) { } catch (err) { notifyUser(projectRootDir, "fatal error occurred while parsing workflow:", err); await updateProjectState(projectRootDir, "failed"); - //runProject() (core/projectController.js) owns removeSsh/removeExecuters/ - //removeTransferrers on every non-throwing exit (natural completion or external stop, - //aicshud/WHEEL#1020) - only do it here as a safety net for the case where runProject() - //threw before ever reaching its own cleanup (e.g. rootDispatcher.start() itself - //rejected). Doing this unconditionally in a shared `finally` below used to always fire - //on every exit path, including the external-stop one - racing stopProject()'s own - //still-in-flight teardown of the very same SSH connections. - removeSsh(projectRootDir); + //runProject() (core/projectController.js) owns releaseRuntimeResources() (removeSsh/ + //removeExecuters/removeTransferrers) on every non-throwing exit (natural completion or + //external stop, aicshud/WHEEL#1020) - only do it here as a safety net for the case where + //runProject() threw before ever reaching its own cleanup (e.g. rootDispatcher.start() + //itself rejected). Doing this unconditionally in a shared `finally` below used to always + //fire on every exit path, including the external-stop one - racing stopProject()'s own + //still-in-flight teardown of the very same SSH connections. This used to call removeSsh() + //alone, leaking stale removeExecuters()/removeTransferrers() entries on this path + //(aicshud/WHEEL#1022) - use the same 3-point cleanup every other exit path uses instead. + releaseRuntimeResources(projectRootDir); removeAllJWTServerPassphrase(projectRootDir); ack(err); } finally {