Make start-translation fail loud instead of exiting 0 when submodule discovery finds nothing. (#69)

* make start-translation fail loud instead of exiting 0 when submodule discovery finds nothing

* fix the coderabbitai review comments

* fix the second reviewer comments

* fix the coderabbitai review comments

Author: whisper67265

.github/workflows/assets/lib.sh

@@ -154,18 +154,19 @@ ensure_local_branch_in_translations() {
   local branch="${LOCAL_BRANCH_PREFIX}${lang_code}"
   if git -C "$dir" ls-remote --exit-code --heads origin "$branch" &>/dev/null; then
     echo "  Branch $branch already exists in $TRANSLATIONS_REPO." >&2
-  else
-    echo "  Creating branch $branch in $TRANSLATIONS_REPO from $MASTER_BRANCH..." >&2
-    git -C "$dir" checkout -B "$MASTER_BRANCH" "origin/$MASTER_BRANCH"
-    git -C "$dir" checkout -b "$branch"
-    rm -rf "$dir/libs" "$dir/.gitmodules"
-    git -C "$dir" rm -rf --cached libs .gitmodules 2>/dev/null || true
-    if ! git -C "$dir" diff --cached --quiet; then
-      git -C "$dir" commit -m "Init $branch"
-    fi
-    git -C "$dir" push -u origin "$branch"
-    echo "  Created branch $branch." >&2
+    return 0
+  fi
+  echo "  Creating branch $branch in $TRANSLATIONS_REPO from $MASTER_BRANCH..." >&2
+  git -C "$dir" checkout -B "$MASTER_BRANCH" "origin/$MASTER_BRANCH" || return 1
+  git -C "$dir" checkout -b "$branch" || return 1
+  rm -rf "$dir/libs" "$dir/.gitmodules"
+  git -C "$dir" rm -rf --cached libs .gitmodules 2>/dev/null || true
+  if ! git -C "$dir" diff --cached --quiet; then
+    git -C "$dir" commit -m "Init $branch" || return 1
   fi
+  git -C "$dir" push -u origin "$branch" || return 1
+  echo "  Created branch $branch." >&2
+  return 0
 }
 
 ensure_translations_cloned() {

.github/workflows/assets/submodule_ops.sh

@@ -47,6 +47,17 @@ libs_submodule_names_from_gitmodules_file() {
     | sed 's|^libs/||'
 }
 
+# Return 1 when .gitmodules has no libs/ submodule paths.
+require_libs_submodules_in_gitmodules() {
+  local gitmodules_file="$1"
+  local names
+  names=$(libs_submodule_names_from_gitmodules_file "$gitmodules_file")
+  if [[ -z "$names" ]]; then
+    phase_err "No libs/ submodules in .gitmodules. Run add-submodules first."
+    return 1
+  fi
+}
+
 # Fetch ${BOOST_ORG}/boost .gitmodules at ref; print raw content. Return 1 on failure.
 fetch_boost_gitmodules_at_ref() {
   local ref="$1"
@@ -100,7 +111,10 @@ process_submodule_list() {
     sub="${names[$i]}"
     echo "[$(( i + 1 ))/$total] $sub ..." >&2
     if "$processor" "$sub"; then
-      record_submodule_update "$sub" || true
+      if ! record_submodule_update "$sub"; then
+        record_submodule_fatal "$sub"
+        submodule_fatal=$((submodule_fatal + 1))
+      fi
     else
       rc=$?
       if [[ $rc -eq 2 ]]; then

.github/workflows/start-translation.yml

@@ -10,10 +10,11 @@
 # Jobs:
 #   setup — validate lang_codes and emit matrix JSON.
 #   sync-mirrors — sync mirror master for all submodules; update super-repo master.
+#     Zero libs/ entries in .gitmodules is a fatal precondition (run add-submodules first).
 #     On partial submodule failure: finalize successful pointers, then exit non-zero.
 #   start-local (matrix per lang) — local-{lang_code} in mirrors + super-repo + Weblate.
-#     Runs per language from setup when sync-mirrors produced successes (see job if:).
-#     Processes only submodules that succeeded in sync-mirrors.
+#     Runs per language when sync-mirrors succeeded (see job if:).
+#     Requires a non-empty updated_submodules handoff; empty array is a job failure.
 #
 # client_payload:
 #   lang_codes: (optional) Comma-separated lang codes (e.g. zh_Hans,ja). Defaults to vars.LANG_CODES.
@@ -129,14 +130,12 @@ jobs:
             exit 1
           }
 
-          mapfile -t submodule_names < <(libs_submodule_names_from_gitmodules_file ".gitmodules")
-
-          [[ ${#submodule_names[@]} -eq 0 ]] && {
-            echo "No libs/ submodules in .gitmodules, nothing to sync." >&2
-            echo "updated_submodules=[]" >> "$GITHUB_OUTPUT"
+          require_libs_submodules_in_gitmodules ".gitmodules" || {
             end_phase
-            exit 0
+            exit 1
           }
+
+          mapfile -t submodule_names < <(libs_submodule_names_from_gitmodules_file ".gitmodules")
           end_phase
 
           begin_phase "$PHASE_PROCESS_SUBMODULES" "Sync mirror master"
@@ -168,8 +167,8 @@ jobs:
           fi
 
   start-local:
-    # always() still evaluates outputs when sync-mirrors fails; updated_submodules is only set on full success.
-    if: ${{ always() && !cancelled() && needs.setup.result == 'success' && needs.sync-mirrors.outputs.updated_submodules != '' && needs.sync-mirrors.outputs.updated_submodules != '[]' }}
+    # Run when sync-mirrors succeeded; empty updated_submodules is a fatal error inside the job.
+    if: ${{ always() && !cancelled() && needs.setup.result == 'success' && needs.sync-mirrors.result == 'success' }}
     needs: [setup, sync-mirrors]
     runs-on: ubuntu-latest
     strategy:
@@ -241,9 +240,9 @@ jobs:
           mapfile -t submodule_names < <(parse_submodule_names_json "${SYNC_MIRROR_UPDATES:-[]}")
 
           [[ ${#submodule_names[@]} -eq 0 ]] && {
-            echo "No submodules succeeded in sync-mirrors, nothing to process." >&2
+            phase_err "No submodules from sync-mirrors to process (updated_submodules is empty)."
             end_phase
-            exit 0
+            exit 1
           }
           end_phase
 

docs/ARCHITECTURE.md

@@ -224,10 +224,16 @@ On partial submodule failure, **`sync-mirrors`** still finalizes successful subm
 pointers in the super-repo via **`finalize_translations_master`**, then exits non-zero
 when **`combine_batch_and_finalize_rc`** reports failure. In that case it does **not**
 set the job output **`updated_submodules`** (only written when **`exit_rc == 0`**), so
-**`start-local`** is skipped: its **`if:`** requires a non-empty **`updated_submodules`**
-JSON array. Operators may expect partial mirror success to still hand off per-language
-Weblate work, but the workflow gate requires **full** **`sync-mirrors`** success before
-any **`start-local`** matrix job runs.
+**`start-local`** is skipped: its **`if:`** requires **`sync-mirrors`** job success.
+Operators may expect partial mirror success to still hand off per-language Weblate work,
+but the workflow gate requires **full** **`sync-mirrors`** success before any
+**`start-local`** matrix job runs.
+
+**Precondition guards.** Zero **`libs/`** entries in this repo’s **`.gitmodules`** is a
+fatal error in **`sync-mirrors`** (`Run add-submodules first.`). When **`sync-mirrors`**
+succeeds but **`updated_submodules`** is an empty JSON array (all submodules non-fatally
+skipped), **`start-local`** runs and exits non-zero with a distinct empty-handoff message
+rather than silently succeeding.
 
 ### 6.4 Related conventions (not the batch 0/1/2 contract)
 

tests/test_submodule_ops.bats

@@ -36,6 +36,43 @@ EOF
   [ "$output" = "json" ]
 }
 
+@test "require_libs_submodules_in_gitmodules: succeeds when libs/ paths exist" {
+  local gitmodules i name
+  gitmodules="$(mktemp)"
+  for i in {1..256}; do
+    name="lib${i}"
+    cat >>"$gitmodules" <<EOF
+[submodule "libs/${name}"]
+	path = libs/${name}
+EOF
+  done
+  run bash -c '
+    set -o pipefail
+    # shellcheck source=tests/helpers/common.bash
+    source "$1/helpers/common.bash"
+    load_submodule_ops
+    require_libs_submodules_in_gitmodules "$2"
+  ' _ "$BATS_TEST_DIRNAME" "$gitmodules"
+  rm -f "$gitmodules"
+  [ "$status" -eq 0 ]
+}
+
+@test "require_libs_submodules_in_gitmodules: fails when no libs/ paths" {
+  local gitmodules
+  gitmodules="$(mktemp)"
+  : >"$gitmodules"
+  run bash -c '
+    set -o pipefail
+    # shellcheck source=tests/helpers/common.bash
+    source "$1/helpers/common.bash"
+    load_submodule_ops
+    require_libs_submodules_in_gitmodules "$2"
+  ' _ "$BATS_TEST_DIRNAME" "$gitmodules"
+  rm -f "$gitmodules"
+  [ "$status" -eq 1 ]
+  [[ "$output" == *"Run add-submodules first."* ]]
+}
+
 @test "resolve_add_submodules_names: uses SUBMODULES when set" {
   SUBMODULES="algorithm, system"
   LIBS_REF="develop"
@@ -95,6 +132,21 @@ EOF
   process_submodule_list stub_processor ok ok
 }
 
+@test "process_submodule_list: buckets record_submodule_update failures as fatal" {
+  stub_processor() { return 0; }
+  record_submodule_update() { return 1; }
+
+  set +e
+  process_submodule_list stub_processor bad_name
+  status=$?
+  set -e
+  [ "$status" -eq 0 ]
+  [ "$submodule_fatal" -eq 1 ]
+  [ "${#SUBMODULE_FATAL[@]}" -eq 1 ]
+  [ "${SUBMODULE_FATAL[0]}" = "bad_name" ]
+  [ "${#UPDATES[@]}" -eq 0 ]
+}
+
 @test "combine_batch_and_finalize_rc: zero when no failures" {
   submodule_fatal=0
   run combine_batch_and_finalize_rc 0