From 0fa0ece024fecae0847459b5949c66f40fcc6e11 Mon Sep 17 00:00:00 2001
From: hathach <thach@tinyusb.org>
Date: Thu, 20 Aug 2026 18:30:45 +0700
Subject: hil: address Copilot review — loud extraction markers, exit-visible
 variant warnings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The workflow-logic harness slices hil-validate.js between marker strings (the body is not
a module; the runtime wraps it, so markers are the only handle). A renamed marker used to
produce a garbage slice and a confusing ReferenceError; it now fails naming the missing
marker, proven by mutating the marker and watching the message.

The variant-warning loop in hil_ci.sh read variant_names through a process substitution --
the exact exit-status blindness the comment in resolve_build_dirs warns about, two
functions earlier in the same file. A plain command-substitution assignment is visible to
set -e, so a malformed roster now aborts instead of silently skipping the warnings.
---
 .claude/workflows/test-hil-validate.mjs | 14 ++++++++++++--
 test/hil/hil_ci.sh                      |  5 ++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/.claude/workflows/test-hil-validate.mjs b/.claude/workflows/test-hil-validate.mjs
index 80f37b27d..db73095f6 100644
--- a/.claude/workflows/test-hil-validate.mjs
+++ b/.claude/workflows/test-hil-validate.mjs
@@ -11,8 +11,18 @@
 import { readFileSync } from 'node:fs'
 
 const src = readFileSync(new URL('./hil-validate.js', import.meta.url), 'utf8')
-const body = src.slice(src.indexOf('const byBoard ='), src.indexOf('const first = await runBoards'))
-  + src.slice(src.indexOf('const summarize ='), src.indexOf('const { pass, wedged, locked } ='))
+// slice by marker, but never silently: a renamed marker must fail with its name, not with a
+// confusing ReferenceError from a garbage slice
+const cut = (start, end) => {
+  const a = src.indexOf(start), b = src.indexOf(end)
+  if (a < 0 || b < 0 || b <= a) {
+    console.error(`FAIL: extraction marker moved — cannot find ${a < 0 ? `'${start}'` : `'${end}'`} in hil-validate.js`)
+    process.exit(1)
+  }
+  return src.slice(a, b)
+}
+const body = cut('const byBoard =', 'const first = await runBoards')
+  + cut('const summarize =', 'const { pass, wedged, locked } =')
 const { byBoard, summarize, wedgedFor } = new Function(`${body}; return { byBoard, summarize, wedgedFor }`)()
 
 let failed = 0
diff --git a/test/hil/hil_ci.sh b/test/hil/hil_ci.sh
index f474c3055..66f4e48d4 100644
--- a/test/hil/hil_ci.sh
+++ b/test/hil/hil_ci.sh
@@ -163,12 +163,15 @@ for b in ${BOARDS[@]+"${BOARDS[@]}"}; do
     # variant-suffixed, so this is the normal state for e.g. the -DMA variants. It is worth
     # saying out loud: hil_test.py logs `Skip (no binary)` and counts zero errors for it, so
     # the run exits 0 and the operator reads a green table for cells that never ran.
+    # plain assignment, not process substitution: set -e sees a variant_names failure here,
+    # the same trap the comment in resolve_build_dirs warns about
+    vnames=$(variant_names "$b")
     while IFS= read -r v; do
       [ -z "$v" ] && continue
       # whole lines: a substring match lets cmake-build-<v>-DMA silence the warning for <v>
       grep -qxF -- "$ROOT_DIR/examples/cmake-build-$v" <<< "$dirs" \
         || echo "warning: $b variant '$v' has no build dir -- its cells will be skipped, not tested" >&2
-    done < <(variant_names "$b")
+    done <<< "$vnames"
   fi
 done
 if [ ${#missing[@]} -gt 0 ]; then
-- 
cgit v1.3.1