diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index f37d3b3..c12b66f 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -2335,39 +2335,10 @@ 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() { - # Spelled-out ASCII sets, not [[:space:]]/[0-9]: those follow the locale, and - # the runner's _positive_int takes ASCII only (review R2: U+2003 30 U+2003 - # was 30 here under en_US.UTF-8 and invalid there). - local _ws=$' \t\n\r\v\f' _re - _re="^[${_ws}]*([0123456789]+)[${_ws}]*\$" - [[ "${1-}" =~ $_re ]] || 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" -} +# mam_int / mam_int_knob (Issue #26) live in lib_int.sh, shared with the +# scripts that run without lib.sh. +# shellcheck source=lib_int.sh +source "$(dirname "${BASH_SOURCE[0]}")/lib_int.sh" # delegate_wait_started [ack_timeout_s] [dispatch_token] # @@ -2443,7 +2414,7 @@ wait_for_tui_ready() { fi local empty_streak=0 - local empty_giveup="${MAM_READY_EMPTY_GIVEUP:-5}" + local empty_giveup="$(mam_int_knob MAM_READY_EMPTY_GIVEUP 5 1)" local modal_pat="$_MAM_MODAL_TOKENS" [ -n "$modal_tokens" ] && modal_pat="${modal_pat}|${modal_tokens}" @@ -2639,7 +2610,7 @@ _wait_session_gone() { _pane_quiescent() { local sess="$1" tries="${2:-20}" interval="${3:-0.5}" prev="__none__" cur i local saw_output=0 empty_streak=0 - local empty_giveup="${SKS_EMPTY_GIVEUP:-3}" + local empty_giveup="$(mam_int_knob SKS_EMPTY_GIVEUP 3 1)" for ((i = 0; i < tries; i++)); do cur=$(_pane_capture "$sess") if [ -z "$cur" ]; then @@ -2709,8 +2680,8 @@ except Exception: return 0 fi - local max_retries="${SKS_IDLE_RETRIES:-2}" - local backoff_s="${SKS_IDLE_RETRY_BACKOFF_S:-10}" + local max_retries="$(mam_int_knob SKS_IDLE_RETRIES 2 0)" + local backoff_s="$(mam_int_knob SKS_IDLE_RETRY_BACKOFF_S 10 0)" local try=0 while [ "$try" -le "$max_retries" ]; do local watcher_out="" watcher_rc=0 diff --git a/.agents/skills/lib_int.sh b/.agents/skills/lib_int.sh new file mode 100644 index 0000000..39ef2dd --- /dev/null +++ b/.agents/skills/lib_int.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# lib_int.sh - the integer rule for knobs (Issue #26), on its own so scripts +# that run without lib.sh (diff_collect.sh and loop_lock.sh, which the Python +# runner and the tests source alone) read their knobs the same way. lib.sh +# sources it. Functions only: sourcing it has no other effect. + +# 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() { + # Spelled-out ASCII sets, not [[:space:]]/[0-9]: those follow the locale, and + # the runner's _positive_int takes ASCII only (review R2: U+2003 30 U+2003 + # was 30 here under en_US.UTF-8 and invalid there). + local _ws=$' \t\n\r\v\f' _re + _re="^[${_ws}]*([0123456789]+)[${_ws}]*\$" + [[ "${1-}" =~ $_re ]] || 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" +} diff --git a/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh b/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh index e95d3c6..e5d6de6 100755 --- a/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh +++ b/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh @@ -9,8 +9,11 @@ MAM_DIFF_NESTED_MARKER="!!! NOT EXPANDED IN THIS DIFF !!!" # These caps bound the evidence a reviewer is shown, nothing else. The prompt # reaches the agent by file (Issue #36), so delivery has no size cap of its own: # only a diff over these limits is truncated (and marked). -MAM_DIFF_MAX_BYTES="${MAM_DIFF_MAX_BYTES:-200000}" -MAM_DIFF_MAX_LINES="${MAM_DIFF_MAX_LINES:-4000}" +# The runner sources this file without lib.sh; the integer rule is lib_int.sh's. +# shellcheck source=../../lib_int.sh +declare -F mam_int_knob >/dev/null || source "$(dirname "${BASH_SOURCE[0]}")/../../lib_int.sh" +MAM_DIFF_MAX_BYTES="$(mam_int_knob MAM_DIFF_MAX_BYTES 200000 1)" +MAM_DIFF_MAX_LINES="$(mam_int_knob MAM_DIFF_MAX_LINES 4000 1)" _mam_nested_dir_diff() { local d="$1" outer_top="$2" inner_top="" diff --git a/.agents/skills/multi-agent-mux-loop/scripts/loop_lock.sh b/.agents/skills/multi-agent-mux-loop/scripts/loop_lock.sh index 2be32a7..1dc8b83 100755 --- a/.agents/skills/multi-agent-mux-loop/scripts/loop_lock.sh +++ b/.agents/skills/multi-agent-mux-loop/scripts/loop_lock.sh @@ -2,7 +2,10 @@ # loop_lock.sh — Atomic race-free loop lock acquisition and marker ownership (O-2) # Manages .mam/loop-guard-active with atomic hard links and pid+lstart identity validation. -MAM_LOCK_STEAL_RETRIES="${MAM_LOCK_STEAL_RETRIES:-3}" +# This file is also sourced without lib.sh (tests); the integer rule is lib_int.sh's. +# shellcheck source=../../lib_int.sh +declare -F mam_int_knob >/dev/null || source "$(dirname "${BASH_SOURCE[0]}")/../../lib_int.sh" +MAM_LOCK_STEAL_RETRIES="$(mam_int_knob MAM_LOCK_STEAL_RETRIES 3 0)" mam_lstart() { local pid="${1:-$$}" diff --git a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh index e6d02ce..4544be4 100755 --- a/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh +++ b/.agents/skills/multi-agent-mux-monitor/scripts/reconcile.sh @@ -38,7 +38,7 @@ SUBSCRIBE=0 # (raised from 600s to align with job timeout defaults); idle 0 = never idle-out. SUB_TIMEOUT=0 SUB_IDLE_TIMEOUT=3600 -POLL_INTERVAL="${RECONCILE_POLL_INTERVAL:-15}" +POLL_INTERVAL="$(mam_int_knob RECONCILE_POLL_INTERVAL 15 1)" while [ $# -gt 0 ]; do case "$1" in diff --git a/deploy/remove.sh b/deploy/remove.sh index f47045f..08b09a6 100644 --- a/deploy/remove.sh +++ b/deploy/remove.sh @@ -82,6 +82,7 @@ if [ -f "$MANIFEST_FILE" ]; then else fallback_assets=( ".agents/skills/lib.sh" + ".agents/skills/lib_int.sh" ".agents/skills/lib_py" ".agents/skills/multi-agent-mux-create" ".agents/skills/multi-agent-mux-delegate-job" diff --git a/tests/test_issue21_loop_b.py b/tests/test_issue21_loop_b.py index 1e426c2..339e4d5 100644 --- a/tests/test_issue21_loop_b.py +++ b/tests/test_issue21_loop_b.py @@ -393,6 +393,7 @@ def test_l15_truncated_diff_refuses_diff_only_but_not_local(wt_repo, monkeypatch dst = main / ".agents" / "skills" / "multi-agent-mux-loop" / "scripts" dst.mkdir(parents=True) (dst / "diff_collect.sh").write_text(DIFF_SH.read_text()) + (dst.parent.parent / "lib_int.sh").write_text((SKILLS / "lib_int.sh").read_text()) # its integer rule monkeypatch.setenv("MAM_DIFF_MAX_LINES", "1") _state(monkeypatch, LOCAL_ROWS + [REMOTE_ROW]) argv = ["--worktree", str(wt), "--base", before, "--review-only", "--remote-review", "diff-only"] diff --git a/tests/test_issue26_ints.py b/tests/test_issue26_ints.py index 76766ca..4dc06e9 100644 --- a/tests/test_issue26_ints.py +++ b/tests/test_issue26_ints.py @@ -43,6 +43,18 @@ INT_KNOBS = { "SKS_DELIVERY_GRACE_S": (0, "15"), # _pane_quiescent's `for ((i = 0; i < tries; i++))` (review R2); 0 keeps "headless" (rc 2) "SKS_QUIESCENT_TRIES": (0, "20"), + # Follow-up F4b: `[ -ge ]`/`[ -le ]`/`[ -gt ]` operands and integer sleeps, where + # "abc" was a `[` error or a sleep error. lib.sh, reconcile.sh (which sources + # it), and loop_lock.sh / diff_collect.sh, which run without lib.sh and + # source lib_int.sh themselves. + "MAM_READY_EMPTY_GIVEUP": (1, "5"), # consecutive empty captures; 0 would act as 1 + "SKS_EMPTY_GIVEUP": (1, "3"), # the same, in _pane_quiescent + "SKS_IDLE_RETRIES": (0, "2"), # 0: one wait, no retry + "SKS_IDLE_RETRY_BACKOFF_S": (0, "10"), # sleep between retries; test_issue6 uses 0 + "MAM_LOCK_STEAL_RETRIES": (0, "3"), # 0: one attempt, no steal retry + "MAM_DIFF_MAX_BYTES": (1, "200000"), # 0 would truncate every diff + "MAM_DIFF_MAX_LINES": (1, "4000"), + "RECONCILE_POLL_INTERVAL": (1, "15"), # sleep; 0 would poll in a busy loop # delegate_wait_started's ack_s (MAM_ONBOARD_ACK_S / DELEGATE_JOB_ACK_S via create_session.sh) "delegate_wait_started:ack_s": (0, "180"), # wrapper flags @@ -85,11 +97,8 @@ NOT_ARITHMETIC = { "MAM_MAX_PANE_COLS": "argv of python -m lib_py.layout", "MAM_MAX_PANE_ROWS": "argv of python -m lib_py.layout", "MAM_BUFFER_GC_MINUTES": "find -mmin", "MAM_DIALOG_TAIL_LINES": "tail -n", "SKS_IDLE_TIMEOUT_MS": "argv of the python idle watcher", "MAM_DIALOG_RESEND_S": "python int()", - "MAM_READY_EMPTY_GIVEUP": "[ -ge ] (decimal)", "SKS_EMPTY_GIVEUP": "[ -ge ] (decimal)", - "SKS_IDLE_RETRIES": "[ -le ] (decimal)", "SKS_IDLE_RETRY_BACKOFF_S": "sleep", - "SKS_QUIESCENT_INTERVAL": "sleep", "MAM_LOCK_STEAL_RETRIES": "[ -le ] (decimal)", - "MAM_DIFF_MAX_BYTES": "[ -gt ] (decimal)", "MAM_DIFF_MAX_LINES": "[ -gt ] (decimal)", - "RECONCILE_POLL_INTERVAL": "sleep", + # sleep's fractional seconds are its meaning here (default 0.5): mam_int would refuse them + "SKS_QUIESCENT_INTERVAL": "sleep, fractional by default (0.5) - left raw", "DELEGATE_JOB_ACK_S": "create_session.sh -> delegate_wait_started, which runs mam_int on it", } @@ -407,7 +416,7 @@ def test_b5_no_env_default_inside_arithmetic(path): def test_b5_every_normalised_knob_uses_the_table_minimum(): - src = WRAPPER.read_text() + "\n" + LIB_SH.read_text() + src = "\n".join(f.read_text() for f in [WRAPPER, *sorted(SKILLS.glob("**/*.sh"))]) calls = re.findall(r"mam_int_knob ([A-Z_]+) (\S+) (\d+)\)", src) seen = {} for name, default, minimum in calls: @@ -445,6 +454,36 @@ def test_b5_every_integer_env_default_is_classified(): assert not stale, f"no longer in the code: {stale}" +LOOP_LOCK = SKILLS / "multi-agent-mux-loop" / "scripts" / "loop_lock.sh" +DIFF_COLLECT = SKILLS / "multi-agent-mux-loop" / "scripts" / "diff_collect.sh" + + +@pytest.mark.parametrize("value", ["abc", "0900", " 7 ", "-1", "1.5", "0", ""], ids=_id) +@pytest.mark.parametrize("script,knobs", [(LOOP_LOCK, ["MAM_LOCK_STEAL_RETRIES"]), + (DIFF_COLLECT, ["MAM_DIFF_MAX_BYTES", "MAM_DIFF_MAX_LINES"])], + ids=lambda v: v.name if isinstance(v, Path) else "") +def test_b5_scripts_sourced_without_lib_sh_read_their_knobs_as_decimal(tmp_path, script, knobs, value): + """F4b: these files are sourced alone (the runner, the tests): they bring + the integer rule with them, and "abc" no longer reaches `[ -le ]`/`[ -gt ]`.""" + probe = "; ".join(f'printf "%s=%s\\n" {k} "${k}"' for k in knobs) + res = subprocess.run(["bash", "-c", f'source "{script}"; {probe}'], capture_output=True, text=True, + env=_base_env(tmp_path, {k: value for k in knobs}), cwd=str(tmp_path), timeout=60) + assert res.returncode == 0, res.stderr + for k in knobs: + minimum, default = INT_KNOBS[k] + want = _expected(value, minimum) + assert f"{k}={default if want is None else want}" in res.stdout.splitlines(), (res.stdout, res.stderr) + assert (f"WARNING: {k}=" in res.stderr) is (want is None and value != ""), res.stderr + + +def test_b5_pane_quiescent_gives_up_on_empty_captures_with_a_bad_knob(tmp_path): + """F4b: SKS_EMPTY_GIVEUP=abc was a `[` error on every empty capture.""" + res = _bash('_pane_capture() { :; }; _pane_quiescent s 20 0; echo "rc=$?"', tmp_path, {"SKS_EMPTY_GIVEUP": "abc"}) + assert "rc=2" in res.stdout, (res.stdout, res.stderr) + assert "integer expression expected" not in res.stderr, res.stderr + assert "WARNING: SKS_EMPTY_GIVEUP=abc ignored" in res.stderr, res.stderr + + @pytest.mark.parametrize("value,rc", [("900", 0), ("0900", 0), ("0", 2)]) def test_b5_pane_quiescent_reads_the_tries_knob_as_decimal(tmp_path, value, rc): """The consuming function with the call site's normalisation (review R2):