From f1331d36ce88b9a1d1a394e2bd1ba3fa87f7a59d Mon Sep 17 00:00:00 2001 From: Godopu Date: Wed, 30 Sep 2026 15:47:40 +0000 Subject: [PATCH 1/8] fix(issue-26): the wrapper reads .mam.env through the Python loader's parser, never `source` - lib_py/delegation/envfile.py (stdlib only) holds the one set of rules: comments and blank lines skipped, key and value stripped, one matching quote pair removed, no shell interpretation, keys must match ^[A-Za-z_][A-Za-z0-9_]*$ (bad or `=`-less lines warn with the line number, never the value). - mqtt_common._parse_env_file delegates to envfile.apply. - The delegate-job wrapper runs envfile.py --shell by path and evals the shlex-quoted exports; values never reach a command line. The environment now wins over the file in both loaders. - .mam.env.example: commented assignments no longer use $HOME, which the parser keeps literally. The shell parsed the env file in one place only (wrapper :38), not three. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib_py/delegation/envfile.py | 78 ++++++ .../skills/lib_py/delegation/mqtt_common.py | 19 +- .../multi-agent-mux-delegate-job | 6 +- .mam.env.example | 8 +- tests/test_issue26_envfile.py | 253 ++++++++++++++++++ 5 files changed, 345 insertions(+), 19 deletions(-) create mode 100644 .agents/skills/lib_py/delegation/envfile.py create mode 100644 tests/test_issue26_envfile.py diff --git a/.agents/skills/lib_py/delegation/envfile.py b/.agents/skills/lib_py/delegation/envfile.py new file mode 100644 index 0000000..6fcce29 --- /dev/null +++ b/.agents/skills/lib_py/delegation/envfile.py @@ -0,0 +1,78 @@ +"""The one `.mam.env` parser (Issue #26). + +Rules: blank lines and `#` comments are skipped; `KEY=VALUE` with the key and +the value stripped; one matching pair of surrounding quotes is removed; no +shell interpretation (`$(...)`, `${...}` and backticks stay literal). A key +must match ``^[A-Za-z_][A-Za-z0-9_]*$``; a line that does not, or that has no +`=`, is skipped with a warning naming the line (never the value — the file +holds secrets). Variables already in the environment win over the file. + +Stdlib only: the delegate-job wrapper runs this file by path +(``envfile.py --shell ``) and evals the ``export`` lines it prints, so +values travel over a pipe, never on a command line. +""" +import os +import re +import shlex +import sys +from typing import List, MutableMapping, Tuple + +_KEY = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") + + +def parse(path: str) -> Tuple[List[Tuple[str, str]], List[str]]: + """Return ([(key, value)], [warning]) for the file at ``path``.""" + pairs: List[Tuple[str, str]] = [] + warnings: List[str] = [] + try: + with open(path, "r", encoding="utf-8") as f: + lines = f.read().splitlines() + except (OSError, UnicodeDecodeError) as e: + return pairs, [f"{path}: cannot read ({type(e).__name__})"] + for n, raw in enumerate(lines, 1): + line = raw.strip() + if not line or line.startswith("#"): + continue + key, sep, val = line.partition("=") + key = key.strip() + if not sep: + warnings.append(f"{path}:{n}: skipped (no '=')") + continue + if not _KEY.fullmatch(key): + warnings.append(f"{path}:{n}: skipped (invalid key)") + continue + val = val.strip() + if len(val) >= 2 and val[0] == val[-1] and val[0] in "\"'": + val = val[1:-1] + pairs.append((key, val)) + return pairs, warnings + + +def apply(path: str, environ: MutableMapping[str, str]) -> Tuple[List[Tuple[str, str]], List[str]]: + """Set each pair not already in ``environ``; return (the pairs set, warnings).""" + pairs, warnings = parse(path) + applied = [] + for key, val in pairs: + if key not in environ: + environ[key] = val + applied.append((key, val)) + return applied, warnings + + +def main(argv: List[str]) -> int: + if len(argv) != 2 or argv[0] != "--shell": + print("usage: envfile.py --shell ", file=sys.stderr) + return 2 + path = argv[1] + if not os.path.isfile(path): + return 0 + applied, warnings = apply(path, dict(os.environ)) + for w in warnings: + print(f"WARNING: {w}", file=sys.stderr) + for key, val in applied: + print(f"export {key}={shlex.quote(val)}") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/.agents/skills/lib_py/delegation/mqtt_common.py b/.agents/skills/lib_py/delegation/mqtt_common.py index d506b47..cad9466 100644 --- a/.agents/skills/lib_py/delegation/mqtt_common.py +++ b/.agents/skills/lib_py/delegation/mqtt_common.py @@ -31,6 +31,8 @@ from typing import Any, Callable, Dict, Iterable, List, Optional import paho.mqtt.client as mqtt +from lib_py.delegation import envfile + logger = logging.getLogger("delegate_job.mqtt_common") _warned_deprecated_env = False @@ -94,20 +96,9 @@ def _load_dotenv(workspace_dir: Optional[str] = None) -> None: _parse_env_file(legacy_env_path) def _parse_env_file(path: str) -> None: - try: - with open(path, "r", encoding="utf-8") as f: - for line in f: - line = line.strip() - if not line or line.startswith("#"): - continue - if "=" in line: - key, _, val = line.partition("=") - key = key.strip() - val = val.strip().strip('"').strip("'") - if key and key not in os.environ: - os.environ[key] = val - except Exception as e: - logger.warning("Failed to parse env file %s: %s", path, e) + """Issue #26: the rules live in envfile.py, shared with the wrapper.""" + for warning in envfile.apply(path, os.environ)[1]: + logger.warning("%s", warning) _env_loaded = False diff --git a/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job b/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job index 103501b..2875cfd 100755 --- a/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job +++ b/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job @@ -34,8 +34,12 @@ if [[ -z "$TARGET_ENV" ]]; then fi fi +# Issue #26: parsed by envfile.py (the Python loader's rules; environment wins), +# never sourced. Values come back as shlex-quoted exports over the pipe. if [[ -n "$TARGET_ENV" && -f "$TARGET_ENV" ]]; then - set -a; source "$TARGET_ENV"; set +a + _env_exports="$("${DELEGATE_JOB_PYTHON:-python3}" -I "$SCRIPT_DIR/../lib_py/delegation/envfile.py" --shell "$TARGET_ENV")" \ + || { echo "ERROR: cannot read $TARGET_ENV" >&2; exit 1; } + eval "$_env_exports"; unset _env_exports fi # Source EARLY (before any herdr usage in run_agent) — this is what turns diff --git a/.mam.env.example b/.mam.env.example index 0b77b0f..57ace7d 100644 --- a/.mam.env.example +++ b/.mam.env.example @@ -29,11 +29,11 @@ # Root directory that holds Claude Code per-project conversation logs (*.jsonl). #default: $HOME/.claude/projects -# CLAUDE_PROJECT_DIR=$HOME/.claude/projects +# CLAUDE_PROJECT_DIR=/home/you/.claude/projects # Directory scanned for per-session launcher wrappers (~/.local/bin/). #default: $HOME/.local/bin -# LOCAL_BIN=$HOME/.local/bin +# LOCAL_BIN=/home/you/.local/bin # Isolated Herdr session socket name (`herdr --session `). #default: mam- @@ -46,7 +46,7 @@ # Path to the Herdr Unix domain socket. #default: $HOME/.config/herdr/herdr.sock -# HERDR_SOCKET_PATH=$HOME/.config/herdr/herdr.sock +# HERDR_SOCKET_PATH=/home/you/.config/herdr/herdr.sock # Explicit Herdr workspace scope identifier (`herdr pane list --workspace `). # Hard filters pane/agent resolution to this workspace. When unset, falls back @@ -107,7 +107,7 @@ # Optional override for Codex CLI state / session directory. #default: $HOME/.codex -# CODEX_HOME=$HOME/.codex +# CODEX_HOME=/home/you/.codex # Optional inline JSON permissions configuration for OpenCode. #default: (unset → defaults to --auto CLI bypass) diff --git a/tests/test_issue26_envfile.py b/tests/test_issue26_envfile.py new file mode 100644 index 0000000..d2bb1e2 --- /dev/null +++ b/tests/test_issue26_envfile.py @@ -0,0 +1,253 @@ +"""Issue #26 (A): one `.mam.env` parser for the wrapper and the Python loader. + +- A1: the issue's file no longer kills the real wrapper, runs no shell, warns + per bad line without leaking values. +- A2: the real wrapper and the real Python loader read one table the same way. +- A3: an environment variable beats the file, in both loaders. +- A4: the parser's command line carries the path only. +- A5: the files this repo ships or generates parse without a warning. +""" + +from __future__ import annotations + +import json +import os +import re +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SKILLS = REPO_ROOT / ".agents" / "skills" +WRAPPER = SKILLS / "multi-agent-mux-delegate-job" / "multi-agent-mux-delegate-job" +ENVFILE = SKILLS / "lib_py" / "delegation" / "envfile.py" + +if str(SKILLS) not in sys.path: + sys.path.insert(0, str(SKILLS)) + +from lib_py.delegation import envfile # noqa: E402 + +ISSUE_FILE = ( + "AGENT_SESSIONS_YAML= .mam/agent-sessions.yaml\n" + "X=$(touch pwned)\n" + "BAD KEY=1\n" + "noequals\n" + 'MQTT_PASSWORD="s3cr3t-a1"\n' +) +CLEAN_FILE = ( + "# comment\n" + "\n" + "AGENT_SESSIONS_YAML=.mam/agent-sessions.yaml\n" + 'MQTT_PASSWORD="s3cr3t-a1"\n' +) +# key -> value both loaders must produce (A2) +TABLE_FILE = ( + " SPACED = padded value \n" + "# ignored=1\n" + "\n" + "DQ=\"double quoted\"\n" + "SQ='single quoted'\n" + "HALF=\"unmatched\n" + "MIXED=\"a'\n" + "EQ=a=b=c\n" + "EMPTY=\n" + "LIT=$(echo no) ${HOME} `echo no`\n" + "export EXPORTED=1\n" + "1BAD=1\n" +) +TABLE = { + "SPACED": "padded value", + "DQ": "double quoted", + "SQ": "single quoted", + "HALF": '"unmatched', + "MIXED": "\"a'", + "EQ": "a=b=c", + "EMPTY": "", + "LIT": "$(echo no) ${HOME} `echo no`", +} +TABLE_SKIPPED = {"EXPORTED", "export EXPORTED", "1BAD"} + + +def _env(home: Path, env_file: Path, extra=None) -> dict: + env = {k: v for k, v in os.environ.items() if k not in TABLE and k not in ("MAM_JOB_TIMEOUT_S", "MQTT_PASSWORD", + "AGENT_SESSIONS_YAML", "X")} + env.update({"HOME": str(home), "WORKSPACE_ROOT": str(home), "MAM_ENV_FILE": str(env_file), + "DELEGATE_JOB_PYTHON": sys.executable, "AGENT_PYTHON_BIN": sys.executable, + "PYTHONPATH": str(SKILLS), "DELEGATE_JOB_LOGS_DIR": str(home / "logs")}) + if extra: + env.update(extra) + return env + + +def _wrapper_values(env: dict, keys, cwd: Path) -> subprocess.CompletedProcess: + """The values the real wrapper exported after loading its env file.""" + probe = "; ".join(f'printf "%s=%s\\0" {k} "${{{k}-}}"' for k in keys) + return subprocess.run(["bash", "-c", f'source "{WRAPPER}"; {probe}'], capture_output=True, text=True, + env={**env, "MAM_DELEGATE_JOB_NO_MAIN": "1"}, cwd=str(cwd), timeout=60) + + +def _python_values(env: dict, keys) -> dict: + """The values the real Python loader (mqtt_common) set from the same file.""" + code = ("import json, os; from lib_py.delegation import mqtt_common; mqtt_common.ensure_env_loaded(); " + f"print(json.dumps({{k: os.environ.get(k, '') for k in {list(keys)!r}}}))") + res = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, env=env, timeout=60) + assert res.returncode == 0, res.stderr + return json.loads(res.stdout.strip().splitlines()[-1]) + + +def _split(out: str) -> dict: + return dict(item.split("=", 1) for item in out.split("\0") if item) + + +# --------------------------------------------------------------------------- +# A1 - the issue's file +# --------------------------------------------------------------------------- + +def test_a1_the_issue_file_does_not_kill_the_wrapper_nor_run_shell(tmp_path): + f = tmp_path / "issue.env" + f.write_text(ISSUE_FILE) + res = subprocess.run(["bash", str(WRAPPER), "--help"], capture_output=True, text=True, + env=_env(tmp_path, f), cwd=str(tmp_path), timeout=60) + assert res.returncode == 0, res.stderr + assert "submit" in res.stdout + assert not (tmp_path / "pwned").exists(), "a value must never be run as shell" + for n, reason in ((3, "invalid key"), (4, "no '='")): + assert f"{f}:{n}: skipped ({reason})" in res.stderr, res.stderr + assert ":1: skipped" not in res.stderr and ":2: skipped" not in res.stderr + assert "s3cr3t-a1" not in res.stdout + res.stderr + + pairs, warnings = envfile.parse(str(f)) + assert dict(pairs) == {"AGENT_SESSIONS_YAML": ".mam/agent-sessions.yaml", "X": "$(touch pwned)", + "MQTT_PASSWORD": "s3cr3t-a1"} + assert len(warnings) == 2 and not any("s3cr3t" in w for w in warnings) + + +def test_a1_a_clean_file_loads_silently(tmp_path): + f = tmp_path / "clean.env" + f.write_text(CLEAN_FILE) + res = subprocess.run(["bash", str(WRAPPER), "--help"], capture_output=True, text=True, + env=_env(tmp_path, f), cwd=str(tmp_path), timeout=60) + assert res.returncode == 0, res.stderr + assert "skipped" not in res.stderr and "WARNING" not in res.stderr, res.stderr + got = _wrapper_values(_env(tmp_path, f), ["AGENT_SESSIONS_YAML", "MQTT_PASSWORD"], tmp_path) + assert _split(got.stdout) == {"AGENT_SESSIONS_YAML": ".mam/agent-sessions.yaml", "MQTT_PASSWORD": "s3cr3t-a1"} + + +def test_a1_an_unreadable_parser_fails_loudly(tmp_path): + """The wrapper stops when the parser cannot run instead of going on unconfigured.""" + f = tmp_path / "clean.env" + f.write_text(CLEAN_FILE) + res = subprocess.run(["bash", str(WRAPPER), "--help"], capture_output=True, text=True, + env=_env(tmp_path, f, {"DELEGATE_JOB_PYTHON": str(tmp_path / "no-python")}), + cwd=str(tmp_path), timeout=60) + assert res.returncode == 1 + assert f"ERROR: cannot read {f}" in res.stderr + + +# --------------------------------------------------------------------------- +# A2 - one table, two real loaders, same values +# --------------------------------------------------------------------------- + +def test_a2_the_wrapper_and_the_python_loader_read_the_same_values(tmp_path): + f = tmp_path / "table.env" + f.write_text(TABLE_FILE) + env = _env(tmp_path, f) + keys = list(TABLE) + ["EXPORTED"] + res = _wrapper_values(env, keys, tmp_path) + assert res.returncode == 0, res.stderr + wrapper = _split(res.stdout) + python = _python_values(env, keys) + assert wrapper == python == {**TABLE, "EXPORTED": ""} + for n in (11, 12): + assert f"{f}:{n}: skipped (invalid key)" in res.stderr, res.stderr + pairs, _ = envfile.parse(str(f)) + assert not TABLE_SKIPPED & {k for k, _ in pairs} + + +# --------------------------------------------------------------------------- +# A3 - the environment beats the file, in both loaders +# --------------------------------------------------------------------------- + +def _dry_budget(env: dict, cwd: Path) -> int: + reg = cwd / "reg" + res = subprocess.run(["bash", str(WRAPPER), "submit", "--agent", "human", "--dry-run", + "--registry-dir", str(reg), "--prompt", "p"], + capture_output=True, text=True, env=env, cwd=str(cwd), timeout=120) + assert res.returncode == 0, res.stdout + res.stderr + jid = re.search(r"registered job:\s*(\S+)", res.stdout).group(1) + return json.loads((reg / f"{jid}.json").read_text())["timeout_sec"] + + +def test_a3_an_environment_value_beats_the_file(tmp_path): + f = tmp_path / "budget.env" + f.write_text("MAM_JOB_TIMEOUT_S=50\n") + (tmp_path / "file").mkdir() + (tmp_path / "both").mkdir() + assert _dry_budget(_env(tmp_path, f), tmp_path / "file") == 50 + both = _env(tmp_path, f, {"MAM_JOB_TIMEOUT_S": "70"}) + assert _dry_budget(both, tmp_path / "both") == 70 + assert _python_values(both, ["MAM_JOB_TIMEOUT_S"]) == {"MAM_JOB_TIMEOUT_S": "70"} + assert _python_values(_env(tmp_path, f), ["MAM_JOB_TIMEOUT_S"]) == {"MAM_JOB_TIMEOUT_S": "50"} + + +# --------------------------------------------------------------------------- +# A4 - the parser sees the path only +# --------------------------------------------------------------------------- + +def test_a4_the_parser_command_line_carries_the_path_only(): + calls = [l for l in WRAPPER.read_text().splitlines() if "envfile.py" in l and not l.lstrip().startswith("#")] + assert len(calls) == 1, calls + assert re.search(r'envfile\.py" --shell "\$TARGET_ENV"\)"', calls[0]), calls[0] + src = WRAPPER.read_text() + assert not re.search(r'^\s*(source|\.)\s+"?\$TARGET_ENV', src, re.M), "the env file must never be sourced" + assert "set -a" not in src + + +# --------------------------------------------------------------------------- +# A5 - the files the repo ships or generates follow the format +# --------------------------------------------------------------------------- + +def _install_seed(tmp_path: Path) -> Path: + """The .mam.env install.sh writes, from its own heredoc run by bash.""" + src = (REPO_ROOT / "deploy" / "install.sh").read_text() + m = re.search(r'cat < "\$MAM_ENV"\n(.*?\nEOF)\n', src, re.S) + assert m, "install.sh no longer seeds .mam.env with a heredoc" + script = f'MAM_ENV="{tmp_path / "seed.env"}"; MAM_PORT=1883; MAM_CLIENT_PREFIX=hermes\ncat < "$MAM_ENV"\n{m.group(1)}\n' + subprocess.run(["bash", "-c", script], check=True, env={**os.environ, "HOME": str(tmp_path)}, timeout=30) + return tmp_path / "seed.env" + + +def test_a5_shipped_and_generated_env_files_parse_without_warnings(tmp_path): + ws = tmp_path / "ws" + (ws / "deploy").mkdir(parents=True) + (ws / ".mam.env.example").write_text((REPO_ROOT / ".mam.env.example").read_text()) + (ws / "deploy" / "generate-env.sh").write_text((REPO_ROOT / "deploy" / "generate-env.sh").read_text()) + subprocess.run(["bash", str(ws / "deploy" / "generate-env.sh")], check=True, capture_output=True, timeout=30) + for f in (REPO_ROOT / ".mam.env.example", ws / ".mam.env", _install_seed(tmp_path)): + pairs, warnings = envfile.parse(str(f)) + assert warnings == [], (f, warnings) + assert not any("$" in v or "`" in v for _, v in pairs), (f, pairs) + + # The template's commented assignments are what a user uncomments: they must + # parse too, and hold no shell syntax the parser would keep literally. + commented = tmp_path / "uncommented.env" + lines = [l[2:] for l in (REPO_ROOT / ".mam.env.example").read_text().splitlines() + if re.match(r"# [A-Za-z_][A-Za-z0-9_]*=", l)] + assert len(lines) > 20 + commented.write_text("\n".join(lines) + "\n") + pairs, warnings = envfile.parse(str(commented)) + assert warnings == [] and len(pairs) == len(lines) + assert not [(k, v) for k, v in pairs if "$" in v or "`" in v] + + +@pytest.mark.parametrize("missing", [True, False]) +def test_a5_shell_mode_exit_code_is_zero_for_absent_and_present_files(tmp_path, missing): + f = tmp_path / "x.env" + if not missing: + f.write_text("A=1\n") + res = subprocess.run([sys.executable, "-I", str(ENVFILE), "--shell", str(f)], capture_output=True, text=True, + env={k: v for k, v in os.environ.items() if k != "A"}, timeout=30) + assert res.returncode == 0 + assert res.stdout == ("" if missing else "export A=1\n") -- 2.54.0 From 80dc2eb45e45ba812178f2edeec72c4ea17f7f86 Mon Sep 17 00:00:00 2001 From: Godopu Date: Wed, 30 Sep 2026 15:47:40 +0000 Subject: [PATCH 2/8] fix(issue-26): integer knobs that reach bash arithmetic are read as decimal bash read MAM_JOB_TIMEOUT_S=0700 as octal 448 (and failed on 0900) while the loop runner read 700. One rule now, with a per-knob minimum: lib.sh mam_int / mam_int_knob and runner._positive_int (blanks allowed, digits only, decimal, min <= n <= 999999999). - minimum 1: MAM_JOB_TIMEOUT_S, MAM_HEARTBEAT_SEC, MAM_HEARTBEAT_MISSES, MAM_ACK_TIMEOUT_S (invalid env: warning + default) and the wrapper's --timeout/--idle-timeout/--heartbeat/--heartbeat-misses/--ack-timeout (invalid: rc 1); the runner's job_budget, --job-timeout, contract_args. - minimum 0 (0 = check once): MAM_INJECT_ACK_GRACE_S, delegate_wait_started's ack_s, SKS_DIALOG_TIMEOUT, SKS_BUSY_TIMEOUT, SKS_DELIVERY_GRACE_S. - tests/test_issue26_ints.py: INT_KNOBS is the one table of minimums; a static guard keeps env defaults out of arithmetic and checks each normalised knob against it. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/lib.sh | 36 ++- .agents/skills/lib_py/loop/runner.py | 34 +- .../multi-agent-mux-delegate-job | 26 +- tests/test_issue26_ints.py | 295 ++++++++++++++++++ 4 files changed, 367 insertions(+), 24 deletions(-) create mode 100644 tests/test_issue26_ints.py diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index fa931a9..ef5db06 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -2335,6 +2335,35 @@ sys.exit(1) ' 2>/dev/null } +# mam_int +# +# Issue #26: the one integer rule for knobs that reach bash arithmetic (the +# Python twin is lib_py/loop/runner.py::_positive_int). Surrounding blanks are +# allowed, then digits only, read as DECIMAL (bash would read 0700 as octal +# 448 and fail on 0900), and min <= n <= 999999999. Prints n and returns 0, or +# returns 1. The per-knob minimum lives in tests/test_issue26_ints.py. +mam_int() { + [[ "${1-}" =~ ^[[:space:]]*([0-9]+)[[:space:]]*$ ]] || return 1 + local d="${BASH_REMATCH[1]}" + d="${d#"${d%%[!0]*}"}" + [ ${#d} -le 9 ] || return 1 + local n=$(( 10#${d:-0} )) + [ "$n" -ge "$2" ] || return 1 + printf '%s\n' "$n" +} + +# mam_int_knob +# +# The value of $ENV_NAME through mam_int; unset or empty gives , an +# invalid value gives with a warning on stderr. +mam_int_knob() { + local raw="${!1-}" + [ -n "$raw" ] || { printf '%s\n' "$2"; return 0; } + mam_int "$raw" "$3" && return 0 + echo "WARNING: $1=$raw ignored (not a decimal integer >= $3); using $2" >&2 + printf '%s\n' "$2" +} + # delegate_wait_started [ack_timeout_s] [dispatch_token] # # ISSUE-23 / ISSUE-18: delivery is confirmed by the protocol, not by the @@ -2347,6 +2376,7 @@ sys.exit(1) delegate_wait_started() { local job_id="$1" ack_s="${2:-180}" tok="${3:-}" [ -n "$job_id" ] || return 1 + ack_s=$(mam_int "$ack_s" 0) || { echo "WARNING: onboarding ack timeout ${2-} ignored (not a decimal integer >= 0); using 180" >&2; ack_s=180; } local deadline deadline=$(( $(date +%s) + ack_s )) while :; do @@ -2759,7 +2789,7 @@ send_keys_safe() { fi if [ "$_q_rc" != "2" ]; then - deadline=$(( $(date +%s) + ${SKS_DIALOG_TIMEOUT:-30} )) + deadline=$(( $(date +%s) + $(mam_int_knob SKS_DIALOG_TIMEOUT 30 0) )) while _pane_dialog_open "$sess"; do if [ "${SKS_DIALOG_ESCAPE:-0}" = "1" ]; then _sks_herdr send-keys -t "$sess" Escape @@ -2778,7 +2808,7 @@ send_keys_safe() { # Busy gate: wait up to SKS_BUSY_TIMEOUT only if wait_for_agent_idle returned 3 (unknown) if [ "${_SKS_IDLE_RC:-0}" = "3" ]; then - local b_deadline=$(( $(date +%s) + ${SKS_BUSY_TIMEOUT:-120} )) + local b_deadline=$(( $(date +%s) + $(mam_int_knob SKS_BUSY_TIMEOUT 120 0) )) local still_busy=1 while [ "$(date +%s)" -lt "$b_deadline" ]; do local tail_pane @@ -2847,7 +2877,7 @@ send_keys_safe() { # still fails closed (rc 4); the wrapper then waits for `started` up to # the job's ack timeout T before finalising the inject as failed. if printf '%s\n' "$p_out" | grep -Eq '"code": *"(timeout|agent_prompt_stalled)"'; then - local g_deadline=$(( $(date +%s) + ${SKS_DELIVERY_GRACE_S:-15} )) + local g_deadline=$(( $(date +%s) + $(mam_int_knob SKS_DELIVERY_GRACE_S 15 0) )) while [ "$(date +%s)" -lt "$g_deadline" ]; do [ "$(_sks_agent_status "$sess" "$agent_target")" = "working" ] && return 0 _sks_job_started "$job_id" && return 0 diff --git a/.agents/skills/lib_py/loop/runner.py b/.agents/skills/lib_py/loop/runner.py index cbce7b7..679946d 100644 --- a/.agents/skills/lib_py/loop/runner.py +++ b/.agents/skills/lib_py/loop/runner.py @@ -67,6 +67,16 @@ def log_error(msg: str) -> None: print(f"{_COLOR_ERROR} {msg}", file=sys.stderr) + +def _positive_int(text: Optional[str], minimum: int = 1) -> Optional[int]: + """Issue #26: lib.sh mam_int's rule - blanks around, digits only, decimal + (0700 is 700), minimum <= n <= 999999999; else None.""" + s = (text or "").strip(" \t\n\r\v\f") + if not s or not s.isascii() or not s.isdigit(): + return None + n = int(s) + return n if minimum <= n <= 999_999_999 else None + class LoopRunner: """Orchestrates planning, code implementation, and multi-agent peer reviews.""" @@ -142,7 +152,7 @@ class LoopRunner: " --all-reviewer Enforce PASS verdict from all active reviewer sessions", " --max-loop N Max execution-review corrective loop runs (default: 3)", " --max-rebut N Max rebuttal attempts per review iteration (default: 1, 0: disabled)", - " --job-timeout Budget of each delegated job (default: $MAM_JOB_TIMEOUT_S or 3600)", + " --job-timeout Budget of each delegated job (positive decimal integer; default: $MAM_JOB_TIMEOUT_S or 3600)", " --verbose Print detailed execution timeline traces", " --cleanup Purge temporary job directories upon success", " --worktree Source worktree for diff/base/head (control plane stays here)", @@ -214,10 +224,10 @@ class LoopRunner: self.max_loop = int(args[i + 1]) i += 2 elif arg == "--job-timeout": - if i + 1 >= len(args) or not re.match(r"^[0-9]+$", args[i + 1]) or int(args[i + 1]) <= 0: - print("ERROR: --job-timeout requires a positive non-zero integer.", file=sys.stderr) + self.job_timeout = _positive_int(args[i + 1]) if i + 1 < len(args) else None + if self.job_timeout is None: + print("ERROR: --job-timeout requires a positive decimal integer.", file=sys.stderr) sys.exit(1) - self.job_timeout = int(args[i + 1]) i += 2 elif arg == "--max-rebut": if i + 1 >= len(args) or not re.match(r"^[0-9]+$", args[i + 1]): @@ -471,9 +481,9 @@ class LoopRunner: for env_name, flag in (("MAM_HEARTBEAT_SEC", "--heartbeat"), ("MAM_HEARTBEAT_MISSES", "--heartbeat-misses"), ("MAM_ACK_TIMEOUT_S", "--ack-timeout")): - val = os.environ.get(env_name) - if val and val.isdigit() and int(val) > 0: - args += [flag, val] + val = _positive_int(os.environ.get(env_name)) + if val is not None: + args += [flag, str(val)] return args def resolve_all_reviewers(self, state: Dict[str, Any]) -> List[str]: @@ -593,11 +603,17 @@ class LoopRunner: @property def job_budget(self) -> int: - """Issue #33: --job-timeout > $MAM_JOB_TIMEOUT_S (positive integer) > 3600.""" + """Issue #33: --job-timeout > $MAM_JOB_TIMEOUT_S (positive decimal integer) > 3600.""" if self.job_timeout: return self.job_timeout env = os.environ.get("MAM_JOB_TIMEOUT_S", "") - return int(env) if env.isdigit() and int(env) > 0 else 3600 + n = _positive_int(env) + if n is None and env: + if not getattr(self, "_warned_job_timeout", False): + self._warned_job_timeout = True + print(f"WARNING: MAM_JOB_TIMEOUT_S={env} ignored (not a decimal integer >= 1); using 3600", file=sys.stderr) + return 3600 + return n or 3600 @property def job_wait_s(self) -> int: diff --git a/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job b/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job index 2875cfd..290187f 100755 --- a/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job +++ b/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job @@ -115,7 +115,7 @@ usage() { multi-agent-mux-delegate-job [options] submit --agent (--prompt | --prompt-file ) [--workdir ] [--agent-session