From 23c59d7a172daae8a590af864aee585888790c1d Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Sat, 29 Aug 2026 03:17:37 -0400
Subject: [PATCH] chore: address Copilot review round 10
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The CI half of the wiring vouch matched the workflow's raw text, so a YAML
comment, a step `name:`, or an unused anchor mentioning `npm run
verify:skills:cli` satisfied it — including this guard's own docblock wording.
Deleting the real step would then have left `verify:skills` green, which is the
vacuous-guard failure the vouch exists to prevent.

It now reads the workflow's executable regions via `extractExecutableRegions`,
the same helper the #2146 workflow gate is built on, and matches only a `run:`.
Covered by three should-not-count cases (comment, step name, workflow input)
plus a multi-line `run:` that must count; verified by mutation — restoring the
substring check fails them.

Signed-off-by: cliffhall <cliff@futurescale.com>
---
 scripts/verify-skills.main.test.mjs | 31 +++++++++++++++++++++++++++--
 scripts/verify-skills.mjs           | 18 +++++++++++++++--
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/scripts/verify-skills.main.test.mjs b/scripts/verify-skills.main.test.mjs
index 4ead997c..cdd114ba 100644
--- a/scripts/verify-skills.main.test.mjs
+++ b/scripts/verify-skills.main.test.mjs
@@ -236,12 +236,15 @@ test("checkWiring catches the authoritative validator dropped from CI", () => {
       WIRED_SCRIPTS,
       "jobs:\n  build:\n    steps:\n      - run: npm run validate\n",
     ).join(),
-    /workflows\/main\.yml` no longer runs/,
+    /has no step that runs/,
   );
 });
 
 test("checkWiring reports every broken link at once", () => {
-  assert.equal(checkWiring({ validate: "", "local:gate": "" }, "").length, 3);
+  assert.equal(
+    checkWiring({ validate: "", "local:gate": "" }, "jobs: {}\n").length,
+    3,
+  );
 });
 
 test("the repository as it stands is wired", () => {
@@ -259,3 +262,27 @@ test("the repository as it stands is wired", () => {
   );
   assert.deepEqual(checkWiring(scripts, workflow), []);
 });
+
+test("checkWiring is not satisfied by a mention outside an executable step", () => {
+  // A raw substring match would pass on any of these while CI ran nothing —
+  // including on a step whose NAME describes the check it no longer performs,
+  // which is exactly how a wiring guard goes quietly vacuous.
+  const mentions = [
+    "# npm run verify:skills:cli\njobs:\n  build:\n    steps:\n      - run: npm run validate\n",
+    "jobs:\n  build:\n    steps:\n      - name: npm run verify:skills:cli\n        run: npm run validate\n",
+    "on:\n  workflow_dispatch:\n    inputs:\n      cmd:\n        default: npm run verify:skills:cli\njobs:\n  build:\n    steps:\n      - run: npm run validate\n",
+  ];
+  for (const workflow of mentions) {
+    assert.match(
+      checkWiring(WIRED_SCRIPTS, workflow).join(),
+      /has no step that runs/,
+      `should not count: ${workflow.split("\n")[0]}`,
+    );
+  }
+});
+
+test("checkWiring accepts the command inside a multi-line run step", () => {
+  const workflow =
+    "jobs:\n  build:\n    steps:\n      - run: |\n          npm run validate\n          npm run verify:skills:cli\n";
+  assert.deepEqual(checkWiring(WIRED_SCRIPTS, workflow), []);
+});
diff --git a/scripts/verify-skills.mjs b/scripts/verify-skills.mjs
index fd4d44c9..126e3519 100755
--- a/scripts/verify-skills.mjs
+++ b/scripts/verify-skills.mjs
@@ -32,6 +32,7 @@ import path from "node:path";
 import { fileURLToPath } from "node:url";
 import { claudeSpawnArgs, probeClaudeVersion } from "./lib/claude-cli.mjs";
 import { reachableScripts, rootReachesScript } from "./lib/npm-scripts.mjs";
+import { extractExecutableRegions } from "./lib/workflow-gate.mjs";
 import {
   compareVersions,
   parseClaudeVersion,
@@ -73,6 +74,14 @@ export function skillDirs(
  *     the step that actually runs the authoritative validator, and it is
  *     reachable from neither `validate` nor any test, so nothing else would
  *     notice it going missing.
+ *
+ * The workflow half reads the **executable** regions rather than the file's
+ * text. A raw substring match is satisfied by a comment, a step `name:`, or an
+ * unused anchor that merely mentions the command — so deleting the real step
+ * while leaving this very docblock's wording nearby would keep the guard green
+ * (Copilot). `extractExecutableRegions` already descends only the schema paths
+ * Actions actually runs, with aliases resolved, and it is the same helper the
+ * #2146 workflow gate is built on.
  */
 export function checkWiring(rootScripts, workflowText) {
   const problems = [];
@@ -86,9 +95,14 @@ export function checkWiring(rootScripts, workflowText) {
       "`npm run local:gate` no longer runs `verify:skills:cli`, so the authoritative validator never runs locally. Restore it.",
     );
   }
-  if (!/npm run verify:skills:cli/.test(workflowText)) {
+  const runsInCi = extractExecutableRegions(workflowText, "main.yml").some(
+    (region) =>
+      region.kind === "run" &&
+      /\bnpm run verify:skills:cli\b/.test(region.text),
+  );
+  if (!runsInCi) {
     problems.push(
-      "`.github/workflows/main.yml` no longer runs `npm run verify:skills:cli`, so the authoritative validator never runs in CI. Restore it.",
+      "`.github/workflows/main.yml` has no step that runs `npm run verify:skills:cli`, so the authoritative validator never runs in CI. Restore it.",
     );
   }
   return problems;