fix(ci): cover native-only protocol event drift

steipete · steipete · commit 4eb030b58af7 · 2026-07-05T07:08:11.000-04:00
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -424,6 +424,10 @@ jobs:
           }
           EOF
 
+      - name: Check mobile protocol event coverage
+        if: ${{ steps.manifest.outputs.run_node == 'true' || steps.manifest.outputs.run_ios_build == 'true' || steps.manifest.outputs.run_android_job == 'true' }}
+        run: node scripts/check-protocol-event-coverage.mjs
+
   # Run dependency-free security checks in parallel with scope detection so the
   # main Node jobs do not have to wait for Python/pre-commit setup.
   security-fast:
@@ -1206,7 +1210,6 @@ jobs:
           case "$TASK" in
             guards)
               pnpm check:no-conflict-markers
-              pnpm check:protocol-coverage
               pnpm tool-display:check
               pnpm check:host-env-policy:swift
               pnpm dup:check:coverage
diff --git a/scripts/check-protocol-event-coverage.mjs b/scripts/check-protocol-event-coverage.mjs
@@ -11,11 +11,11 @@
 // Client "handled" sets are extracted with deliberately simple parsing over
 // the mobile app sources: Swift `switch <x>.event { case "..." }` blocks plus
 // `.event == "..."` comparisons, and Kotlin `when (event) { "..." -> }` blocks
-// plus `event == "..."` comparisons scoped to `fun handle*Event(...)` bodies
-// so predicate helpers outside the dispatch path do not count as coverage. Events a client intentionally does not
-// consume live in scripts/protocol-event-coverage.allowlist.json with a
-// one-line reason. New gateway events that no client handles (and are not
-// allowlisted) fail the check.
+// plus `event == "..."` comparisons scoped to `fun handle*Event(...)` bodies.
+// Swift case labels may use qualified static string constants; those are
+// resolved across the scanned source tree so deleting the real handler cannot
+// hide behind an allowlist entry. Events a client intentionally does not
+// consume live in scripts/protocol-event-coverage.allowlist.json.
 import fs from "node:fs";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
@@ -42,6 +42,10 @@ const MIN_EXPECTED_GATEWAY_EVENTS = 10;
 const GATEWAY_EVENTS_BLOCK_RE = /export const GATEWAY_EVENTS = \[([\s\S]*?)\];/u;
 const SWIFT_EVENT_SWITCH_RE = /\bswitch\s+\w+(?:\.\w+)*\.event\s*\{/u;
 const SWIFT_CASE_LABEL_RE = /^\s*case\s+(.+?):/u;
+const SWIFT_TYPE_DECLARATION_RE =
+  /^\s*(?:(?:private|fileprivate|internal|public)\s+)?(?:enum|struct|class|actor|extension)\s+([A-Za-z_]\w*)[^{]*\x7b/u;
+const SWIFT_STATIC_STRING_CONSTANT_RE = /^\s*static\s+let\s+([A-Za-z_]\w*)\s*=\s*"([^"]+)"/u;
+const SWIFT_QUALIFIED_CONSTANT_RE = /\b([A-Za-z_]\w*\.[A-Za-z_]\w*)\b/gu;
 const KOTLIN_EVENT_WHEN_RE = /\bwhen\s*\(\s*event\s*\)\s*\{/u;
 // Kotlin gateway handlers follow the `handle*Event` naming convention
 // (handleEvent, handleGatewayEvent, handleExecApprovalGatewayEvent, ...).
@@ -148,16 +152,56 @@ function pushStringLiterals(segment, names) {
 }
 
 /**
- * Extracts event names a Swift source handles: string-literal case labels of
- * `switch <x>.event` blocks plus `.event == "..."` comparisons. Case labels
- * built from constants are invisible to this extractor and need an allowlist
- * entry explaining that.
+ * Extracts qualified static string constants declared at Swift type scope.
+ * Type qualification avoids resolving unrelated constants that share a short
+ * member name elsewhere in the app.
  */
-export function extractSwiftHandledEvents(source) {
+export function extractSwiftStaticStringConstants(source) {
+  const constants = new Map();
+  const lines = source.split("\n");
+  for (let i = 0; i < lines.length; i += 1) {
+    const declaration = SWIFT_TYPE_DECLARATION_RE.exec(lines[i]);
+    if (!declaration) {
+      continue;
+    }
+    const typeName = declaration[1];
+    let depth = 0;
+    for (let j = i; j < lines.length; j += 1) {
+      const line = lines[j];
+      if (depth === 1) {
+        const constant = SWIFT_STATIC_STRING_CONSTANT_RE.exec(line);
+        if (constant) {
+          constants.set(`${typeName}.${constant[1]}`, constant[2]);
+        }
+      }
+      const braceSource = sanitizeLineForBraces(line);
+      for (const char of braceSource) {
+        if (char === "{") {
+          depth += 1;
+        } else if (char === "}") {
+          depth -= 1;
+        }
+      }
+      if (j > i && depth <= 0) {
+        break;
+      }
+    }
+  }
+  return constants;
+}
+
+/** Extracts Swift gateway-event case labels, including qualified constants. */
+export function extractSwiftHandledEvents(source, constants = new Map()) {
   const names = collectBlockCaseLabels(source, SWIFT_EVENT_SWITCH_RE, (line, sink) => {
     const label = SWIFT_CASE_LABEL_RE.exec(line);
     if (label) {
       pushStringLiterals(label[1], sink);
+      for (const reference of label[1].matchAll(SWIFT_QUALIFIED_CONSTANT_RE)) {
+        const value = constants.get(reference[1]);
+        if (value) {
+          sink.push(value);
+        }
+      }
     }
   });
   for (const comparison of source.matchAll(SWIFT_EVENT_COMPARISON_RE)) {
@@ -325,8 +369,9 @@ function loadAllowlist(rootDir, fsImpl) {
 }
 
 function collectClientHandledEvents(params) {
-  const { rootDir, roots, extension, extract, sentinels, fsImpl } = params;
+  const { rootDir, roots, extension, extract, buildExtractContext, sentinels, fsImpl } = params;
   const handled = new Set();
+  const sources = new Map();
   for (const root of roots) {
     const rootPath = path.resolve(rootDir, root);
     if (!fsImpl.existsSync(rootPath)) {
@@ -335,14 +380,18 @@ function collectClientHandledEvents(params) {
       );
     }
     for (const filePath of listFilesRecursive(rootPath, extension, fsImpl)) {
-      for (const event of extract(fsImpl.readFileSync(filePath, "utf8"))) {
-        handled.add(event);
-      }
+      sources.set(filePath, fsImpl.readFileSync(filePath, "utf8"));
+    }
+  }
+  const extractContext = buildExtractContext?.(sources.values());
+  for (const source of sources.values()) {
+    for (const event of extract(source, extractContext)) {
+      handled.add(event);
     }
   }
   for (const sentinel of sentinels) {
     const source = readRequiredFile(rootDir, sentinel, fsImpl);
-    if (extract(source).size === 0) {
+    if (extract(source, extractContext).size === 0) {
       throw new Error(
         `Sentinel dispatch file ${sentinel} no longer matches any event names; ` +
           "its event handling likely moved or changed shape. Update scripts/check-protocol-event-coverage.mjs.",
@@ -352,6 +401,20 @@ function collectClientHandledEvents(params) {
   return handled;
 }
 
+function collectSwiftStaticStringConstants(sources) {
+  const constants = new Map();
+  for (const source of sources) {
+    for (const [name, value] of extractSwiftStaticStringConstants(source)) {
+      const existing = constants.get(name);
+      if (existing !== undefined && existing !== value) {
+        throw new Error(`Conflicting Swift string constant values for ${name}.`);
+      }
+      constants.set(name, value);
+    }
+  }
+  return constants;
+}
+
 /**
  * Runs the full coverage check against a repo checkout and returns error
  * strings plus a summary for logging.
@@ -373,6 +436,7 @@ export function collectProtocolEventCoverageErrors(params = {}) {
         roots: IOS_SCAN_ROOTS,
         extension: ".swift",
         extract: extractSwiftHandledEvents,
+        buildExtractContext: collectSwiftStaticStringConstants,
         sentinels: [IOS_SENTINEL_FILE],
         fsImpl,
       }),
diff --git a/scripts/protocol-event-coverage.allowlist.json b/scripts/protocol-event-coverage.allowlist.json
@@ -13,8 +13,6 @@
     "device.pair.requested": "Device pairing flows poll via device.pair.* methods on iOS.",
     "device.pair.resolved": "Device pairing flows poll via device.pair.* methods on iOS.",
     "voicewake.routing.changed": "iOS only consumes voicewake.changed trigger updates; routing changes are not surfaced.",
-    "exec.approval.requested": "Handled in NodeAppModel via ExecApprovalNotificationBridge constants; the literal-only extractor cannot see constant case labels.",
-    "exec.approval.resolved": "Handled in NodeAppModel via ExecApprovalNotificationBridge constants; the literal-only extractor cannot see constant case labels.",
     "plugin.approval.requested": "Plugin approval prompts are not implemented on iOS.",
     "plugin.approval.resolved": "Plugin approval prompts are not implemented on iOS.",
     "terminal.data": "Embedded terminal is a web/desktop surface; iOS has no terminal client.",
diff --git a/test/scripts/check-protocol-event-coverage.test.ts b/test/scripts/check-protocol-event-coverage.test.ts
@@ -5,6 +5,7 @@ import {
   extractGatewayEventNames,
   extractKotlinHandledEvents,
   extractSwiftHandledEvents,
+  extractSwiftStaticStringConstants,
 } from "../../scripts/check-protocol-event-coverage.mjs";
 
 const GATEWAY_LIST_FIXTURE = `
@@ -51,6 +52,11 @@ describe("extractGatewayEventNames", () => {
 
 describe("extractSwiftHandledEvents", () => {
   it("collects switch case literals and comparisons, skipping nested and non-event code", () => {
+    const constants = extractSwiftStaticStringConstants(`
+      enum SomeBridge {
+        static let requestedKind = "exec.approval.requested"
+      }
+    `);
     const source = `
       static func mapEventFrame(_ evt: EventFrame) -> Event? {
         switch evt.event {
@@ -80,14 +86,29 @@ describe("extractSwiftHandledEvents", () => {
       }
       if evt.event == "connect.challenge" { return }
     `;
-    const handled = extractSwiftHandledEvents(source);
+    const handled = extractSwiftHandledEvents(source, constants);
     expect([...handled].toSorted()).toEqual([
       "chat",
       "connect.challenge",
+      "exec.approval.requested",
       "session.message",
       "tick",
     ]);
   });
+
+  it("extracts only type-scoped static string constants", () => {
+    const constants = extractSwiftStaticStringConstants(`
+      enum ApprovalBridge {
+        static let requestedKind = "exec.approval.requested"
+        private static let nested = makeValue {
+          "not.an.event"
+        }
+      }
+      let requestedKind = "wrong.global.value"
+    `);
+
+    expect([...constants]).toEqual([["ApprovalBridge.requestedKind", "exec.approval.requested"]]);
+  });
 });
 
 describe("extractKotlinHandledEvents", () => {
diff --git a/test/scripts/ci-workflow-guards.test.ts b/test/scripts/ci-workflow-guards.test.ts
@@ -623,6 +623,22 @@ describe("ci workflow guards", () => {
     expect(preflightGuards).toContain("pnpm deps:patches:check");
   });
 
+  it("runs mobile protocol coverage for Node and native-only changes", () => {
+    const workflow = readCiWorkflow();
+    const coverageStep = workflow.jobs.preflight.steps.find(
+      (step) => step.name === "Check mobile protocol event coverage",
+    );
+    const checkShardRun = workflow.jobs["check-shard"].steps.find(
+      (step) => step.name === "Run check shard",
+    ).run;
+
+    expect(coverageStep.run).toBe("node scripts/check-protocol-event-coverage.mjs");
+    expect(coverageStep.if).toContain("steps.manifest.outputs.run_node == 'true'");
+    expect(coverageStep.if).toContain("steps.manifest.outputs.run_ios_build == 'true'");
+    expect(coverageStep.if).toContain("steps.manifest.outputs.run_android_job == 'true'");
+    expect(checkShardRun).not.toContain("check:protocol-coverage");
+  });
+
   it("does not rebuild Control UI after build:ci-artifacts", () => {
     const workflow = readCiWorkflow();
     const buildArtifactSteps = workflow.jobs["build-artifacts"].steps;
