fix(issue-17): follow-ups - a failed decision ends the wrapper with 1; resume closes herdr 0.9.3's placeholder (v4.8.7) #52
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"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
||||||
Reference in new issue
Block a user