fix(followups): F4b - bash integer knobs that reach [ -ge/-le/-gt ] or sleep go through mam_int_knob

A non-numeric MAM_READY_EMPTY_GIVEUP, SKS_EMPTY_GIVEUP, SKS_IDLE_RETRIES,
SKS_IDLE_RETRY_BACKOFF_S, MAM_LOCK_STEAL_RETRIES, MAM_DIFF_MAX_BYTES,
MAM_DIFF_MAX_LINES or RECONCILE_POLL_INTERVAL was a `[` error or a sleep
error at the point of use (PR #43 review claude R3). They now go through
mam_int_knob: decimal, a warning and the default for a bad value. Minimums
are in test_issue26_ints.py INT_KNOBS: 1 for the give-up counts (0 acts as
1), the diff caps (0 truncates every diff) and the poll interval (0 is a
busy loop); 0 for the retry counts (one attempt) and the retry backoff.

SKS_QUIESCENT_INTERVAL stays raw: its default is 0.5 s and sleep's
fraction is its meaning; NOT_ARITHMETIC says so.

loop_lock.sh and diff_collect.sh also run without lib.sh (the Python runner
and the tests source them alone), so mam_int / mam_int_knob move verbatim
to lib_int.sh, which lib.sh sources and those two source when the function
is missing. remove.sh's no-manifest fallback list names the new file
(install copies every file under .agents/skills). test_issue21_loop_b l15,
which builds a control root holding only diff_collect.sh, copies it too.

Tests: the table guard now reads every shell file; scripts sourced alone
read their knobs as decimal; _pane_quiescent with SKS_EMPTY_GIVEUP=abc
gives up with a warning and no `[` error (14 of the new cases fail at
0051fd6).

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-02 08:23:31 +09:00
1 parent 8b046390be
commit fb1b93c665
8 files changed
+104 -47

No files matched your search

+8 -37
View File
@@ -2335,39 +2335,10 @@ sys.exit(1)
' 2>/dev/null ' 2>/dev/null
} }
# mam_int <value> <min> # mam_int / mam_int_knob (Issue #26) live in lib_int.sh, shared with the
# # scripts that run without lib.sh.
# Issue #26: the one integer rule for knobs that reach bash arithmetic (the # shellcheck source=lib_int.sh
# Python twin is lib_py/loop/runner.py::_positive_int). Surrounding blanks are source "$(dirname "${BASH_SOURCE[0]}")/lib_int.sh"
# 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 <ENV_NAME> <default> <min>
#
# The value of $ENV_NAME through mam_int; unset or empty gives <default>, an
# invalid value gives <default> 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 <job_id> [ack_timeout_s] [dispatch_token] # delegate_wait_started <job_id> [ack_timeout_s] [dispatch_token]
# #
@@ -2443,7 +2414,7 @@ wait_for_tui_ready() {
fi fi
local empty_streak=0 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" local modal_pat="$_MAM_MODAL_TOKENS"
[ -n "$modal_tokens" ] && modal_pat="${modal_pat}|${modal_tokens}" [ -n "$modal_tokens" ] && modal_pat="${modal_pat}|${modal_tokens}"
@@ -2639,7 +2610,7 @@ _wait_session_gone() {
_pane_quiescent() { _pane_quiescent() {
local sess="$1" tries="${2:-20}" interval="${3:-0.5}" prev="__none__" cur i local sess="$1" tries="${2:-20}" interval="${3:-0.5}" prev="__none__" cur i
local saw_output=0 empty_streak=0 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 for ((i = 0; i < tries; i++)); do
cur=$(_pane_capture "$sess") cur=$(_pane_capture "$sess")
if [ -z "$cur" ]; then if [ -z "$cur" ]; then
@@ -2709,8 +2680,8 @@ except Exception:
return 0 return 0
fi fi
local max_retries="${SKS_IDLE_RETRIES:-2}" local max_retries="$(mam_int_knob SKS_IDLE_RETRIES 2 0)"
local backoff_s="${SKS_IDLE_RETRY_BACKOFF_S:-10}" local backoff_s="$(mam_int_knob SKS_IDLE_RETRY_BACKOFF_S 10 0)"
local try=0 local try=0
while [ "$try" -le "$max_retries" ]; do while [ "$try" -le "$max_retries" ]; do
local watcher_out="" watcher_rc=0 local watcher_out="" watcher_rc=0
+39
View File
@@ -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 <value> <min>
#
# 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 <ENV_NAME> <default> <min>
#
# The value of $ENV_NAME through mam_int; unset or empty gives <default>, an
# invalid value gives <default> 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"
}
@@ -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 # 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: # 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). # only a diff over these limits is truncated (and marked).
MAM_DIFF_MAX_BYTES="${MAM_DIFF_MAX_BYTES:-200000}" # The runner sources this file without lib.sh; the integer rule is lib_int.sh's.
MAM_DIFF_MAX_LINES="${MAM_DIFF_MAX_LINES:-4000}" # 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() { _mam_nested_dir_diff() {
local d="$1" outer_top="$2" inner_top="" local d="$1" outer_top="$2" inner_top=""
@@ -2,7 +2,10 @@
# loop_lock.sh — Atomic race-free loop lock acquisition and marker ownership (O-2) # 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. # 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() { mam_lstart() {
local pid="${1:-$$}" local pid="${1:-$$}"
@@ -38,7 +38,7 @@ SUBSCRIBE=0
# (raised from 600s to align with job timeout defaults); idle 0 = never idle-out. # (raised from 600s to align with job timeout defaults); idle 0 = never idle-out.
SUB_TIMEOUT=0 SUB_TIMEOUT=0
SUB_IDLE_TIMEOUT=3600 SUB_IDLE_TIMEOUT=3600
POLL_INTERVAL="${RECONCILE_POLL_INTERVAL:-15}" POLL_INTERVAL="$(mam_int_knob RECONCILE_POLL_INTERVAL 15 1)"
while [ $# -gt 0 ]; do while [ $# -gt 0 ]; do
case "$1" in case "$1" in
+1
View File
@@ -82,6 +82,7 @@ if [ -f "$MANIFEST_FILE" ]; then
else else
fallback_assets=( fallback_assets=(
".agents/skills/lib.sh" ".agents/skills/lib.sh"
".agents/skills/lib_int.sh"
".agents/skills/lib_py" ".agents/skills/lib_py"
".agents/skills/multi-agent-mux-create" ".agents/skills/multi-agent-mux-create"
".agents/skills/multi-agent-mux-delegate-job" ".agents/skills/multi-agent-mux-delegate-job"
+1
View File
@@ -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 = main / ".agents" / "skills" / "multi-agent-mux-loop" / "scripts"
dst.mkdir(parents=True) dst.mkdir(parents=True)
(dst / "diff_collect.sh").write_text(DIFF_SH.read_text()) (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") monkeypatch.setenv("MAM_DIFF_MAX_LINES", "1")
_state(monkeypatch, LOCAL_ROWS + [REMOTE_ROW]) _state(monkeypatch, LOCAL_ROWS + [REMOTE_ROW])
argv = ["--worktree", str(wt), "--base", before, "--review-only", "--remote-review", "diff-only"] argv = ["--worktree", str(wt), "--base", before, "--review-only", "--remote-review", "diff-only"]
+45 -6
View File
@@ -43,6 +43,18 @@ INT_KNOBS = {
"SKS_DELIVERY_GRACE_S": (0, "15"), "SKS_DELIVERY_GRACE_S": (0, "15"),
# _pane_quiescent's `for ((i = 0; i < tries; i++))` (review R2); 0 keeps "headless" (rc 2) # _pane_quiescent's `for ((i = 0; i < tries; i++))` (review R2); 0 keeps "headless" (rc 2)
"SKS_QUIESCENT_TRIES": (0, "20"), "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's ack_s (MAM_ONBOARD_ACK_S / DELEGATE_JOB_ACK_S via create_session.sh)
"delegate_wait_started:ack_s": (0, "180"), "delegate_wait_started:ack_s": (0, "180"),
# wrapper flags # 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_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", "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()", "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)", # sleep's fractional seconds are its meaning here (default 0.5): mam_int would refuse them
"SKS_IDLE_RETRIES": "[ -le ] (decimal)", "SKS_IDLE_RETRY_BACKOFF_S": "sleep", "SKS_QUIESCENT_INTERVAL": "sleep, fractional by default (0.5) - left raw",
"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",
"DELEGATE_JOB_ACK_S": "create_session.sh -> delegate_wait_started, which runs mam_int on it", "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(): 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) calls = re.findall(r"mam_int_knob ([A-Z_]+) (\S+) (\d+)\)", src)
seen = {} seen = {}
for name, default, minimum in calls: 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}" 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)]) @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): 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): """The consuming function with the call site's normalisation (review R2):