From 1c4c2f7eb96fa5e335e563be15a006ddfc9dbedf Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Wed, 16 Sep 2026 17:39:57 +0900 Subject: [PATCH 1/2] test: reproduce aicshud/WHEEL#1028 (red) _getState() never returns "stopped", the documented state for an externally-stopped project (documentMD/user_guide/_reference/ 3_workflow_screen/1_graphview.md), even though start()'s onStop handler sets stoppedExternally (aicshud/WHEEL#1020) specifically to record that case. Currently masked at the whole-project level because onStopProject() unconditionally force-overwrites the project state to "stopped" afterward - but runProject() also writes _getState()'s (wrong) return value straight to the root workflow component's own cmp.wheel.json, so that component's own recorded state stays wrong ("finished"/"failed"/"unknown") even though the project overall correctly shows "stopped". Also update the existing #1020 test to call remove() (what stopProject() actually calls) instead of a bare pause() - the two are not equivalent: pause() alone is also used internally by _jumpHandler's "break" handling (a normal, successful in-workflow loop exit, not a stop), and the existing #_getState reproduction below documents that a break-terminated loop's own state must still be "finished", not "stopped". Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/test/app/core/dispatcher.js | 73 ++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/server/test/app/core/dispatcher.js b/server/test/app/core/dispatcher.js index df4f78ad..6f62a0ee 100644 --- a/server/test/app/core/dispatcher.js +++ b/server/test/app/core/dispatcher.js @@ -1655,14 +1655,16 @@ describe("UT for Dispatcher class", function () { //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 ()=>{ + it("should set stoppedExternally and resolve when stopped via remove() (what stopProject() actually calls) 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(); + await DP.remove(); const state = await startPromise; expect(DP.stoppedExternally).to.be.true; - expect(state).to.equal("finished"); + //aicshud/WHEEL#1028: start() must resolve with "stopped" (the documented state for an + //externally-stopped project), not "finished" - see #_getState below. + expect(state).to.equal("stopped"); }); it("should NOT set stoppedExternally when the dispatcher finishes naturally", async ()=>{ @@ -1671,6 +1673,71 @@ describe("UT for Dispatcher class", function () { await DP.start(); expect(DP.stoppedExternally).to.not.be.true; }); + + //reproduction for aicshud/WHEEL#1028: pause() alone (as opposed to remove(), which + //stopProject() calls) must NOT set stoppedExternally - _jumpHandler's "break" handling + //calls pause() directly as part of a normal, successful in-workflow loop exit, which is + //not an external stop and must not be reported as "stopped" (see the #Break tests, which + //this would otherwise break: a break-terminated loop's own state must still be "finished"). + it("should NOT set stoppedExternally when only pause() (not remove()) is called", 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.not.be.true; + expect(state).to.equal("finished"); + }); + }); + + //reproduction for aicshud/WHEEL#1028: _getState() never returned "stopped", the documented + //state for an externally-stopped project (documentMD/user_guide/_reference/3_workflow_screen/ + //1_graphview.md), even though start()'s onStop handler sets stoppedExternally (aicshud/ + //WHEEL#1020) specifically to record this case. Currently masked at the whole-project level + //because onStopProject() unconditionally force-overwrites the project state to "stopped" + //afterward - but runProject() also writes _getState()'s (wrong) return value straight to the + //root workflow component's own cmp.wheel.json, so that component's own recorded state was + //still wrong ("finished"/"failed"/"unknown") even though the project overall correctly showed + //"stopped". + describe("#_getState (aicshud/WHEEL#1028)", ()=>{ + it("should return 'stopped' when the dispatcher was stopped externally and no component failed or is unknown", async ()=>{ + const projectJson = await fs.readJson(path.resolve(projectRootDir, projectJsonFilename)); + const DP = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + DP.stoppedExternally = true; + expect(DP._getState()).to.equal("stopped"); + }); + + //issue #1000 already established that a task-failure-triggered project abort must report + //"failed", not "stopped" - the "taskStateChanged" listener in handlers/projectController.js + //aborts the rest of the project by calling stopProject(projectRootDir, task.state), which + //sets hasFailedComponent/hasUnknownComponent (via setStateFlag()) *before* remove() runs - + //so stoppedExternally being true here must NOT override that more specific outcome. + it("should still return 'failed'/'unknown' even when stopped externally, if a component failed or is unknown", async ()=>{ + const projectJson = await fs.readJson(path.resolve(projectRootDir, projectJsonFilename)); + const DP1 = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + DP1.stoppedExternally = true; + DP1.hasFailedComponent = true; + expect(DP1._getState()).to.equal("failed"); + + const DP2 = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + DP2.stoppedExternally = true; + DP2.hasUnknownComponent = true; + expect(DP2._getState()).to.equal("unknown"); + }); + + it("should still return 'unknown'/'failed'/'finished' as before when not stopped externally", async ()=>{ + const projectJson = await fs.readJson(path.resolve(projectRootDir, projectJsonFilename)); + const DP1 = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + expect(DP1._getState()).to.equal("finished"); + + const DP2 = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + DP2.hasFailedComponent = true; + expect(DP2._getState()).to.equal("failed"); + + const DP3 = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, ""); + DP3.hasUnknownComponent = true; + expect(DP3._getState()).to.equal("unknown"); + }); }); describe("#_checkMandatoryInputFilesExist", ()=>{ From ac2e59e514cc5331654eafb6e8f42c8743995868 Mon Sep 17 00:00:00 2001 From: Naoyuki Sogo Date: Wed, 16 Sep 2026 17:40:07 +0900 Subject: [PATCH 2/2] fix(dispatcher): _getState() reports "stopped" for a genuine external stop (aicshud/WHEEL#1028) - _getState() now falls through to "stopped" (aicshud/WHEEL#1020's stoppedExternally flag) once hasUnknownComponent/hasFailedComponent are ruled out, instead of never reporting the documented "stopped" state at all. Checked last, not first: stopProject() is also used to abort the rest of a project when one of its own tasks fails (the "taskStateChanged" listener in handlers/projectController.js), and that path already records the failure via setStateFlag() before remove() runs - hasFailedComponent/hasUnknownComponent must keep taking priority over stoppedExternally there, matching the existing "failed must not be clobbered by stopped" contract (issue #1000). - Move where stoppedExternally is set from the generic "stop" handler (onStop, inside start()) to remove() itself, the one and only caller of which is stopProject(). pause() is also called directly by _jumpHandler's "break" handling as a normal, successful in-workflow loop exit - not a stop - and both emit the same "stop" event that onStop listens for, so leaving the flag in the generic handler would have made a break-terminated loop's own state incorrectly report "stopped" too. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C3jKNM1qubomM8UTRdkEWu --- server/app/core/dispatcher.js | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/server/app/core/dispatcher.js b/server/app/core/dispatcher.js index f2a4f667..38a6e871 100644 --- a/server/app/core/dispatcher.js +++ b/server/app/core/dispatcher.js @@ -466,6 +466,16 @@ class Dispatcher extends EventEmitter { state = "unknown"; } else if (this.hasFailedComponent) { state = "failed"; + } else if (this.stoppedExternally) { + //a plain external stop (aicshud/WHEEL#1020/#1028), reported only once + //hasUnknownComponent/hasFailedComponent are ruled out. stopProject() is also called + //when a task's own failure aborts the rest of the project (the "taskStateChanged" + //listener in handlers/projectController.js) - that path already records the failure + //via setStateFlag() before remove() runs, so hasFailedComponent/hasUnknownComponent + //(checked above) correctly take priority over stoppedExternally in that case, + //matching the existing "failed must not be clobbered by stopped" contract (issue + //#1000, referenced in that listener). + state = "stopped"; } return state; } @@ -513,13 +523,17 @@ class Dispatcher extends EventEmitter { const onStop = ()=>{ logTrace(this.projectRootDir, this.cwfDir, "dispatcher stopped externally"); removeSettleListeners(); - //the dispatcher was stopped from outside the normal dispatch loop (e.g. the whole - //project being aborted because a task failed, or a manual "stop project"). settle - //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. + //"stop" fires both for a genuine external stop (remove(), the whole project being + //aborted because a task failed, or a manual "stop project") and for _jumpHandler's + //"break" handling (a normal, successful in-workflow loop exit calling pause() + //directly) - stoppedExternally (set by remove() itself, aicshud/WHEEL#1020/#1028) + //distinguishes the two, so _getState() only reports "stopped" for the former. + //settle start()'s promise with the current outcome instead of leaving it pending + //forever either way - 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 + //note for the genuine-external-stop case (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 @@ -527,7 +541,6 @@ class Dispatcher extends EventEmitter { //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); @@ -553,6 +566,12 @@ class Dispatcher extends EventEmitter { } async remove() { + //record that this was a genuine external stop (aicshud/WHEEL#1020/#1028) before pause() + //synchronously emits "stop" below - pause() is also called directly by _jumpHandler's + //"break" handling (a normal, successful in-workflow loop exit, not a stop), so the flag + //must be set here, the one and only caller of remove() (stopProject()), rather than + //unconditionally inside the "stop" handler itself. + this.stoppedExternally = true; await this.pause(); const p = []; for (const child of this.children) {