fix(issue-17): a decision the wrapper cannot make ends it with 1 (N2)

`exit "$(_decide exit-code ...)"` became `exit ""` when wrapper.py could
not answer (its module or $PY broken): rc 255 on bash 3.2. The other printed
decisions failed open in their own ways: a failed wait-rc returned "" (255),
a failed finalize-begin left finalize-end with an empty --final (argparse
rc 2), and a failed sub-status or finalize-end ended under set -e with the
interpreter's own code (1 for a traceback, 127 for one that cannot run).

Printed decisions now go through _decided <var> <subcommand>, and the
exits through _exit_for <status>: a failed call, an empty answer or a
non-numeric exit code / wait rc prints one ERROR line and exits 1. finalize-
end may legitimately answer nothing (its wrapper gone), so only its rc is
checked. The normal path is unchanged: same answers, exit codes and output.

Left as they are: subscriber-up and stall-watch are answered by rc, so a
failure reads as "no" - the subscriber is then treated as dead (the error
path, whose sub-status/exit-code are now fail-closed) and the notify-only
stall watcher is not started.

Tests (F): the real submit (human job, unreachable broker, 3 s budget) with
a $PY that fails one decision - exit-code, sub-status, finalize-begin,
finalize-end; traceback, rc 127 or no answer - ends with 1 and the ERROR
line; the same run without a failure still ends with 3. cmd_wait with a
failing wait-rc ends with 1. A static guard keeps printed decisions off
raw $(...). At 56d0bfd: exit-code 255/255, finalize-begin 2/2, wait-rc
255/255, sub-status and finalize-end 127 for "gone"; the traceback and
no-answer cases there already ended with 1 (by set -e or by the empty
status mapping to 1) and fail only on the missing ERROR line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HCGpJXhSZasJLU38ksDghe
This commit is contained in:
GodopuandClaude Opus 5.5 committed 2026-10-08 08:12:16 +09:00
1 parent f19f94099f
commit a670ef53ee
2 files changed
+118 -14

No files matched your search

@@ -100,6 +100,32 @@ _decide() {
PYTHONPATH="$SCRIPT_DIR/..${PYTHONPATH:+:$PYTHONPATH}" "$PY" -m lib_py.delegation.wrapper "$@" PYTHONPATH="$SCRIPT_DIR/..${PYTHONPATH:+:$PYTHONPATH}" "$PY" -m lib_py.delegation.wrapper "$@"
} }
# A decision wrapper.py could not make (its module or $PY broken) ends the
# wrapper with 1 (fail-closed): never with its own code, 0, or the empty answer
# (`exit ""` is 255 on bash 3.2).
_decision_failed() {
echo "ERROR: the wrapper could not decide '$1' (lib_py.delegation.wrapper: $2); exiting 1 (fail-closed)" >&2
exit 1
}
# _decided <var> <subcommand> <args...>: a printed decision into $var.
_decided() {
local __var="$1" __out __rc=0
shift
__out="$(_decide "$@")" || __rc=$?
[ "$__rc" -eq 0 ] || _decision_failed "$1" "rc $__rc"
[ -n "$__out" ] || _decision_failed "$1" "no answer"
printf -v "$__var" '%s' "$__out"
}
# _exit_for <status>: exit with the wrapper exit code for a job status.
_exit_for() {
local code
_decided code exit-code "$1"
[[ "$code" =~ ^[0-9]+$ ]] || _decision_failed exit-code "answer '$code'"
exit "$code"
}
# _pub_cmd <extra...>: the publish command as one shell-quoted string. # _pub_cmd <extra...>: the publish command as one shell-quoted string.
_pub_cmd() { _pub_cmd() {
local s local s
@@ -361,12 +387,12 @@ EOF
else else
echo "ERROR: subscriber died early (pid=$sub_pid, exit=$sub_exit, check $logf)" >&2 echo "ERROR: subscriber died early (pid=$sub_pid, exit=$sub_exit, check $logf)" >&2
local mapped_st local mapped_st
mapped_st="$(_decide sub-status "$sub_exit" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR")" _decided mapped_st sub-status "$sub_exit" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR"
finalize_unfinished "$JOB_ID" "$mapped_st" "$sub_exit" "subscriber" finalize_unfinished "$JOB_ID" "$mapped_st" "$sub_exit" "subscriber"
mapped_st="${FINAL_JOB_STATUS:-$mapped_st}" mapped_st="${FINAL_JOB_STATUS:-$mapped_st}"
local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}" local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}"
echo "$logs_root/$JOB_ID" echo "$logs_root/$JOB_ID"
exit "$(_decide exit-code "$mapped_st")" _exit_for "$mapped_st"
fi fi
fi fi
sleep 0.2 sleep 0.2
@@ -402,7 +428,7 @@ EOF
echo "subscriber output:"; cat "$logf" || true echo "subscriber output:"; cat "$logf" || true
local job_status local job_status
job_status="$(_decide sub-status "$sub_rc" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR")" _decided job_status sub-status "$sub_rc" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR"
finalize_unfinished "$JOB_ID" "$job_status" "$sub_rc" "direct" finalize_unfinished "$JOB_ID" "$job_status" "$sub_rc" "direct"
job_status="${FINAL_JOB_STATUS:-$job_status}" job_status="${FINAL_JOB_STATUS:-$job_status}"
@@ -411,7 +437,7 @@ EOF
# "Audit Logs"). Callers can scrape `tail -n1` to find it. # "Audit Logs"). Callers can scrape `tail -n1` to find it.
local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}" local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}"
echo "$logs_root/$JOB_ID" echo "$logs_root/$JOB_ID"
exit "$(_decide exit-code "$job_status")" _exit_for "$job_status"
else else
# Implement loop/discuss orchestrator # Implement loop/discuss orchestrator
local iteration=1 local iteration=1
@@ -533,12 +559,12 @@ EOF
else else
echo "ERROR: subscriber died early (pid=$sub_pid, exit=$sub_exit, check $logf)" >&2 echo "ERROR: subscriber died early (pid=$sub_pid, exit=$sub_exit, check $logf)" >&2
local mapped_st local mapped_st
mapped_st="$(_decide sub-status "$sub_exit" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR")" _decided mapped_st sub-status "$sub_exit" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR"
finalize_unfinished "$JOB_ID" "$mapped_st" "$sub_exit" "subscriber" finalize_unfinished "$JOB_ID" "$mapped_st" "$sub_exit" "subscriber"
mapped_st="${FINAL_JOB_STATUS:-$mapped_st}" mapped_st="${FINAL_JOB_STATUS:-$mapped_st}"
local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}" local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}"
echo "$logs_root/$JOB_ID" echo "$logs_root/$JOB_ID"
exit "$(_decide exit-code "$mapped_st")" _exit_for "$mapped_st"
fi fi
fi fi
sleep 0.2 sleep 0.2
@@ -571,7 +597,7 @@ EOF
echo "subscriber output:"; cat "$logf" || true echo "subscriber output:"; cat "$logf" || true
local job_status local job_status
job_status="$(_decide sub-status "$sub_rc" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR")" _decided job_status sub-status "$sub_rc" --job "$JOB_ID" --registry-dir "$REGISTRY_DIR"
echo "Job role $display_role finished with status: $job_status" echo "Job role $display_role finished with status: $job_status"
@@ -590,7 +616,7 @@ EOF
else else
local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}" local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}"
echo "$logs_root/$JOB_ID" echo "$logs_root/$JOB_ID"
exit "$(_decide exit-code "$job_status")" _exit_for "$job_status"
fi fi
fi fi
@@ -615,7 +641,7 @@ EOF
else else
local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}" local logs_root="${DELEGATE_JOB_LOGS_DIR:-$WORKDIR/delegate_job_logs}"
echo "$logs_root/$JOB_ID" echo "$logs_root/$JOB_ID"
exit "$(_decide exit-code "$job_status")" _exit_for "$job_status"
fi fi
fi fi
@@ -805,10 +831,11 @@ _within_deadline() {
finalize_unfinished() { finalize_unfinished() {
local job_id="$1" job_status="$2" sub_rc="$3" phase="${4:-}" local job_id="$1" job_status="$2" sub_rc="$3" phase="${4:-}"
local target_sess="${5:-${current_session:-${target_session:-$AGENT_SESSION}}}" local target_sess="${5:-${current_session:-${target_session:-$AGENT_SESSION}}}"
local step final evidence_until deadline out local step final evidence_until deadline out begin end_rc=0
read -r step final evidence_until deadline <<<"$(_decide finalize-begin --job "$job_id" \ _decided begin finalize-begin --job "$job_id" \
--registry-dir "$REGISTRY_DIR" --status "$job_status" ${START_TIME:+--start-time "$START_TIME"} \ --registry-dir "$REGISTRY_DIR" --status "$job_status" ${START_TIME:+--start-time "$START_TIME"} \
--timeout "$TIMEOUT" --idle-timeout "$IDLE_TIMEOUT" --agent "$AGENT" --session "$target_sess")" --timeout "$TIMEOUT" --idle-timeout "$IDLE_TIMEOUT" --agent "$AGENT" --session "$target_sess"
read -r step final evidence_until deadline <<<"$begin"
case "$step" in case "$step" in
keep) return 0 ;; keep) return 0 ;;
done) FINAL_JOB_STATUS="$final"; return 0 ;; done) FINAL_JOB_STATUS="$final"; return 0 ;;
@@ -826,7 +853,8 @@ finalize_unfinished() {
${START_TIME:+--start-time "$START_TIME"} --timeout "$TIMEOUT" --idle-timeout "$IDLE_TIMEOUT" \ ${START_TIME:+--start-time "$START_TIME"} --timeout "$TIMEOUT" --idle-timeout "$IDLE_TIMEOUT" \
--sub-rc "$sub_rc" --phase "$phase" --type "$TYPE" --final "$final" --evidence-until "$evidence_until" \ --sub-rc "$sub_rc" --phase "$phase" --type "$TYPE" --final "$final" --evidence-until "$evidence_until" \
--dispatch-token "${_disp_tok:-}" --preflight-reason "${PREFLIGHT_REASON:-}" \ --dispatch-token "${_disp_tok:-}" --preflight-reason "${PREFLIGHT_REASON:-}" \
--python "$PY" --scripts-dir "$SCRIPT_DIR/scripts" --watch-pid "$$" 3>&1 1>&4 4>&-)"; } 4>&1 --python "$PY" --scripts-dir "$SCRIPT_DIR/scripts" --watch-pid "$$" 3>&1 1>&4 4>&-)" || end_rc=$?; } 4>&1
[ "$end_rc" -eq 0 ] || _decision_failed finalize-end "rc $end_rc"
FINAL_JOB_STATUS="$out" FINAL_JOB_STATUS="$out"
} }
@@ -1039,7 +1067,8 @@ cmd_wait() {
fi fi
# Issue #33: either clock is a timeout (rc 2) to callers of `wait`, as before; # Issue #33: either clock is a timeout (rc 2) to callers of `wait`, as before;
# the subscriber's own log says which clock it was. # the subscriber's own log says which clock it was.
rc="$(_decide wait-rc "$rc")" _decided rc wait-rc "$rc"
[[ "$rc" =~ ^[0-9]+$ ]] || _decision_failed wait-rc "answer '$rc'"
return "$rc" return "$rc"
} }
+75
View File
@@ -628,3 +628,78 @@ def test_u_a_wait_is_not_stretched_by_a_wall_clock_step_back(monkeypatch, second
monkeypatch.setattr(decide, "time", clock) monkeypatch.setattr(decide, "time", clock)
decide._sleep(argparse.Namespace(watch_pid=None), seconds) decide._sleep(argparse.Namespace(watch_pid=None), seconds)
assert seconds <= clock.slept <= seconds + 0.5, clock.slept assert seconds <= clock.slept <= seconds + 0.5, clock.slept
# ---------------------------------------------------------------------------
# F - a decision wrapper.py cannot make ends the wrapper with 1 (N2)
# ---------------------------------------------------------------------------
def _failing_py(tmp_path: Path, decision: str, mode: str) -> Path:
"""The wrapper's $PY: the real interpreter, except that `decision` fails -
with a traceback and rc 1 (a broken module), with rc 127 (an interpreter
that cannot run), or with rc 0 and no answer.
job_subscriber.py is a stand-in exiting $SUB_RC when SUB_STANDIN=1."""
shim = tmp_path / "py-fail"
fail = {"error": 'echo "ModuleNotFoundError: No module named lib_py.delegation.wrapper" >&2; exit 1',
"gone": 'echo "py: not found" >&2; exit 127', "empty": "exit 0"}[mode]
shim.write_text(
"#!/bin/sh\n"
'if [ "${SUB_STANDIN:-0}" = 1 ]; then case "$1" in */scripts/job_subscriber.py) exit "${SUB_RC:-0}";; esac; fi\n'
f'case "$*" in *"lib_py.delegation.wrapper {decision} "*|*"lib_py.delegation.wrapper {decision}") {fail};; esac\n'
f'exec "{sys.executable}" "$@"\n')
shim.chmod(0o755)
return shim
def _human_submit(tmp_path: Path, py: str) -> subprocess.CompletedProcess:
"""The real wrapper, one human job, an unreachable broker: the subscriber
ends with rc 3, the job is closed at its 3 s wall clock and the wrapper
exits through sub-status, finalize-begin/-end and exit-code (rc 3)."""
env = {k: v for k, v in os.environ.items()
if not k.startswith(("HERDR_", "MAM_", "SKS_", "MQTT_")) and k not in ("PYTHONPATH", "WORKSPACE_ROOT")}
env.update(HOME=str(tmp_path), WORKSPACE_ROOT=str(tmp_path), DELEGATE_JOB_PYTHON=py,
AGENT_PYTHON_BIN=sys.executable, PYTHONPATH=str(SKILLS), DELEGATE_JOB_LOGS_DIR=str(tmp_path / "logs"),
MAM_ENV_FILE=str(tmp_path / ".mam.env.absent"), MQTT_BROKER="127.0.0.1", MQTT_PORT="9", MQTT_TLS="0")
return subprocess.run(["bash", str(WRAPPER), "submit", "--agent", "human", "--registry-dir", str(tmp_path / "reg"),
"--workdir", str(tmp_path), "--prompt", "n2", "--timeout", "3", "--idle-timeout", "2"],
capture_output=True, text=True, env=env, cwd=str(tmp_path), timeout=120)
def test_f_the_normal_path_is_unchanged(tmp_path):
res = _human_submit(tmp_path, sys.executable)
assert res.returncode == 3, res.stdout[-800:] + res.stderr[-800:]
assert res.stdout.strip().splitlines()[-1] == str(tmp_path / "logs" / next((tmp_path / "reg").glob("*.json")).stem)
assert "could not decide" not in res.stderr
@pytest.mark.parametrize("decision,mode", [
("exit-code", "error"), ("exit-code", "empty"),
("sub-status", "error"), ("sub-status", "gone"), ("sub-status", "empty"),
("finalize-begin", "error"), ("finalize-begin", "empty"),
("finalize-end", "error"), ("finalize-end", "gone"), # it may answer nothing (its wrapper gone): rc only
])
def test_f_a_failed_decision_ends_the_submit_with_1(tmp_path, decision, mode):
res = _human_submit(tmp_path, str(_failing_py(tmp_path, decision, mode)))
assert res.returncode == 1, (res.returncode, res.stderr[-800:])
assert f"ERROR: the wrapper could not decide '{decision}'" in res.stderr, res.stderr[-800:]
@pytest.mark.parametrize("mode", ["error", "empty"])
def test_f_a_failed_wait_rc_ends_wait_with_1(tmp_path, mode):
py = _failing_py(tmp_path, "wait-rc", mode)
script = f'source "{WRAPPER}"; PY="{py}"; set +e; (cmd_wait --job j1 --timeout 5); echo "rc=$?"'
res = subprocess.run(["bash", "-c", script], capture_output=True, text=True, cwd=str(tmp_path), timeout=60,
env=_env(tmp_path, SUB_STANDIN=1, SUB_RC=4, DELEGATE_JOB_PYTHON=py))
assert "rc=1" in res.stdout, (res.stdout, res.stderr[-800:])
assert "ERROR: the wrapper could not decide 'wait-rc'" in res.stderr, res.stderr[-800:]
def test_s_no_printed_decision_is_read_raw():
"""Every printed answer goes through _decided/_exit_for; the boolean ones
(subscriber-up, stall-watch) are read by their rc, and finalize-end is
checked by its rc."""
text = "\n".join(l for l in WRAPPER.read_text().splitlines() if not l.lstrip().startswith("#"))
raw = [l.strip() for l in text.splitlines() if "$(_decide" in l]
assert raw == ['__out="$(_decide "$@")" || __rc=$?',
'{ out="$(_decide finalize-end --job "$job_id" --registry-dir "$REGISTRY_DIR" --status "$job_status" \\'], raw
assert 'exit "$(' not in text