fix: prevent duplicate portable serial shard assignments

bin/fm-test-run.sh

@@ -621,14 +621,22 @@ standalone
 EOF
 }
 
-family_is_concurrent_safe() {
-  local want=$1 line
-  while IFS= read -r line; do
-    [ "$line" = "$want" ] && return 0
-  done < <(list_concurrent_safe_families)
+# Exact whole-line membership of <want> in the newline-separated <list>, with no
+# process substitution. macOS /bin/bash 3.2 loses lines from later reads once one
+# long-lived shell has run enough per-item "while read ... done < <(cmd)" loops
+# (see list_portable_serial), so per-item membership tests must not use them.
+list_has_line() {
+  local want=$1 list=$2
+  case $'\n'"$list"$'\n' in
+    *$'\n'"$want"$'\n'*) return 0 ;;
+  esac
   return 1
 }
 
+family_is_concurrent_safe() {
+  list_has_line "$1" "$(list_concurrent_safe_families)"
+}
+
 concurrent_safe_family_jobs_max() {
   case "$1" in
     watcher-wake-lock|pure-contract-unit|pr-forge) printf '4\n' ;;
@@ -640,22 +648,15 @@ concurrent_safe_family_jobs_max() {
 # A script may run under --jobs when it is individually proven isolated or is
 # an exact repository member of a family carrying a recorded concurrent proof.
 script_allows_concurrency() {
-  local s=$1 family repo_script
+  local s=$1 family
   is_proven_isolated_script "$s" && return 0
   family=$(family_for_basename "$(basename "$s")")
   family_is_concurrent_safe "$family" || return 1
-  while IFS= read -r repo_script; do
-    [ "$repo_script" = "$s" ] && return 0
-  done < <(all_repo_tests)
-  return 1
+  list_has_line "$s" "$(all_repo_tests)"
 }
 
 is_proven_isolated_script() {
-  local want=$1 line
-  while IFS= read -r line; do
-    [ "$line" = "$want" ] && return 0
-  done < <(list_proven_isolated)
-  return 1
+  list_has_line "$1" "$(list_proven_isolated)"
 }
 
 # The portable serial remainder: every tests/*.test.sh that is neither
@@ -664,19 +665,21 @@ is_proven_isolated_script() {
 # and other unproven work stays here. Derived rather than enumerated so a newly added test
 # lands here by default instead of falling out of every lane.
 list_portable_serial() {
-  local s base fam
+  local s base fam proven_set all
+  proven_set=$(list_proven_isolated)
+  all=$(all_repo_tests) || return 1
   while IFS= read -r s; do
     [ -n "$s" ] || continue
-    base=$(basename "$s")
+    base=${s##*/}
     fam=$(family_for_basename "$base")
     if [ "$fam" = "real-herdr-gated" ]; then
       continue
     fi
-    if is_proven_isolated_script "$s"; then
+    if list_has_line "$s" "$proven_set"; then
       continue
     fi
-    printf '%s\n' "$s"
-  done < <(all_repo_tests)
+    printf '%s\n' "$s" || return 1
+  done <<<"$all"
 }
 
 # Measured portable-serial script durations in milliseconds, from the CI timing
@@ -897,49 +900,40 @@ portable_parallel_weight_for() {
 }
 
 portable_serial_weight_for() {
-  local want=$1 path ms
-  while read -r path ms; do
-    if [ "$path" = "$want" ]; then
-      printf '%s\n' "$ms"
-      return 0
-    fi
-  done < <(portable_serial_weight_hints)
-  printf '%s\n' "$PORTABLE_SERIAL_DEFAULT_WEIGHT_MS"
+  local want=$1
+  portable_serial_weight_hints | awk -v want="$want" -v def="$PORTABLE_SERIAL_DEFAULT_WEIGHT_MS" '
+    $1 == want { print $2; found = 1; exit }
+    END { if (!found) print def }
+  '
 }
 
 # Longest-processing-time assignment of the serial remainder to
 # PORTABLE_SERIAL_SHARDS bins, printing "<shard>\t<script>" for every script.
 # Deterministic: candidates are ordered by hint descending then path, and ties
 # between equally loaded bins always take the lowest bin index.
-portable_serial_assignments() {
-  local ms script i best best_load
-  local -a loads=()
-  i=1
-  while [ "$i" -le "$PORTABLE_SERIAL_SHARDS" ]; do
-    loads[i]=0
-    i=$((i + 1))
-  done
-  while IFS=$'\t' read -r ms script; do
-    [ -n "$script" ] || continue
-    best=1
-    best_load=${loads[1]}
-    i=2
-    while [ "$i" -le "$PORTABLE_SERIAL_SHARDS" ]; do
-      if [ "${loads[i]}" -lt "$best_load" ]; then
-        best_load=${loads[i]}
-        best=$i
-      fi
-      i=$((i + 1))
-    done
-    loads[best]=$((best_load + ms))
-    printf '%s\t%s\n' "$best" "$script"
-  done < <(
-    while IFS= read -r script; do
-      [ -n "$script" ] || continue
-      printf '%s\t%s\n' "$(portable_serial_weight_for "$script")" "$script"
-    done < <(list_portable_serial) | LC_ALL=C sort -t$'\t' -k1,1nr -k2,2
-  )
-}
+portable_serial_assignments() (
+  # Keep pipeline failures visible without changing the caller's shell options.
+  # Bash 3.2 builtin writes to an asynchronous pipe can fail with EINTR; let awk
+  # own assignment output and require the complete producer to succeed.
+  set -o pipefail
+  { portable_serial_weight_hints || return 1; printf '%s\n' '--' || return 1; list_portable_serial; } \
+    | awk -v def="$PORTABLE_SERIAL_DEFAULT_WEIGHT_MS" '
+        $0 == "--" { scripts = 1; next }
+        !scripts { if (NF && !($1 in w)) w[$1] = $2; next }
+        NF { printf "%s\t%s\n", (($0 in w) ? w[$0] : def), $0 }
+      ' \
+    | LC_ALL=C sort -t$'\t' -k1,1nr -k2,2 \
+    | awk -F '\t' -v shards="$PORTABLE_SERIAL_SHARDS" '
+        BEGIN { for (i = 1; i <= shards; i++) loads[i] = 0 }
+        {
+          best = 1
+          for (i = 2; i <= shards; i++)
+            if (loads[i] < loads[best]) best = i
+          loads[best] += $1
+          printf "%s\t%s\n", best, $2
+        }
+      '
+)
 
 # Parse "<k>of<n>" from a portable-serial shard lane and echo <k>, refusing when
 # <n> disagrees with this script's configured count so a CI matrix built for a
@@ -977,7 +971,7 @@ select_proven_isolated() {
 }
 
 select_lane() {
-  local want=$1 s shard idx found=0
+  local want=$1 s shard idx assignments found=0
   case "$want" in
     portable-parallel-1)
       while IFS= read -r s; do
@@ -1003,13 +997,15 @@ select_lane() {
     portable-serial-*)
       # One separate-runner shard of the same remainder, still serial in itself.
       shard=$(portable_serial_shard_index "$want")
+      assignments=$(portable_serial_assignments) \
+        || die "could not generate complete portable serial shard assignments"
       while IFS=$'\t' read -r idx s; do
         [ -n "$s" ] || continue
         if [ "$idx" = "$shard" ]; then
           add_script "$s"
           found=1
         fi
-      done < <(portable_serial_assignments)
+      done <<<"$assignments"
       ;;
     real-herdr-gated)
       select_family real-herdr-gated
@@ -1242,7 +1238,7 @@ all_repo_tests() {
   # shellcheck disable=SC2035
   for f in tests/*.test.sh; do
     [ -f "$f" ] || continue
-    printf '%s\n' "$f"
+    printf '%s\n' "$f" || return 1
   done | LC_ALL=C sort
 }
 

docs/fm-test-portable-shards.md

@@ -65,6 +65,8 @@ All three shard summaries report `failed=0`, and each measured row reports `exit
 An unfinished or failed invocation is not a healthy duration sample.
 A script with no hint gets the conservative `PORTABLE_SERIAL_DEFAULT_WEIGHT_MS` default.
 Hints only affect balance: the coverage guard keeps the partition complete and disjoint whatever they say, so a stale hint costs a slower shard rather than lost coverage.
+Shard selection checks the complete assignment generator before consuming its output; a generation failure refuses the lane instead of returning a successful partial list.
+Runner regressions check standalone shard unions with five, six, and twenty added scripts, and require a failed producer to refuse without publishing a partial list.
 Balance is still worth keeping current, because enough unmeasured scripts let one shard carry more than twice another shard's real work and reach the job cap while another runner sits idle.
 That is not hypothetical: by 2026-09-01 the lane had grown from 116 to 139 scripts and from ~42 to ~63 minutes, 17 scripts were still unmeasured, and several hints were low by 2-5x, so shard 3 of 4 ran 17-20 minutes against its 20-minute cap while shard 1 ran 11.5 minutes and run [33574154856](https://github.com/kunchenguid/firstmate/actions/runs/33574154856) timed out seconds after a passing test.
 `bin/fm-test-run.sh --check-coverage` now reports the unmeasured share as `serial_unhinted=` and refuses past `PORTABLE_SERIAL_MAX_UNHINTED_PERCENT`, so hint drift fails the coverage guard instead of silently pushing one shard into its job cap.

tests/fm-test-run.test.sh

@@ -1418,6 +1418,174 @@ test_portable_serial_hint_coverage_is_reported_and_bounded() {
   pass "coverage guard reports and bounds the unmeasured portable serial share"
 }
 
+# The serial shard lists are computed again for every shard and for the whole
+# lane inside one coverage-guard process. macOS /bin/bash 3.2 loses lines from
+# later reads once one long-lived shell has run enough per-script process
+# substitutions, so a lane past a size threshold used to land the same script in
+# two shards inside the guard while every standalone --list was correct. Build a
+# fixture runner with extra scripts and require the guard and the shard lists to
+# stay a disjoint cover of the serial lane whatever the lane size is.
+#
+# The corruption depends on the process environment's memory layout, so the
+# guard runs under a scrubbed environment here: the exported variables the test
+# harness adds are enough to hide it, and a regression test that cannot fail on
+# the old code proves nothing.
+shard_fixture_init() {
+  local dir=$1 name
+  mkdir -p "$dir/bin" "$dir/tests"
+  cp "$RUNNER" "$dir/bin/fm-test-run.sh"
+  chmod +x "$dir/bin/fm-test-run.sh"
+  for name in "$ROOT"/tests/*.test.sh; do
+    : >"$dir/tests/${name##*/}"
+  done
+}
+
+# Grow the fixture's extra scripts to <count> (never shrinks).
+shard_fixture_grow() {
+  local dir=$1 count=$2 i=1
+  while [ "$i" -le "$count" ]; do
+    : >"$dir/tests/zz-shard-dummy-$i.test.sh"
+    i=$((i + 1))
+  done
+}
+
+shard_fixture_guard() {
+  env -i PATH="$PATH" HOME="${HOME:-/}" TMPDIR="${TMPDIR:-/tmp}" \
+    "$1/bin/fm-test-run.sh" --check-coverage 2>&1
+}
+
+assert_serial_shards_are_a_disjoint_cover() {
+  local runner=$1 label=$2 shards shard serial union dups listed
+  shards=$(env -i PATH="$PATH" HOME="${HOME:-/}" TMPDIR="${TMPDIR:-/tmp}" \
+    "$runner" --list-lanes | grep -c '^portable-serial-[0-9]*of[0-9]*$')
+  serial=$(env -i PATH="$PATH" HOME="${HOME:-/}" TMPDIR="${TMPDIR:-/tmp}" \
+    "$runner" --list --lane portable-serial | LC_ALL=C sort)
+  union=""
+  shard=1
+  while [ "$shard" -le "$shards" ]; do
+    listed=$(env -i PATH="$PATH" HOME="${HOME:-/}" TMPDIR="${TMPDIR:-/tmp}" \
+      "$runner" --list --lane "portable-serial-${shard}of${shards}") \
+      || fail "$label: serial shard $shard listing failed"
+    union=$(printf '%s\n%s' "$union" "$listed")
+    shard=$((shard + 1))
+  done
+  union=$(printf '%s\n' "$union" | grep -v '^$' || true)
+  dups=$(printf '%s\n' "$union" | LC_ALL=C sort | uniq -d || true)
+  [ -z "$dups" ] || fail "$label: a script runs in two serial shards: $dups"
+  [ "$(printf '%s\n' "$union" | LC_ALL=C sort)" = "$serial" ] \
+    || fail "$label: serial shards must exactly cover the serial lane"
+}
+
+test_serial_shard_guard_holds_at_every_lane_size() {
+  local tmp dir count out rc
+  tmp=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run-shard-sizes.XXXXXX")
+  dir="$tmp/fixture"
+  shard_fixture_init "$dir"
+  # 5 and 6 extra scripts are the lane sizes that failed in the coverage guard;
+  # 20 proves the result does not depend on one particular size.
+  for count in 5 6 20; do
+    shard_fixture_grow "$dir" "$count"
+    set +e
+    out=$(shard_fixture_guard "$dir")
+    rc=$?
+    set -e
+    assert_not_contains "$out" "serial shards share scripts" \
+      "$count extra scripts: the guard put one script in two serial shards"
+    assert_not_contains "$out" "serial shards must equal" \
+      "$count extra scripts: serial shards no longer cover the serial lane"
+    # The only other refusal an unmeasured new script may cause is the bounded
+    # unmeasured share, which is the guard working as intended.
+    if [ "$rc" -ne 0 ]; then
+      assert_contains "$out" "have no measured duration hint" \
+        "$count extra scripts: the guard refused for an unexpected reason"
+    fi
+    assert_serial_shards_are_a_disjoint_cover "$dir/bin/fm-test-run.sh" "$count extra scripts"
+  done
+  rm -rf "$tmp"
+  pass "serial shards stay a disjoint cover of the serial lane with 5, 6, and 20 extra scripts"
+}
+
+test_serial_shard_generation_failure_is_not_a_partial_success() {
+  local tmp dir real_sort rc
+  tmp=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run-shard-failure.XXXXXX")
+  dir="$tmp/fixture"
+  shard_fixture_init "$dir"
+  shard_fixture_grow "$dir" 5
+  real_sort=$(command -v sort)
+  mkdir -p "$tmp/fakebin"
+  cat >"$tmp/fakebin/sort" <<'SH'
+#!/usr/bin/env bash
+for arg in "$@"; do
+  if [ "$arg" = "-k1,1nr" ]; then
+    "$REAL_SORT" "$@" | awk 'NR == 1'
+    exit 1
+  fi
+done
+exec "$REAL_SORT" "$@"
+SH
+  chmod +x "$tmp/fakebin/sort"
+  set +e
+  env -i PATH="$tmp/fakebin:$PATH" REAL_SORT="$real_sort" \
+    HOME="${HOME:-/}" TMPDIR="${TMPDIR:-/tmp}" \
+    "$dir/bin/fm-test-run.sh" --list --lane portable-serial-1of9 \
+    >"$tmp/out" 2>"$tmp/err"
+  rc=$?
+  set -e
+  [ "$rc" -ne 0 ] || fail "failed shard generation returned successful partial coverage"
+  [ ! -s "$tmp/out" ] || fail "failed shard generation published a partial script list"
+  rm -rf "$tmp"
+  pass "serial shard generation failures refuse rather than publish partial coverage"
+}
+
+test_coverage_guard_passes_with_extra_scripts() {
+  local tmp dir out unmeasured script
+  tmp=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run-guard-pass.XXXXXX")
+  dir="$tmp/fixture"
+  shard_fixture_init "$dir"
+  shard_fixture_grow "$dir" 5
+  # Drop the scripts that have no measured duration hint so the five new ones
+  # stay inside the guard's unmeasured-share bound; the guard then has nothing
+  # left to refuse except a shard conflict.
+  set +e
+  out=$(shard_fixture_guard "$dir")
+  set -e
+  unmeasured=$(printf '%s\n' "$out" | grep '^tests/.*\.test\.sh$' | grep -v 'zz-shard-dummy' || true)
+  for script in $unmeasured; do
+    rm -f "$dir/$script"
+  done
+  out=$(shard_fixture_guard "$dir") \
+    || fail "coverage guard refused a lane with five extra scripts: $out"
+  assert_contains "$out" "FM_TEST_COVERAGE ok" "coverage guard success marker with five extra scripts"
+  rm -rf "$tmp"
+  pass "coverage guard passes with five extra scripts and no script in two shards"
+}
+
+test_coverage_guard_still_refuses_a_true_duplicate() {
+  local tmp dir out rc dup
+  tmp=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run-guard-dup.XXXXXX")
+  dir="$tmp/fixture"
+  shard_fixture_init "$dir"
+  # Put one script in both portable parallel shards, which is a real duplicate
+  # the guard must keep refusing whatever the shard-assignment code does.
+  dup=$("$dir/bin/fm-test-run.sh" --list --lane portable-parallel-1 | head -n 1)
+  awk -v dup="$dup" '
+    /^list_portable_parallel_2\(\) \{/ { inside = 1 }
+    { print }
+    inside && /^  cat <<.EOF.$/ { print dup; inside = 0 }
+  ' "$dir/bin/fm-test-run.sh" >"$dir/bin/fm-test-run.sh.new"
+  mv "$dir/bin/fm-test-run.sh.new" "$dir/bin/fm-test-run.sh"
+  chmod +x "$dir/bin/fm-test-run.sh"
+  set +e
+  out=$(shard_fixture_guard "$dir")
+  rc=$?
+  set -e
+  [ "$rc" -ne 0 ] || fail "coverage guard accepted a script that sits in two shards: $out"
+  assert_contains "$out" "shards share scripts" "duplicate refusal names the shared scripts"
+  assert_contains "$out" "$dup" "duplicate refusal names the duplicated script"
+  rm -rf "$tmp"
+  pass "coverage guard still refuses a script that sits in two shards"
+}
+
 test_portable_serial_shard_lane_refusals() {
   local tmp count rc other
   tmp=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run-shard-lane.XXXXXX")
@@ -2044,6 +2212,10 @@ test_portable_shard_union_and_coverage_guard
 test_portable_parallel_lanes_stay_duration_balanced
 test_portable_serial_shards_partition_the_serial_lane
 test_portable_serial_hint_coverage_is_reported_and_bounded
+test_serial_shard_guard_holds_at_every_lane_size
+test_serial_shard_generation_failure_is_not_a_partial_success
+test_coverage_guard_passes_with_extra_scripts
+test_coverage_guard_still_refuses_a_true_duplicate
 test_portable_serial_shard_lane_refusals
 test_jobs_requires_proven_isolated
 test_jobs_admits_a_concurrent_safe_family