SEP-1943: Report an unlaunchable executor; drop a redundant sudo (#1445)

An execution whose interpreter the target Nomad node cannot launch used
to land in `FAILED`, indistinguishable from a script that ran and exited
non-zero on its own terms — the launcher's own error arrived as ordinary
script output. This does two things.

**1. Reports it as its own outcome.** A `check-launchable` prestart step
(`app/tasks/db/seed.py`) is added to the three seeded specs that
interpolate a launch command from meta — `run-command`, `exec-artifact`,
`exec-python-artifact`. `run-python` is excluded: it is payload-driven
and declares no launch-command meta. The step resolves the launch
command chain on the node and aborts with sentinel exit `78`, which the
Nomad executor maps to a new terminal
`TaskHistoryStatusEnum.UNLAUNCHABLE` — the same shape as the existing
`check-staleness` / exit `75` / `STALE` mechanism. It logs
`SEP_UNLAUNCHABLE: command=<cmd> node=<node>`, naming which command
could not be launched and where.

**2. Stops causing the failure on a root node.** Where a node's
`raw_exec` tasks already run as uid 0 and no `sudo` binary exists, the
`sudo ` prefix `build_execution_meta` prepends
(`app/sep/apps/framework/script_helpers.py`, unchanged here) is the
*sole* cause of the failure — the script would have run without it. The
check strips a redundant bare `sudo` prefix in exactly that case and
writes the effective interpreter to
`${NOMAD_ALLOC_DIR}/sep_interpreter`; both artifact specs' `run-script`
steps now launch from that file. `exec-artifact` therefore changes from
a direct `xargs` exec to `sh -c`, which passes identical argv to the
payload.

The strip is deliberately narrow. Only a **bare** `sudo` first token
followed by a plain word is dropped, and only when the token the
invocation actually names does not resolve on the node. `sudo -u
postgres bash` is never stripped: `sudo -u` *lowers* privilege, so
dropping it would run the payload as root instead of as `postgres`. An
operator-supplied `/opt/x/sudo` that exists is kept for the same reason
— a binary named by path may be a wrapper that changes the target user.

**The check recognises a small grammar and declines everything else.**
It tokenizes with `sh` word-splitting; the launcher tokenizes with `env
-S`. These are different grammars, so a form only one of them
understands is passed through unchanged rather than resolved — quotes,
`$`, backticks, backslashes, an `env` first token, a first token
beginning with `-` (`env -S` parses leading options itself, so `-u FOO
bash` unsets `FOO` and runs `bash`), a leading `NAME=VALUE` (which `env
-S` applies *before* locating the command, so it can decide where the
command resolves), any non-absolute path (this step pins no `work_dir`
and `run-script` pins one, so the check cannot tell where the launcher
would resolve it), and any `sudo` option outside the two tables the
walker knows — it enumerates the options that take a value and the
options that take none, and declines every other `-*` rather than
assuming it takes none and resolving the word after it. `set -f` stops
it globbing where `env -S` does not. Those are deliberate
false-negatives: behaviour for them is exactly what it is today, and the
alternative error — aborting a working execution — is the one that
hurts.

**Resolution tests the exec bit, not just the name.** For a token
holding a slash the check uses `[ -x ] && [ ! -d ]` rather than `command
-v`. Under dash and busybox ash — the two most common node shells —
`command -v` reports a bare-existing path as found whatever its mode, so
a non-executable interpreter, or a directory, would have passed the
check and landed in `FAILED`: the outcome this step exists to separate
out. Only bash checks the mode, which is also why the suite, running
under a bash `/bin/sh`, could not have surfaced it. Bare names keep
`command -v`, which does search `PATH` for an executable.

`exec-python-artifact`'s check resolves `python3` rather than the
interpreter meta, because that spec's `run-script` always execs the venv
python and reads the meta only for a `"sudo "` prefix test. Resolving
the meta's own token there would abort a runnable execution for any
operator who maps `.py` to something else through `INTERPRETERS`.

**Supporting changes.** An Alembic revision widens `taskhistory.status`
from `VARCHAR(7)` to `VARCHAR(12)`: the column stores enum member
*names*, and there is no CHECK constraint on it, so the length is the
only DB-side gate. `_TERMINAL_STATUS_EVENT_MAP` gains an entry (it is
indexed, not `.get()`-ed, in `stop_task`). `alert_for_status` gains an
arm with a `:unlaunchable` dedup-key suffix plus its paired resolve on
the `SUCCESS` arm, so the incident can clear. The ATW support bundle
selects the check step's log for this status. Frontend status badges and
the `FINISHED_TASK_STATUSES` set gain the new member.

`is_finished()` gaining a member changes chain dispatch through
`is_terminal()`: a parent carrying `_chain_on_failure` now dispatches
its successor on `UNLAUNCHABLE`, consistent with `FAILED` / `STOPPED` /
`LOST` / `STALE`. That is intended. The two chain tests now **derive**
the non-success terminal set from the enum rather than spelling it out,
which is why no test turned red when the previous terminal status
landed.

## Verified against PostgreSQL

The suite runs on SQLite, which ignores `VARCHAR` length, so a green
suite proves nothing about the migration. Checked directly on PostgreSQL
16: the column is `character varying(7)` with no CHECK constraints
before and `character varying(12)` after; a row with `status =
'UNLAUNCHABLE'` writes successfully; and `downgrade()` remaps such rows
to `FAILED` before re-narrowing, which is what keeps it runnable once
the feature has been used.

Author: yyyyyyyan

app/sep/apps/atw/send.py

@@ -82,6 +82,10 @@
 #: ``NOMAD_EXEC_ARTIFACT`` and ``NOMAD_EXEC_PYTHON_ARTIFACT`` in
 #: ``app/tasks/db/seed.py``.
 _MAIN_LOG_STEP = "run-script"
+#: The step carrying an unlaunchable execution's whole diagnostic. That
+#: execution never starts :data:`_MAIN_LOG_STEP` and writes no output files, so
+#: the general "prestart is setup machinery" rule would leave its bundle empty.
+_LAUNCH_CHECK_LOG_STEP = "check-launchable"
 _WORKER_LOST_ERROR = (
     "The worker running this send did not report back in time; it was most "
     "likely lost. Re-send to try again."
@@ -424,13 +428,30 @@ def close(self) -> dict[str, Any]:
         }
 
 
+def _log_step_for(status: TaskHistoryStatusEnum | None) -> str:
+    """Return the step whose logs carry this execution's diagnostic.
+
+    :param status: The execution's terminal status.
+    :return: :data:`_LAUNCH_CHECK_LOG_STEP` for an execution the node could not
+        launch, :data:`_MAIN_LOG_STEP` otherwise.
+    """
+    if status == TaskHistoryStatusEnum.UNLAUNCHABLE:
+        return _LAUNCH_CHECK_LOG_STEP
+    return _MAIN_LOG_STEP
+
+
 async def _add_execution_logs(
-    archive: zipfile.ZipFile, tasks_api: RemoteAPI, execution: dict[str, Any]
+    archive: zipfile.ZipFile,
+    tasks_api: RemoteAPI,
+    execution: dict[str, Any],
+    step: str,
 ) -> tuple[list[dict[str, Any]], int]:
-    """Stream one execution's captured main-step logs into the archive.
+    """Stream one execution's captured logs for ``step`` into the archive.
 
-    Only :data:`_MAIN_LOG_STEP` is fetched: the prestart and poststop steps
-    surrounding it log setup machinery, which is noise on a support case.
+    A single step is fetched: for a run that happened, the prestart and poststop
+    steps surrounding :data:`_MAIN_LOG_STEP` log setup machinery, which is noise
+    on a support case. The one exception is a run that never started, whose only
+    diagnostic lives in a prestart step — see :func:`_log_step_for`.
 
     Members are keyed by a record's ``(step, stream)`` group and replaced when it
     changes -- the two upstream read paths both deliver contiguous runs per group
@@ -441,6 +462,7 @@ async def _add_execution_logs(
     :param archive: The open archive to write into.
     :param tasks_api: The authenticated Tasks API client.
     :param execution: The selected execution descriptor.
+    :param step: The Nomad step whose logs to stream.
     :return: One manifest entry per log group written, and the total number of
         uncompressed bytes they carry.
     :raises AtwSendError: When the execution's logs cannot be streamed.
@@ -456,7 +478,7 @@ async def _add_execution_logs(
     member: _LogMember | None = None
     try:
         async for line in tasks_api.stream(
-            f"/history/{task_history_id}/logs/", params={"step": _MAIN_LOG_STEP}
+            f"/history/{task_history_id}/logs/", params={"step": step}
         ):
             record = _decode_log_line(line, task_history_id)
             if record is None:
@@ -540,7 +562,9 @@ async def _stage_bundle(
             file_count += len(files)
             logs: list[dict[str, Any]] = []
             if status is not None and status.is_finished():
-                logs, written = await _add_execution_logs(archive, tasks_api, execution)
+                logs, written = await _add_execution_logs(
+                    archive, tasks_api, execution, _log_step_for(status)
+                )
                 log_bytes += written
             manifest_executions.append({**execution, "files": files, "logs": logs})
         if not file_count and not log_bytes: