fix: stopProject 中のジョブキャンセル誤判定を修正 (aicshud/WHEEL#1016 を分割: #1017, #1018, #1019)

AGENTS.md

@@ -26,3 +26,20 @@ test/cypress/component   component test with cypress
 - always write code in async/await style
 - always use try/catch to handle errors in async functions
 - use debug module for logging (temporarily use console.log for debugging is allowed, but remember to remove them before commit)
+
+## bug fix workflow (standard)
+Test-first is the principle for every bug fix. Follow these steps in order:
+
+1. File the symptom as a GitLab issue.
+2. Investigate the root cause and post the findings as a comment on that issue.
+3. Write a reproduction test, confirm it fails (red), then commit it.
+4. Implement the fix, confirm the full test suite is green, then commit it.
+5. Push to the forked repository and confirm CI is green there.
+6. Open a pull request to the upstream (RIKEN-RCCS) repository.
+
+Run lint before every commit made in this workflow (not only once at the end).
+
+When one pull request bundles fixes for multiple issues, do steps 1-4 separately and
+sequentially for each issue (one issue's investigation/red-test/fix/commit cycle at a
+time, not in parallel, to limit context/token usage), then do steps 5-6 once for the
+combined branch.

server/app/core/dispatcher.js

@@ -1502,7 +1502,12 @@ class Dispatcher extends EventEmitter {
     const componentDir = this._getComponentDir(component.ID);
     await writeComponentJson(this.projectRootDir, componentDir, component, true);
     const ee = eventEmitters.get(this.projectRootDir);
-    ee.emit("componentStateChanged", component);
+    //ee is undefined if this component settled after the project was already torn down
+    //(e.g. a stale job-status poll or remote command that finished late) - there is no one
+    //left to notify, so just skip it (aicshud/WHEEL#1019).
+    if (ee) {
+      ee.emit("componentStateChanged", component);
+    }
   }
 
   /**

server/app/core/execUtils.js

@@ -32,8 +32,13 @@ async function setTaskState(task, state) {
   _internal.getLogger(task.projectRootDir).trace(`TaskStateList: ${task.ID}'s state is changed to ${state}`);
   await _internal.writeComponentJson(task.projectRootDir, task.workingDir, task, true);
   const ee = _internal.eventEmitters.get(task.projectRootDir);
-  ee.emit("taskStateChanged", task);
-  ee.emit("componentStateChanged", task);
+  //ee is undefined if this task settled after the project was already torn down (e.g. a
+  //stale job-status poll or remote command that finished late) - there is no one left to
+  //notify, so just skip it (aicshud/WHEEL#1019).
+  if (ee) {
+    ee.emit("taskStateChanged", task);
+    ee.emit("componentStateChanged", task);
+  }
 }
 
 /**

server/app/core/jobManager.js

@@ -78,7 +78,10 @@ export function isJobFailed(JS, code) {
   } else {
     return false;
   }
-  return statusList.includes(code);
+  //acceptableJobStatus is a list of codes that mean "OK" - failed means NOT in that list.
+  //acceptableJobStatus in jobScheduler.json is written as JSON numbers while code is always
+  //a string (regexp capture group), so compare as strings on both sides.
+  return !statusList.map(String).includes(String(code));
 }
 _internal.isJobFailed = isJobFailed;
 
@@ -125,12 +128,16 @@ export async function getStatusCode(JS, task, statCmdRt, outputText) {
     strRt = rt;
   }
   if (strRt === null) {
-    _internal.getLogger(task.projectRootDir).warn("get return code failed, code is overwrited by -2");
-    return -2;
-  }
-  if (strRt === "6") {
-    _internal.getLogger(task.projectRootDir).warn("get return code 6, this job was canceled by stepjob dependency");
-    return 0;
+    //script's own return code is not obtainable/trustworthy (job was canceled, held,
+    //rejected, etc. before/without producing a real exit code). Design policy: fall back
+    //to the job status code via isJobFailed() - NOT the raw status code value itself,
+    //because acceptableJobStatus can list more than one "OK" code (Fugaku: [0, 6], where
+    //6 means "canceled by a stepjob dependency expression" - itself not a failure), so the
+    //raw code isn't safe to compare against the universal "rt === 0 means success" rule
+    //downstream (executerManager.js's `state = task.rt === 0 ? "finished" : "failed"`).
+    _internal.getLogger(task.projectRootDir).warn(`return code not available, falling back to job status code (${task.jobStatus})`);
+    task.rt = _internal.isJobFailed(JS, task.jobStatus) ? 1 : 0;
+    return task.rt;
   }
   if (task.type === "bulkjobTask") {
     await _internal.createBulkStatusFile(task, rtCodeList, jobStatusList);
@@ -205,6 +212,15 @@ export function registerJob(hostinfo, task) {
     }
     const request = hostinfo.useWebAPI ? _internal.createRequestForWebAPI(hostinfo, task, JS) : _internal.createRequest(hostinfo, task, JS);
     const id = _internal.addRequest(request);
+    task.jobManagerRequestId = id;
+    task.jobManagerCancel = ()=>{
+      _internal.delRequest(id);
+      //deliberate cancel (e.g. stopProject already confirmed the job itself is canceled via
+      //pjdel): resolve, not reject, with null so the SBS wrapper's
+      //"task.state === 'not-started'" guard (executerManager.js) discards it, the same way a
+      //killed local task's resolved (not rejected) exit is discarded.
+      resolve(null);
+    };
     const result = _internal.getRequest(id);
     const requestName = `${request.argument} on ${request.hostInfo.host}`;
     let statusCheckErrorCount = 0;
@@ -220,6 +236,8 @@ export function registerJob(hostinfo, task) {
         err.numStatusCheckError = statusCheckErrorCount;
         err.maxStatusCheckError = JS.maxStatusCheckError;
         _internal.delRequest(id);
+        delete task.jobManagerCancel;
+        delete task.jobManagerRequestId;
         reject(err);
       }
     });
@@ -251,6 +269,8 @@ export function registerJob(hostinfo, task) {
         });
       }
       const rt = await _internal.getStatusCode(JS, task, hook.rt, hook.output);
+      delete task.jobManagerCancel;
+      delete task.jobManagerRequestId;
       if (_internal.isJobFailed(JS, task.jobStatus)) {
         return reject(task.jobStatus);
       }
@@ -263,9 +283,25 @@ export function registerJob(hostinfo, task) {
       if (typeof hookErr !== "undefined") {
         err.hookErr = hookErr;
       }
+      delete task.jobManagerCancel;
+      delete task.jobManagerRequestId;
       reject(err);
     });
   });
 }
 
+/**
+ * stop watching a job's status that registerJob() started polling for
+ * (aicshud/WHEEL#1018 - e.g. after stopProject has confirmed the job itself is canceled)
+ * @param {object} task - task component instance, as passed to registerJob()
+ */
+export function cancelJobStatusCheck(task) {
+  if (typeof task.jobManagerCancel === "function") {
+    task.jobManagerCancel();
+    delete task.jobManagerCancel;
+    delete task.jobManagerRequestId;
+  }
+}
+_internal.cancelJobStatusCheck = cancelJobStatusCheck;
+
 export { _internal };

server/app/core/taskUtil.js

@@ -5,6 +5,7 @@
  */
 import { getSsh, getSshHostinfo } from "./sshManager.js";
 import { cancel } from "./executerManager.js";
+import { cancelJobStatusCheck } from "./jobManager.js";
 import { jobScheduler } from "../db/db.js";
 import { getLogger } from "../logSettings.js";
 
@@ -13,6 +14,7 @@ const _internal = {
   getSshHostinfo,
   cancel,
   getLogger,
+  cancelJobStatusCheck,
   killTask: null,
   killLocalProcess: null,
   cancelRemoteJob: null,
@@ -38,6 +40,10 @@ export async function cancelRemoteJob(task) {
     output.push(data);
   });
   _internal.getLogger(task.projectRootDir).debug("cacnel done", output.join());
+  //only stop watching once the cancel command itself succeeded (didn't throw) - if it failed
+  //(e.g. connection error), the job might still be running on the scheduler; keep polling so
+  //WHEEL still finds out its real outcome instead of silently losing track of it.
+  _internal.cancelJobStatusCheck(task);
 }
 _internal.cancelRemoteJob = cancelRemoteJob;
 

server/app/db/jobScheduler.json

@@ -46,7 +46,7 @@
     "bulkstat": "pjstat -E -v --choose jid,st",
     "bulkstatAfter": "pjstat -E -v -H day=3 --choose jid,st,ec,pc",
     "reRunning": "{{ JOBID }} *(?:ACC|HLD|QUE|RNA|RNE|RNO|RNP|RSM|RUN|SPD|SPP)",
-    "reReturnCode": "{{ JOBID }} *(?:EXT|CCL) *(\\d+) *\\d+",
+    "reReturnCode": "{{ JOBID }} *EXT *(\\d+) *\\d+",
     "reJobStatusCode": "{{ JOBID }} *(?:CCL|ERR|EXT|RJT) *\\d+ *(\\d+)",
     "reSubReturnCode": "(?:\\d+\\[\\d+\\]) *(?:CCL|ERR|EXT|RJT) *(\\d+) *\\d+",
     "reSubJobStatusCode": "(?:\\d+\\[\\d+\\]) *(?:CCL|ERR|EXT|RJT) *\\d+ *(\\d+)",

server/test/app/core/dispatcher.js

@@ -1580,6 +1580,21 @@ describe("UT for Dispatcher class", function () {
     });
   });
 
+  //reproduction for aicshud/WHEEL#1019: a stale component-state update that arrives after
+  //the project has already been torn down (eventEmitters.delete(projectRootDir) already ran,
+  //e.g. via a late-resolving job-status poll or remote command) must not crash - there is
+  //simply no one left to notify.
+  describe("#_setComponentState (aicshud/WHEEL#1019)", ()=>{
+    it("should not throw when eventEmitters has no entry for the project (project already torn down)", async ()=>{
+      const projectJson = await fs.readJson(path.resolve(projectRootDir, projectJsonFilename));
+      const DP = new Dispatcher(projectRootDir, rootWF.ID, projectRootDir, "dummy start time", projectJson.componentPath, {}, "");
+      eventEmitters.delete(projectRootDir); //simulate: project already torn down
+
+      await DP._setComponentState(rootWF, "running");
+      expect(rootWF.state).to.equal("running");
+    });
+  });
+
   describe("#_checkMandatoryInputFilesExist", ()=>{
     let task;
     beforeEach(async ()=>{

server/test/app/core/execUtils.js

@@ -45,6 +45,21 @@ describe("#setTaskState", ()=>{
     expect(ee.emit.firstCall.args).to.deep.equal(["taskStateChanged", task]);
     expect(ee.emit.secondCall.args).to.deep.equal(["componentStateChanged", task]);
   });
+
+  //reproduction for aicshud/WHEEL#1019: a task completion that arrives after the project has
+  //already been torn down (eventEmitters.delete(projectRootDir) already ran) must not crash -
+  //there is simply no one left to notify.
+  it("should not throw if eventEmitters has no entry for the project (project already torn down)", async ()=>{
+    const task = {
+      projectRootDir: "alreadyTornDownProjectRootDir",
+      workingDir: "/dummy/working/dir",
+      ID: "task123",
+      state: "oldState"
+    };
+
+    await setTaskState(task, "newState");
+    expect(task.state).to.equal("newState");
+  });
 });
 
 describe("#needDownload", ()=>{