fix(dispatcher): _getState() reports "stopped" for a genuine external stop (aicshud/WHEEL#1028)

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,21 +523,24 @@ 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
         //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);
@@ -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) {

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", ()=>{