refactor(bin): collapse fm-pr-check-migrate legacy classifier into one function

bin/fm-pr-check-migrate.sh

@@ -76,18 +76,35 @@ scan_marker_content_valid() {
   [ "$value" = "$SCAN_MARKER_VALUE" ]
 }
 
+# Classify one watcher check file by today's evaluation order: a valid current
+# X-mode shim, then a registered custom check, then a canonical PR poll, else a
+# legacy check to migrate. Prints exactly one of x-shim|custom|poll|legacy.
+classify_check() {
+  local check=$1 id
+  if [ "$(basename "$check")" = x-watch.check.sh ] \
+    && fmx_poll_shim_valid "$check" "$FM_HOME" "$FM_ROOT"; then
+    printf 'x-shim\n'
+    return 0
+  fi
+  id=$(basename "$check" .check.sh)
+  if fm_custom_check_registered "$STATE" "$id"; then
+    printf 'custom\n'
+    return 0
+  fi
+  if fm_pr_poll_artifacts_valid "$STATE" "$id" "$TEMPLATE"; then
+    printf 'poll\n'
+    return 0
+  fi
+  printf 'legacy\n'
+}
+
 current_checks_authenticated() {
-  local check id
+  local check
   for check in "$STATE"/*.check.sh; do
     [ -e "$check" ] || [ -L "$check" ] || continue
-    if [ "$(basename "$check")" = x-watch.check.sh ] \
-      && fmx_poll_shim_valid "$check" "$FM_HOME" "$FM_ROOT"; then
-      continue
-    fi
-    id=$(basename "$check" .check.sh)
-    fm_custom_check_registered "$STATE" "$id" && continue
-    fm_pr_poll_artifacts_valid "$STATE" "$id" "$TEMPLATE" || return 1
+    [ "$(classify_check "$check")" = legacy ] && return 1
   done
+  return 0
 }
 
 private_migration_boundaries_valid() {
@@ -381,35 +398,14 @@ if [ -e "$SCAN_MARKER" ] || [ -L "$SCAN_MARKER" ]; then
   rm -f -- "$SCAN_MARKER" || exit 1
   [ ! -e "$SCAN_MARKER" ] && [ ! -L "$SCAN_MARKER" ] || exit 1
 fi
+# Migration is needed exactly when some check is not already authenticated.
 migration_needed() {
-  local check id
-  for check in "$STATE"/*.check.sh; do
-    [ -e "$check" ] || [ -L "$check" ] || continue
-    if [ "$(basename "$check")" = x-watch.check.sh ] \
-      && fmx_poll_shim_valid "$check" "$FM_HOME" "$FM_ROOT"; then
-      continue
-    fi
-    id=$(basename "$check" .check.sh)
-    fm_custom_check_registered "$STATE" "$id" && continue
-    if ! fm_pr_poll_artifacts_valid "$STATE" "$id" "$TEMPLATE"; then
-      return 0
-    fi
-  done
-  return 1
+  ! current_checks_authenticated
 }
 
+# Byte-identical to current_checks_authenticated: no legacy check remains armed.
 unsafe_checks_absent() {
-  local check id
-  for check in "$STATE"/*.check.sh; do
-    [ -e "$check" ] || [ -L "$check" ] || continue
-    if [ "$(basename "$check")" = x-watch.check.sh ] \
-      && fmx_poll_shim_valid "$check" "$FM_HOME" "$FM_ROOT"; then
-      continue
-    fi
-    id=$(basename "$check" .check.sh)
-    fm_custom_check_registered "$STATE" "$id" && continue
-    fm_pr_poll_artifacts_valid "$STATE" "$id" "$TEMPLATE" || return 1
-  done
+  current_checks_authenticated
 }
 
 revoke_migration_marker() {
@@ -1034,13 +1030,8 @@ if migration_needed; then
 
   for check in "$STATE"/*.check.sh; do
     [ -e "$check" ] || [ -L "$check" ] || continue
-    if [ "$(basename "$check")" = x-watch.check.sh ] \
-      && fmx_poll_shim_valid "$check" "$FM_HOME" "$FM_ROOT"; then
-      continue
-    fi
+    [ "$(classify_check "$check")" = legacy ] || continue
     id=$(basename "$check" .check.sh)
-    fm_custom_check_registered "$STATE" "$id" && continue
-    fm_pr_poll_artifacts_valid "$STATE" "$id" "$TEMPLATE" && continue
 
     if fm_pr_task_id_valid "$id"; then
       prefix=$id

tests/fm-pr-check-security.test.sh

@@ -3372,6 +3372,64 @@ test_gitlab_merged_poll_retires() {
   pass "GitHub and GitLab exact merged results share one retirement path"
 }
 
+test_migration_classifies_each_check_kind() {
+  local dir state x_before x_after custom_before custom_after poll_before poll_after
+  dir=$(make_case migration-four-kinds)
+  state="$dir/home/state"
+
+  # Kind 1: a valid current X-mode shim.
+  fmx_poll_shim_content "$dir/home" "$ROOT" > "$state/x-watch.check.sh"
+  chmod 0700 "$state/x-watch.check.sh"
+
+  # Kind 2: a registered custom check.
+  printf 'trusted custom check bytes\n' > "$state/custom.check.sh"
+  chmod 0700 "$state/custom.check.sh"
+  FM_HOME="$dir/home" "$REGISTER" custom >/dev/null \
+    || fail "could not register the custom check"
+
+  # Kind 3: a canonical armed PR poll.
+  write_poll_meta "$state" task-poll https://github.com/o/r/pull/30
+  seed_canonical_poll "$dir" task-poll https://github.com/o/r/pull/30
+  fm_pr_poll_artifacts_valid "$state" task-poll "$POLL" \
+    || fail "canonical poll fixture was not armed"
+
+  # Kind 4: a legacy check whose metadata is ambiguous, so migration quarantines
+  # and disarms it rather than rebuilding a canonical poll.
+  fm_write_meta "$state/task-legacy.meta" \
+    'window=fm-task-legacy' \
+    'pr=https://github.com/o/r/pull/31' \
+    'window=injected-after-pr'
+  printf 'legacy bytes\n' > "$state/task-legacy.check.sh"
+  chmod 0644 "$state/task-legacy.check.sh"
+
+  x_before=$(state_snapshot "$state" | grep 'x-watch.check.sh')
+  custom_before=$(state_snapshot "$state" | grep 'custom.check.sh')
+  poll_before=$(poll_artifact_snapshot "$state" task-poll)
+
+  FM_HOME="$dir/home" "$MIGRATE" > "$dir/migrate.out" 2> "$dir/migrate.err" \
+    || fail "four-kind migration failed: $(cat "$dir/migrate.err")"
+
+  # Only the legacy check is quarantined and disarmed.
+  [ ! -e "$state/task-legacy.check.sh" ] || fail "legacy check remained armed after migration"
+  find "$state/.pr-check-quarantine" -name 'task-legacy.check.*' -type f | grep . >/dev/null \
+    || fail "legacy check was not quarantined"
+
+  # The other three kinds are untouched, and none of them was quarantined.
+  x_after=$(state_snapshot "$state" | grep 'x-watch.check.sh')
+  [ "$x_after" = "$x_before" ] || fail "migration changed the X-mode shim"
+  custom_after=$(state_snapshot "$state" | grep 'custom.check.sh')
+  [ "$custom_after" = "$custom_before" ] || fail "migration changed the registered custom check"
+  fm_pr_poll_artifacts_valid "$state" task-poll "$POLL" \
+    || fail "migration disarmed the canonical poll"
+  poll_after=$(poll_artifact_snapshot "$state" task-poll)
+  [ "$poll_after" = "$poll_before" ] || fail "migration changed the canonical poll artifacts"
+  [ -z "$(find "$state/.pr-check-quarantine" \
+    \( -name 'custom.*' -o -name 'task-poll.*' -o -name 'x-watch.*' \) -type f)" ] \
+    || fail "migration quarantined a non-legacy check kind"
+
+  pass "migration quarantines only the legacy check when all four check kinds coexist"
+}
+
 test_parser_matrix
 test_gitlab_merge_watch
 test_merged_poll_retires_once
@@ -3407,4 +3465,5 @@ test_bootstrap_migrates_before_other_mutations
 test_bootstrap_isolates_incomplete_poll_migration
 test_custom_snapshot_cleanup_on_signal
 test_returned_custom_check_descendants_are_drained
+test_migration_classifies_each_check_kind
 test_teardown_removes_poll_artifacts