From 8393dc10903f37ac09e7240028e8ad357c8187b8 Mon Sep 17 00:00:00 2001 From: Frank Date: Wed, 21 Oct 2026 01:06:11 -0700 Subject: [PATCH] fix(remote-job): a job name is data, never a shell pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `status` built its job list with `for n in $names`, which word-splits AND globs. The second half is the bug: a job named `x*` was expanded against the caller's working directory, so the report described unrelated files there as jobs and never mentioned the one that ran — on a harness whose whole purpose is a report you can trust after the session that started the job is gone. Splitting stays (several names in one argument is the documented call shape); expansion goes. `tail` was honest-by-accident in the wrong direction: with no jobs it handed a literal `$JOBS/*.out` to tail, which reports a missing *file* — indistinguishable from a broken tool — and a missing name produced tail's errno rather than "no job called that". Both now say what is missing and exit 1. Red first: the name test reads the report from a directory containing an unrelated file, so the old glob is visible as a phantom job; both new tests fail against the previous code and pass against this one. Refs #60 --- omp-docs/tools/remote-job.md | 5 +++-- scripts/remote-job.sh | 26 ++++++++++++++++++++------ tests/remote-job.bats | 25 +++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/omp-docs/tools/remote-job.md b/omp-docs/tools/remote-job.md index 94b526d..2dec74f 100644 --- a/omp-docs/tools/remote-job.md +++ b/omp-docs/tools/remote-job.md @@ -38,10 +38,10 @@ read afterwards by a short-lived connection. That is what this is. ```bash remote-job.sh start NAME [--] CMD... # launch detached, return at once -remote-job.sh status [NAME] # one line per job; exit 0 only if all DONE +remote-job.sh status [NAME] # one line per job; names literal, never globs remote-job.sh wait NAME [SEC] [N] # block until DONE or timeout, then tail N lines remote-job.sh out NAME [N] # tail a job's output -remote-job.sh tail [NAME] # follow one job, or every job at once +remote-job.sh tail [NAME] # follow one job, or all; exit 1 if nothing to follow remote-job.sh watch # refreshing status, for a spare pane ``` @@ -77,6 +77,7 @@ can be made from a fresh session on a machine that rebooted underneath it. | Command name is wrong | `PENDING (no output yet)`, exit 1 — never a false DONE | | Job exits non-zero | `DONE rc=N`; the code is data, not the report's status | | Two jobs, same NAME | the second replaces the first's state files | +| `tail` with no job, or a name that is a glob | exit 1 saying there is nothing to follow; names are matched literally, never expanded | | Job dir missing | created on first use | Pinned by `tests/remote-job.bats` (black-box: output, exit codes, sentinel diff --git a/scripts/remote-job.sh b/scripts/remote-job.sh index 20082b3..8f64105 100755 --- a/scripts/remote-job.sh +++ b/scripts/remote-job.sh @@ -51,13 +51,21 @@ case "${1:-}" in ;; status) + # Job names are data, never patterns. `for n in $names` word-split AND globbed: a job named + # `x*` was expanded against the caller's working directory, so the report described unrelated + # files there as jobs and never mentioned the one that ran. Splitting stays (several names in + # one argument is the documented call shape); expansion does not. + names=() if [[ -n "${2:-}" ]]; then - names="$2" + read -r -a names <<< "$2" else - names=$(cd "$JOBS" && ls *.out 2>/dev/null | sed 's/\.out$//' | sort) + while IFS= read -r -d '' f; do + names+=("$(basename "$f" .out)") + done < <(find "$JOBS" -maxdepth 1 -name '*.out' -print0 2>/dev/null | sort -z) fi + if [[ ${#names[@]} -eq 0 ]]; then echo " (no jobs)"; exit 0; fi pending=0 - for n in $names; do + for n in "${names[@]}"; do if [[ -f "$JOBS/$n.done" ]]; then printf ' %-20s DONE rc=%s %s lines\n' "$n" \ "$(cat "$JOBS/$n.rc" 2>/dev/null)" "$(wc -l < "$JOBS/$n.out" 2>/dev/null | tr -d ' ')" @@ -69,7 +77,6 @@ case "${1:-}" in pending=1 fi done - if [[ -z "$names" ]]; then echo " (no jobs)"; exit 0; fi exit "$pending" ;; @@ -95,8 +102,15 @@ case "${1:-}" in tail) # Multiplexed follow: no NAME means every job at once, so one pane shows the whole harness. - if [[ -n "${2:-}" ]]; then exec tail -f "$JOBS/$2.out"; fi - exec tail -f "$JOBS"/*.out + # Both branches check for something to follow first: an unexpanded `*.out` handed to tail + # reports a missing *file*, which reads as a broken tool instead of an empty harness. + if [[ -n "${2:-}" ]]; then + [[ -f "$JOBS/$2.out" ]] || { echo "remote-job: no job '$2' (no $JOBS/$2.out)" >&2; exit 1; } + exec tail -f "$JOBS/$2.out" + fi + mapfile -d '' -t follow < <(find "$JOBS" -maxdepth 1 -name '*.out' -print0 2>/dev/null | sort -z) + (( ${#follow[@]} )) || { echo "remote-job: no jobs in $JOBS yet" >&2; exit 1; } + exec tail -f "${follow[@]}" ;; watch) diff --git a/tests/remote-job.bats b/tests/remote-job.bats index c980ffc..2598a12 100755 --- a/tests/remote-job.bats +++ b/tests/remote-job.bats @@ -122,6 +122,31 @@ until_done() { [[ "$output" == *38* ]] # the tail, not the head } +@test "status treats a job name as a name, not a pattern" { + "$SCRIPT" start 'x*' -- bash -c 'echo star' + until_done 'x*' + touch "$TEST_TMP/anchor" + # The report is read from a directory full of unrelated files on purpose: a name that is + # expanded as a glob resolves against the CALLER's cwd, so the tool would describe `anchor` + # as a job and never mention the one that really ran. + run bash -c 'cd "$1" && "$2" status "$3"' _ "$TEST_TMP" "$SCRIPT" 'x*' + [[ "$output" == " x*"*"DONE rc=0"* ]] + [[ "$output" != *anchor* ]] +} + +@test "tail on an empty harness and on a missing job both say so" { + # An unexpanded `*.out` reaching tail is reported as a missing FILE, which reads as a broken + # script; the honest answer is that there is nothing to follow yet. + run timeout 5 "$SCRIPT" tail + [ "$status" -ne 0 ] + [[ "$output" == *"no jobs"* ]] + [[ "$output" != *".out"* ]] + run timeout 5 "$SCRIPT" tail nosuchjob + [ "$status" -ne 0 ] + [[ "$output" == *"nosuchjob"* ]] + [[ "$output" != *"No such file"* ]] +} + @test "an unknown verb is a usage error" { run "$SCRIPT" frobnicate [ "$status" -eq 2 ]