Treat Docker OOMKilled as an observation, not a status verdict (#151)

.gitkeep

@@ -1,2 +1,3 @@
 # .gitkeep file auto-generated at 2026-06-26T14:21:03.315Z for PR creation at branch issue-144-d8d3fb7dfd56 for issue https://github.com/link-foundation/start/issues/144
-# Updated: 2026-06-26T16:22:21.672Z
+# Updated: 2026-06-26T16:22:21.672Z
+# Updated: 2026-08-04T03:47:36.189Z

README.md

@@ -169,6 +169,13 @@ include best-effort `processIds` for tracked wrapper processes and detached
 screen, tmux, and Docker isolation containers when those native tools can
 report them.
 
+For detached Docker executions, `oomKilled` is reported as an observation of the
+container cgroup flag, not as a verdict: while `docker inspect` still reports the
+container as running the status stays `executing`, and once it stops the reported
+`exitCode` is the container's real exit code. `137` is only used as a fallback
+when the container is gone and neither a stored exit code nor a log footer can be
+recovered.
+
 `--upload-log` accepts either an execution UUID or an isolation session name. It
 looks up the stored `logPath`, installs `gh-upload-log` with Bun or npm if the
 uploader is missing, and then streams the uploader output directly.

js/.changeset/issue-151-oom-observation-not-verdict.md

@@ -0,0 +1,5 @@
+---
+'start-command': patch
+---
+
+Treat Docker `OOMKilled` as an observation rather than a verdict in `--status` / `--list`: a detached session whose container is still running stays `executing` (with `oomKilled true` alongside), a stopped container reports its real `.State.ExitCode`, and `137` is used only when the container is gone and neither a log footer nor an exit code can be recovered.

js/src/lib/status-formatter.js

@@ -84,27 +84,36 @@ function readDockerState(record) {
  * (currently docker via `docker inspect .State.ExitCode`). Returns null when
  * the backend cannot provide a real code, so callers never surface the `-1`
  * sentinel for a session whose real exit code is simply not available yet.
- * @param {Object} record - Execution record
+ * A running container has no terminal exit code (docker reports `0` for it),
+ * so only a stopped container contributes one.
+ * @param {{running: boolean, exitCode: number|null}|null} dockerState - Inspected state
  * @returns {number|null}
  */
-function readBackendExitCode(record) {
-  const state = readDockerState(record);
-  return state && !state.running ? state.exitCode : null;
+function backendExitCode(dockerState) {
+  return dockerState && !dockerState.running ? dockerState.exitCode : null;
 }
 
-function resolveOomExitCode(footerExit, dockerState) {
-  if (footerExit !== null && footerExit !== undefined) {
-    return footerExit;
+/**
+ * Reconcile the OOM observation from the stored record and from `docker inspect`.
+ *
+ * `State.OOMKilled` is a container-cgroup flag: the kernel sets it when ANY
+ * process in the cgroup is OOM-killed and it is never cleared for the life of
+ * the container (moby/moby#47618). It is therefore an *observation*, never a
+ * verdict about the session (issue #151) — a container that lost one child
+ * process keeps running and can still exit `0`. Once observed, the flag stays
+ * `true` for the record.
+ * @param {Object} record - Execution record
+ * @param {{oomKilled: boolean|null}|null} dockerState - Inspected state
+ * @returns {boolean|undefined} Observation, or undefined when nothing is known
+ */
+function resolveOomObservation(record, dockerState) {
+  if (record.oomKilled === true || dockerState?.oomKilled === true) {
+    return true;
   }
-  if (
-    dockerState &&
-    dockerState.exitCode !== null &&
-    dockerState.exitCode !== undefined &&
-    (!dockerState.running || dockerState.exitCode !== 0)
-  ) {
-    return dockerState.exitCode;
+  if (record.oomKilled === false || dockerState?.oomKilled === false) {
+    return false;
   }
-  return 137;
+  return undefined;
 }
 
 /**
@@ -201,25 +210,18 @@ function enrichDetachedStatus(record) {
       : dockerState.running
     : isDetachedSessionAlive(record);
 
+  // `oomKilled` is exposed alongside the status, but never decides it (#151).
+  const oomKilled = resolveOomObservation(record, dockerState);
+
   // Create a shallow copy to avoid mutating the original
   const cloneRecord = () => {
     const enriched = Object.create(Object.getPrototypeOf(record));
     Object.assign(enriched, record);
-    return enriched;
-  };
-
-  if (record.oomKilled === true || dockerState?.oomKilled === true) {
-    const enriched = cloneRecord();
-    enriched.oomKilled = true;
-    enriched.status = 'executed';
-    if (enriched.exitCode === null || enriched.exitCode === undefined) {
-      enriched.exitCode = resolveOomExitCode(footerExit, dockerState);
-    }
-    if (!enriched.endTime) {
-      enriched.endTime = new Date().toISOString();
+    if (oomKilled !== undefined) {
+      enriched.oomKilled = oomKilled;
     }
     return enriched;
-  }
+  };
 
   if (alive === null) {
     // Liveness is unknown: the backend could not be probed (e.g. a detached
@@ -230,22 +232,35 @@ function enrichDetachedStatus(record) {
     // orchestrators misread as a finished/failed run (issue #136).
     const isDetached =
       record.options && record.options.isolationMode === 'detached';
-    if (isDetached && record.status === 'executing' && footerExit !== null) {
-      const enriched = cloneRecord();
-      enriched.status = 'executed';
-      enriched.exitCode = footerExit;
-      if (!enriched.endTime) {
-        enriched.endTime = new Date().toISOString();
+    if (isDetached && record.status === 'executing') {
+      if (footerExit !== null) {
+        const enriched = cloneRecord();
+        enriched.status = 'executed';
+        enriched.exitCode = footerExit;
+        if (!enriched.endTime) {
+          enriched.endTime = new Date().toISOString();
+        }
+        return enriched;
+      }
+      if (oomKilled === true) {
+        // The container is gone and wrote no footer, so the OOM observation is
+        // the only evidence left: report the session as finished with the
+        // conventional SIGKILL code (issue #148). While the container is still
+        // inspectable this branch is never taken — a live container keeps the
+        // session `executing` no matter what the cgroup flag says (issue #151).
+        const enriched = cloneRecord();
+        enriched.status = 'executed';
+        enriched.exitCode = 137;
+        if (!enriched.endTime) {
+          enriched.endTime = new Date().toISOString();
+        }
+        return enriched;
       }
-      return enriched;
     }
-    return record;
+    return oomKilled !== undefined ? cloneRecord() : record;
   }
 
   const enriched = cloneRecord();
-  if (dockerState?.oomKilled !== null && dockerState?.oomKilled !== undefined) {
-    enriched.oomKilled = dockerState.oomKilled;
-  }
 
   if (alive && enriched.status === 'executed') {
     // A live `screen -ls` (or `tmux`/`docker`) session does NOT mean the command
@@ -266,11 +281,15 @@ function enrichDetachedStatus(record) {
   } else if (!alive && enriched.status === 'executing') {
     // Session ended but record says executing - correct it. Resolve a real exit
     // code: prefer the log footer, then the backend's own record (e.g.
-    // `docker inspect .State.ExitCode`), and only fall back to the `-1` sentinel
-    // as a last resort when no real code can be obtained (issue #136).
+    // `docker inspect .State.ExitCode`), then `137` when the only evidence left
+    // is the OOM observation, and only fall back to the `-1` sentinel as a last
+    // resort when no real code can be obtained (issues #136, #151).
     enriched.status = 'executed';
     if (enriched.exitCode === null || enriched.exitCode === undefined) {
-      enriched.exitCode = footerExit ?? readBackendExitCode(enriched) ?? -1;
+      enriched.exitCode =
+        footerExit ??
+        backendExitCode(dockerState) ??
+        (oomKilled === true ? 137 : -1);
     }
     if (!enriched.endTime) {
       enriched.endTime = new Date().toISOString();

js/test/isolation.js

@@ -274,6 +274,13 @@ describe('Isolation Runner Error Handling', () => {
         console.log('  Skipping: docker not installed');
         return;
       }
+      // Skip if the daemon is not running - the error will be about the daemon
+      // instead of the missing image (Windows CI runners ship the docker CLI
+      // without a running daemon).
+      if (!require('../src/lib/docker-utils').isDockerAvailable()) {
+        console.log('  Skipping: docker daemon not running');
+        return;
+      }
 
       const result = await runInDocker('echo test', { detached: true });
       assert.strictEqual(result.success, false);

js/test/session-name-status.js

@@ -61,6 +61,38 @@ function createExecutable(filePath, content) {
   fs.chmodSync(filePath, 0o755);
 }
 
+// A fake `docker` whose every `inspect` fails, i.e. the container is gone
+// (removed) or not visible yet — the "unknown liveness" case of issue #136.
+function withFakeDockerMissingContainer(fn) {
+  const fakeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'fake-docker-gone-'));
+  const dockerPath = path.join(
+    fakeBin,
+    process.platform === 'win32' ? 'docker.cmd' : 'docker'
+  );
+  createExecutable(
+    dockerPath,
+    process.platform === 'win32'
+      ? ['@echo off', 'exit /b 1', ''].join('\r\n')
+      : ['#!/bin/sh', 'exit 1', ''].join('\n')
+  );
+
+  const originalPath = process.env.PATH;
+  const originalDockerBin = process.env.START_DOCKER_BIN;
+  process.env.PATH = `${fakeBin}${path.delimiter}${originalPath || ''}`;
+  process.env.START_DOCKER_BIN = dockerPath;
+  try {
+    return fn();
+  } finally {
+    process.env.PATH = originalPath;
+    if (originalDockerBin === undefined) {
+      delete process.env.START_DOCKER_BIN;
+    } else {
+      process.env.START_DOCKER_BIN = originalDockerBin;
+    }
+    fs.rmSync(fakeBin, { recursive: true, force: true });
+  }
+}
+
 function withFakeDockerInspect(stateLine, fn) {
   const fakeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'fake-docker-'));
   const dockerPath = path.join(
@@ -677,7 +709,7 @@ describe('Issue #144: detached docker OOMKilled status signal', () => {
   });
 });
 
-describe('Issue #148: detached docker OOMKilled terminal status', () => {
+describe('Issue #148: detached docker OOMKilled terminal status when the container is gone', () => {
   let store;
 
   beforeEach(() => {
@@ -692,7 +724,7 @@ describe('Issue #148: detached docker OOMKilled terminal status', () => {
     cleanupTestDir();
   });
 
-  function saveDockerRecord() {
+  function saveDockerRecord(extra = {}) {
     const record = new ExecutionRecord({
       command: 'sh -c "allocate memory"',
       logPath: '/tmp/issue-148.log',
@@ -701,15 +733,16 @@ describe('Issue #148: detached docker OOMKilled terminal status', () => {
         isolated: 'docker',
         isolationMode: 'detached',
       },
+      ...extra,
     });
     store.save(record);
     return record;
   }
 
-  it('treats oomKilled as terminal even while Docker still reports running', () => {
-    const record = saveDockerRecord();
+  it('makes an OOM-killed session terminal once its container is gone', () => {
+    const record = saveDockerRecord({ oomKilled: true });
 
-    withFakeDockerInspect('true 137 true', () => {
+    withFakeDockerMissingContainer(() => {
       const result = queryStatus(store, record.uuid, 'json');
       expect(result.success).toBe(true);
       const parsed = JSON.parse(result.output);
@@ -721,19 +754,124 @@ describe('Issue #148: detached docker OOMKilled terminal status', () => {
     });
   });
 
-  it('uses 137 when oomKilled is terminal but Docker has no terminal exit code yet', () => {
+  it('uses the container exit code, not 137, for a stopped OOM-flagged container', () => {
     const record = saveDockerRecord();
 
+    withFakeDockerInspect('false 3 true', () => {
+      const result = queryStatus(store, record.uuid, 'json');
+      expect(result.success).toBe(true);
+      const parsed = JSON.parse(result.output);
+      expect(parsed.status).toBe('executed');
+      expect(parsed.exitCode).toBe(3);
+      expect(parsed.oomKilled).toBe(true);
+      expect(parsed.endTime).toBeTruthy();
+    });
+  });
+});
+
+describe('Issue #151: OOMKilled is an observation, not a verdict', () => {
+  let store;
+  let logPath;
+
+  beforeEach(() => {
+    cleanupTestDir();
+    store = new ExecutionStore({
+      appFolder: TEST_APP_FOLDER,
+      useLinks: false,
+    });
+    logPath = path.join(os.tmpdir(), `issue-151-${process.pid}.log`);
+    fs.rmSync(logPath, { force: true });
+  });
+
+  afterEach(() => {
+    fs.rmSync(logPath, { force: true });
+    cleanupTestDir();
+  });
+
+  function saveDockerRecord(extra = {}) {
+    const record = new ExecutionRecord({
+      command: 'node -e "setTimeout(() => process.exit(0), 600000)"',
+      logPath,
+      options: {
+        sessionName: 'issue151-oom',
+        isolated: 'docker',
+        isolationMode: 'detached',
+      },
+      ...extra,
+    });
+    store.save(record);
+    return record;
+  }
+
+  it('keeps a running container executing while exposing oomKilled', () => {
+    const record = saveDockerRecord();
+
+    // `docker inspect` reports the cgroup OOM flag while the container itself
+    // is still running: only the container state may decide the status.
     withFakeDockerInspect('true 0 true', () => {
+      const result = queryStatus(store, record.uuid, 'json');
+      expect(result.success).toBe(true);
+      const parsed = JSON.parse(result.output);
+      expect(parsed.status).toBe('executing');
+      expect(parsed.exitCode).toBeNull();
+      expect(parsed.oomKilled).toBe(true);
+      expect(parsed.endTime).toBeNull();
+      expect(parsed.currentTime).toBeTruthy();
+    });
+  });
+
+  it('never synthesizes 137 for a running container that reports exit code 137', () => {
+    const record = saveDockerRecord();
+
+    withFakeDockerInspect('true 137 true', () => {
+      const enriched = enrichDetachedStatus(store.get(record.uuid));
+      expect(enriched.status).toBe('executing');
+      expect(enriched.exitCode).toBeNull();
+      expect(enriched.oomKilled).toBe(true);
+    });
+  });
+
+  it('reports the container exit code 0 once an OOM-flagged container finishes', () => {
+    const record = saveDockerRecord();
+
+    withFakeDockerInspect('false 0 true', () => {
       const result = queryStatus(store, record.uuid, 'json');
       expect(result.success).toBe(true);
       const parsed = JSON.parse(result.output);
       expect(parsed.status).toBe('executed');
-      expect(parsed.exitCode).toBe(137);
+      expect(parsed.exitCode).toBe(0);
       expect(parsed.oomKilled).toBe(true);
       expect(parsed.endTime).toBeTruthy();
     });
   });
+
+  it('prefers the log footer exit code over the OOM fallback when the container is gone', () => {
+    fs.writeFileSync(logPath, 'Finished: now\nExit Code: 0\n', 'utf8');
+    const record = saveDockerRecord({ oomKilled: true });
+
+    withFakeDockerMissingContainer(() => {
+      const result = queryStatus(store, record.uuid, 'json');
+      expect(result.success).toBe(true);
+      const parsed = JSON.parse(result.output);
+      expect(parsed.status).toBe('executed');
+      expect(parsed.exitCode).toBe(0);
+      expect(parsed.oomKilled).toBe(true);
+    });
+  });
+
+  it('keeps the list output executing for an OOM-flagged running container', () => {
+    saveDockerRecord();
+
+    withFakeDockerInspect('true 0 true', () => {
+      const result = listExecutions(store, 'json');
+      expect(result.success).toBe(true);
+      const parsed = JSON.parse(result.output);
+      expect(parsed.count).toBe(1);
+      expect(parsed.executions[0].status).toBe('executing');
+      expect(parsed.executions[0].exitCode).toBeNull();
+      expect(parsed.executions[0].oomKilled).toBe(true);
+    });
+  });
 });
 
 describe('Issue #105: attachCurrentTime for executing status', () => {