perf(ci): let the readiness suites skip a wait none of them are about

scripts/remote.sh

@@ -1924,6 +1924,36 @@ _remote_sync_engine_status() {
   fi
 }
 
+# How many turns the readiness poll takes before giving up.
+#
+# THE SHIPPED VALUE IS 1600 AND THIS DOES NOT CHANGE IT. The number is lifted out
+# of the loop so a test can ask for it without running it, and so the regression
+# suites can reach the give-up path without paying for the full ceiling -- five
+# of them were spending 12 minutes of a 25-minute CI shard on a wait whose length
+# none of them are about.
+#
+# `AGMSG_TEST_SYNC_READY_TURNS` is a test seam. A value that is not a positive
+# integer falls back to the shipped number rather than to zero: a resolver that
+# quietly answered nothing would turn every one of those suites back into a full
+# ceiling run, which is the failure this exists to remove.
+_remote_sync_ready_turns() {
+  local want="${AGMSG_TEST_SYNC_READY_TURNS:-}"
+  # Unset, or not written in digits at all.
+  case "$want" in
+    ''|*[!0-9]*) printf '1600'; return ;;
+  esac
+  # AND ZERO IS NOT A CEILING. `0` is digits-only, so a guard that only rejects
+  # non-digits accepts it and the poll ends before it begins -- every suite that
+  # drives the give-up path would then reach its assertions without the engine
+  # having been waited on at all, and stay green while measuring nothing. That is
+  # the failure this fallback is described as preventing, and the description was
+  # true of `oops` and false of `0` until review caught it. `00` counts too.
+  case "$want" in
+    *[1-9]*) printf '%s' "$want" ;;
+    *)       printf '1600' ;;
+  esac
+}
+
 _remote_sync_engine_reap_owned() {
   local team="$1" owned_pid="$2" state pid signal attempts
   for signal in TERM KILL; do
@@ -2685,7 +2715,9 @@ cmd_sync_start() {
   # that is late or missing for ANY reason costs this caller its own wait and
   # not the rest of the machine.
   agmsg_lock_release
-  while [ "$i" -lt 1600 ]; do
+  local ready_turns
+  ready_turns="$(_remote_sync_ready_turns)"
+  while [ "$i" -lt "$ready_turns" ]; do
     IFS=$'\t' read -r engine_state ready_pid < <(_remote_sync_engine_status "$team")
     if [ "$engine_state" = "running" ] && [ "$ready_pid" = "$started_pid" ] &&
        tail -c "+$log_offset" "$logfile" 2>/dev/null |

tests/test_remote_engine_start_refusal.bats

@@ -113,7 +113,7 @@ skip_if_root() {
   printf '%s\n' 2147483647 > "$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
   chmod a-w "$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
 
-  run bash "$SCRIPTS/remote.sh" sync start testteam
+  run env AGMSG_TEST_SYNC_READY_TURNS=40 bash "$SCRIPTS/remote.sh" sync start testteam
   [ "$status" -ne 0 ]
 
   # Captured before the next `run`, which overwrites $output.
@@ -133,7 +133,7 @@ skip_if_root() {
   # command fails on a team literally named "'testteam'" -- measured, that is
   # what the first version of this test did. A printed route has to be run the
   # way it is meant to be run.
-  run bash -c "bash '$SCRIPTS/remote.sh' $args"
+  run bash -c "AGMSG_TEST_SYNC_READY_TURNS=40 bash '$SCRIPTS/remote.sh' $args"
   # "not refused" is not enough: a remedy that no longer parses is answered with
   # a usage line, which is also not a refusal. Measured -- changing only the
   # printed verb (start -> begin) left this test green until the two assertions
@@ -159,7 +159,7 @@ skip_if_root() {
 @test "sync start: a writable run dir still starts an engine (#730)" {
   # The control. Without it, every assertion above is satisfied by a
   # `sync start` that refuses unconditionally.
-  run bash "$SCRIPTS/remote.sh" sync start testteam
+  run env AGMSG_TEST_SYNC_READY_TURNS=40 bash "$SCRIPTS/remote.sh" sync start testteam
   # The engine is real here and will fail to reach https://remote.example, so
   # this does not assert success -- only that the refusal above is not what
   # happened, and that the pidfile path was reachable.
@@ -192,7 +192,7 @@ skip_if_root() {
   local pidfile="$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
   local starter i=0 j=0 freed=0
 
-  bash "$SCRIPTS/remote.sh" sync start testteam >/dev/null 2>&1 &
+  AGMSG_TEST_SYNC_READY_TURNS=40 bash "$SCRIPTS/remote.sh" sync start testteam >/dev/null 2>&1 &
   starter=$!
 
   # The engine existing is what says the START is over and the WAIT has begun.
@@ -315,7 +315,7 @@ skip_if_root() {
   local cycles="$TEST_SKILL_DIR/run/remote-sync.testteam.cycles.json"
   local starter engine foreign i=0
 
-  bash "$SCRIPTS/remote.sh" sync start testteam >/dev/null 2>&1 &
+  AGMSG_TEST_SYNC_READY_TURNS=40 bash "$SCRIPTS/remote.sh" sync start testteam >/dev/null 2>&1 &
   starter=$!
   while [ ! -f "$pidfile" ] && [ "$i" -lt 400 ]; do i=$((i + 1)); sleep 0.05; done
   [ -f "$pidfile" ]
@@ -364,7 +364,7 @@ skip_if_root() {
   local cycles="$TEST_SKILL_DIR/run/remote-sync.testteam.cycles.json"
   local starter engine i=0 err="$TEST_SKILL_DIR/retake.err"
 
-  bash "$SCRIPTS/remote.sh" sync start testteam >"$err" 2>&1 &
+  AGMSG_TEST_SYNC_READY_TURNS=40 bash "$SCRIPTS/remote.sh" sync start testteam >"$err" 2>&1 &
   starter=$!
   while [ ! -f "$pidfile" ] && [ "$i" -lt 400 ]; do i=$((i + 1)); sleep 0.05; done
   [ -f "$pidfile" ]

tests/test_remote_status_liveness.bats

@@ -715,7 +715,7 @@ write_windows_spelling_fixtures() {
 
   run env PATH="$fake_bin:$PATH" AGMSG_NODE="$fake_node" \
     AGMSG_TEST_CHILD_PID_FILE="$child_pid_file" \
-    bash "$SCRIPTS/remote.sh" sync start testteam
+    AGMSG_TEST_SYNC_READY_TURNS=40 bash "$SCRIPTS/remote.sh" sync start testteam
   [ "$status" -ne 0 ]
   [[ "$output" == *"did not become ready"* ]]
   [ ! -e "$TEST_SKILL_DIR/run/remote-sync.testteam.pid" ]
@@ -771,7 +771,7 @@ write_unownable_ps_fixture() {
 
   run env PATH="$fake_bin:$PATH" AGMSG_NODE="$fake_node" \
     AGMSG_TEST_CHILD_PID_FILE="$child_pid_file" \
-    bash "$SCRIPTS/remote.sh" sync start testteam
+    AGMSG_TEST_SYNC_READY_TURNS=40 bash "$SCRIPTS/remote.sh" sync start testteam
   [ "$status" -ne 0 ]
 
   child_pid="$(cat "$child_pid_file")"

tests/test_sync_readiness_budget.bats

@@ -0,0 +1,95 @@
+#!/usr/bin/env bats
+
+# The readiness poll's ceiling, and the seam that lets the suites skip it (#831).
+#
+# WHY THE SEAM EXISTS. Reaching `sync start`'s give-up path means waiting out the
+# poll, and five regression cases were doing that at the shipped 1600 turns:
+# 12 minutes 24 seconds of a macOS CI shard whose job cap is 25, measured from
+# the gaps between consecutive `ok` lines on a run that was cancelled for
+# exceeding it. None of those five is about the length of the wait; each needs
+# only a state where readiness never arrives, which 40 turns produces just as
+# well.
+#
+# WHAT MUST NOT MOVE is the shipped number, and that is what this file pins --
+# separately from the suites that use the seam, so that lowering the default
+# reddens something even if every one of those suites is passing.
+
+load test_helper
+
+setup() { setup_test_env; export SKILL_DIR="$TEST_SKILL_DIR"; }
+teardown() { teardown_test_env; }
+
+ask_turns() {
+  cat > "$TEST_SKILL_DIR/turns.sh" <<'EOF_TURNS'
+#!/usr/bin/env bash
+. "$SCRIPTS/remote.sh"
+printf 'turns=%s\n' "$(_remote_sync_ready_turns)"
+EOF_TURNS
+}
+
+@test "the shipped readiness ceiling is 1600 turns (#831)" {
+  # The number itself, asked for rather than run. Bound by letting the poll reach
+  # the ceiling it cost 52 seconds in one case; this costs milliseconds and holds
+  # the same fact.
+  ask_turns
+  run env SCRIPTS="$SCRIPTS" bash "$TEST_SKILL_DIR/turns.sh"
+  [ "$status" -eq 0 ]
+  grep -qF 'turns=1600' <<<"$output"
+}
+
+@test "the seam is honoured when it is set (#831)" {
+  # THE NEGATIVE CONTROL FOR THE CASE ABOVE. Without it, a resolver that ignored
+  # the variable and always answered 1600 would satisfy the shipped-default test
+  # while silently putting every suite that sets the seam back on the full
+  # ceiling -- which is the CI failure this change exists to remove, restored
+  # invisibly.
+  ask_turns
+  run env SCRIPTS="$SCRIPTS" AGMSG_TEST_SYNC_READY_TURNS=40 bash "$TEST_SKILL_DIR/turns.sh"
+  grep -qF 'turns=40' <<<"$output"
+}
+
+@test "a value that is not a count falls back to shipped, not to zero (#831)" {
+  # A ceiling of zero would end the poll before it began: every `sync start`
+  # would report failure instantly, and the suites would still be green because
+  # they are asserting on the give-up path. Asserted for three shapes, because
+  # they reach the fallback down different comparisons.
+  ask_turns
+  for bad in oops -5 12x; do
+    run env SCRIPTS="$SCRIPTS" AGMSG_TEST_SYNC_READY_TURNS="$bad" bash "$TEST_SKILL_DIR/turns.sh"
+    grep -qF 'turns=1600' <<<"$output"
+  done
+}
+
+@test "an explicit zero is not a ceiling: it falls back to shipped (#831)" {
+  # THE ONE THE OTHER FALLBACK CASE DID NOT COVER. `0` is written in digits, so a
+  # guard that rejects only non-digits accepts it -- and a ceiling of zero ends
+  # the poll before it runs once. Every suite that drives the give-up path would
+  # then reach its assertions without the engine ever being waited on, and stay
+  # green while measuring nothing.
+  #
+  # The prose said "not a positive integer falls back". That was true of `oops`
+  # and false of `0`, and the cases tested `oops`, `-5`, `12x` -- none of them
+  # the digits-only shape the sentence was actually about (raised in review).
+  ask_turns
+  for zero in 0 00 000; do
+    run env SCRIPTS="$SCRIPTS" AGMSG_TEST_SYNC_READY_TURNS="$zero" bash "$TEST_SKILL_DIR/turns.sh"
+    grep -qF 'turns=1600' <<<"$output"
+  done
+}
+
+@test "a positive count with a leading zero is still that count (#831)" {
+  # THE NEGATIVE CONTROL FOR THE CASE ABOVE. "reject anything containing a zero"
+  # would satisfy it and quietly send `40` -> 1600, putting every seamed suite
+  # back on the full ceiling -- the exact regression this file exists to catch.
+  ask_turns
+  run env SCRIPTS="$SCRIPTS" AGMSG_TEST_SYNC_READY_TURNS=040 bash "$TEST_SKILL_DIR/turns.sh"
+  grep -qF 'turns=040' <<<"$output"
+}
+
+@test "an unset seam and an empty seam agree (#831)" {
+  # `export FOO=` is not the same shape as never exporting it, and a case split
+  # on `-n`/`-z` can tell them apart. Both must mean shipped.
+  ask_turns
+  run env SCRIPTS="$SCRIPTS" AGMSG_TEST_SYNC_READY_TURNS= bash "$TEST_SKILL_DIR/turns.sh"
+  grep -qF 'turns=1600' <<<"$output"
+}