Skip to content

fix(ci): catch native-only mobile protocol drift#100278

Merged
steipete merged 2 commits into
mainfrom
codex/fix-mobile-protocol-coverage-gaps
Jul 5, 2026
Merged

fix(ci): catch native-only mobile protocol drift#100278
steipete merged 2 commits into
mainfrom
codex/fix-mobile-protocol-coverage-gaps

Conversation

@steipete

@steipete steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Closes #100198

What Problem This Solves

Fixes an issue where mobile protocol coverage drift could escape CI when a change touched only Android or iOS, and where removing the iOS exec-approval handler still appeared covered because its event names lived in the allowlist.

Why This Change Was Made

Moves the dependency-free coverage script into CI preflight, gated by the manifest's Node, iOS, or Android selections, and removes the redundant Node check-shard call. The Swift extractor now resolves qualified type-scoped static string constants across scanned app sources, so the real ExecApprovalNotificationBridge handlers count as coverage and their allowlist exceptions disappear.

User Impact

Maintainers now get a blocking CI failure when native-only changes drop a gateway event handler. Existing iOS exec-approval handling is tracked from its actual dispatch cases instead of a manual exception.

Evidence

  • Blacksmith Testbox tbx_01kwrtn5326f5vr904aa0s9kws: corepack pnpm test test/scripts/check-protocol-event-coverage.test.ts test/scripts/ci-workflow-guards.test.ts — 44 tests passed.
  • Same Testbox: scoped check:changed for the five touched workflow/tooling files — passed.
  • Same Testbox: targeted oxfmt check — passed.
  • node scripts/check-protocol-event-coverage.mjs — 29 gateway events accounted for; iOS handles 12 and allowlists 17; Android handles 12 and allowlists 17.
  • Fresh Codex autoreview against origin/main — clean, no accepted or actionable findings.
  • git diff --check — passed.

AI-assisted: yes.

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: S maintainer Maintainer-authored PR labels Jul 5, 2026
@steipete steipete self-assigned this Jul 5, 2026
@steipete

steipete commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Land-ready on exact head e088522f1ad9904ed4404403fa9e475daa38a7c7 after rebasing onto current main.

  • Native-only iOS, Android, and shared-mobile diffs now run the dependency-free protocol coverage guard from preflight.
  • Swift constant-backed event cases resolve from scanned app sources, so the real exec-approval handlers replace their manual allowlist entries.
  • Blacksmith Testbox focused coverage/workflow tests passed (44 tests), scoped check:changed and formatting passed, the live inventory accounts for all 29 events, and fresh Codex autoreview was clean (0.96).
  • Exact-head hosted CI/Testbox passed after the final current-main rebase.

No remaining proof gaps.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 12:10 PM ET / 16:10 UTC.

Summary
This PR moves the mobile protocol event coverage script into CI preflight for Node/iOS/Android-selected changes, removes its redundant check-shard invocation, and teaches the Swift extractor to resolve type-scoped static string constants while ignoring quoted qualified names.

PR surface: Tests +58, Config +3, Other +63. Total +124 across 5 files.

Reproducibility: yes. Current-main source and tests show native-only iOS/shared mobile changes can set native jobs while leaving runNode:false, and current main only runs protocol coverage in the Node check-shard guard.

Review metrics: 1 noteworthy metric.

  • CI gate movement: 1 guard moved; 3 manifest triggers. The dependency-free protocol coverage check becomes blocking for Node, iOS, and Android-selected changes instead of only running in the Node guard shard.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100198
Summary: This PR is the active fix candidate for the canonical mobile protocol coverage issue; the earlier merged checker left the native-only CI routing and iOS Swift constant extraction gaps handled here.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] Merging this PR intentionally changes CI automation: native-only iOS or Android changes can now fail in preflight on protocol event coverage, so maintainers should accept that new blocking gate before merge.

Maintainer options:

  1. Accept the native preflight guard (recommended)
    Merge after maintainer sign-off that native-only iOS and Android changes should be blocked when mobile protocol coverage drifts.
  2. Require a narrower CI policy
    Ask the author to limit or stage the preflight condition if maintainers want less immediate CI blast radius for native-only changes.
  3. Pause the CI policy change
    Hold the PR if workflow ownership wants more review before adding this blocking preflight behavior.

Next step before merge

  • [P2] The remaining action is maintainer acceptance of the protected-label CI policy change and normal exact-head check completion, not an automated code repair.

Maintainer decision needed

  • Question: Should OpenClaw CI start running the mobile protocol event coverage guard as a blocking preflight step for Node, iOS, and Android-selected changes?
  • Rationale: The code repair is narrow and the prior extractor false positive is fixed, but the diff changes blocking CI policy on a protected maintainer-labeled PR.
  • Likely owner: steipete — steipete authored the first-pass checker and this follow-up and is assigned on the PR.
  • Options:
    • Accept preflight enforcement (recommended): Merge after maintainers accept this manifest-gated coverage check as a blocking preflight step and exact-head required checks pass.
    • Move the guard elsewhere: Ask for a different workflow placement if preflight is not the desired enforcement point.
    • Pause for workflow owner review: Keep the PR open if the added native-only failure mode needs broader CI ownership sign-off.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes a repo-local workflow step, local Node script, tests, and allowlist data without new actions, dependencies, permissions, secrets, or downloaded code execution.

Review details

Best possible solution:

Land the focused CI guard after maintainer acceptance of the preflight enforcement point and normal exact-head required checks.

Do we have a high-confidence way to reproduce the issue?

Yes. Current-main source and tests show native-only iOS/shared mobile changes can set native jobs while leaving runNode:false, and current main only runs protocol coverage in the Node check-shard guard.

Is this the best way to solve the issue?

Yes, with maintainer approval for CI policy. Moving the dependency-free checker into manifest-gated preflight is the narrowest owner-boundary fix, and the current head fixes the earlier quoted Swift constant false positive.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 286d0b9fb340.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and comment provide terminal/Testbox proof for the tooling behavior, including focused tests, scoped changed checks, formatting, direct checker output, hosted CI/Testbox, and clean autoreview.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a bounded CI coverage repair for mobile protocol drift without evidence of a live outage.
  • merge-risk: 🚨 automation: The PR changes blocking GitHub Actions behavior by adding a manifest-gated preflight protocol coverage step.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and comment provide terminal/Testbox proof for the tooling behavior, including focused tests, scoped changed checks, formatting, direct checker output, hosted CI/Testbox, and clean autoreview.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and comment provide terminal/Testbox proof for the tooling behavior, including focused tests, scoped changed checks, formatting, direct checker output, hosted CI/Testbox, and clean autoreview.
Evidence reviewed

PR surface:

Tests +58, Config +3, Other +63. Total +124 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 2 59 1 +58
Docs 0 0 0 0
Config 1 4 1 +3
Generated 0 0 0 0
Other 2 80 17 +63
Total 5 143 19 +124

What I checked:

  • Repository policy read: Root policy plus scripts, test, iOS, and Android scoped AGENTS.md files were read; the relevant review guidance is to inspect beyond the diff and treat CI workflow changes as automation merge risk. (AGENTS.md:1, 286d0b9fb340)
  • Current-main routing gap: Current main runs pnpm check:protocol-coverage only inside the check-shard guards task, and that job is gated by needs.preflight.outputs.run_check == 'true'. (.github/workflows/ci.yml:1157, 286d0b9fb340)
  • Native-only changes skip Node scope: Current scope detection sets iOS and Android flags for native paths while leaving runNode false for iOS/shared native-only examples, which explains how the current check-shard placement can miss mobile-only changes. (src/scripts/ci-changed-scope.test.ts:216, 286d0b9fb340)
  • PR preflight placement: The PR adds Check mobile protocol event coverage to preflight when run_node, run_ios_build, or run_android_job is selected, and removes the check-shard invocation. (.github/workflows/ci.yml:427, 753e4a941df1)
  • Swift constant extraction: The PR extracts type-scoped Swift static string constants, passes the collected constants into Swift event extraction, and sanitizes case-label strings before resolving qualified references. (scripts/check-protocol-event-coverage.mjs:159, 753e4a941df1)
  • iOS handler path: Current iOS code handles exec.approval.requested and exec.approval.resolved through ExecApprovalNotificationBridge constants, matching the blind spot this PR removes from the allowlist. (apps/ios/Sources/Model/NodeAppModel.swift:992, 286d0b9fb340)

Likely related people:

  • steipete: Authored the merged first-pass checker in chore(ci): fail CI when gateway events go unhandled by the mobile apps #100206, opened the canonical issue follow-up, authored this branch, and is assigned on the PR. (role: first-pass guard author and current follow-up owner; confidence: high; commits: 5af24d16bf01, 122a5fdcbb3b, 753e4a941df1; files: scripts/check-protocol-event-coverage.mjs, scripts/protocol-event-coverage.allowlist.json, .github/workflows/ci.yml)
  • Vincent Koc: Current-main blame in this checkout points to Vincent Koc on the check-shard guard placement and Swift extractor lines, so this is a recent-routing history signal rather than authorship of this PR. (role: recent CI routing contributor; confidence: low; commits: fd2e4da00651; files: .github/workflows/ci.yml, scripts/check-protocol-event-coverage.mjs)
  • joshavant: Merged fix: route mobile exec approvals to reviewer device #95175 added the iOS foreground exec-approval event handling whose constant-backed cases the checker now recognizes. (role: adjacent iOS approval contributor; confidence: medium; commits: 5d1e649aeadf; files: apps/ios/Sources/Model/NodeAppModel.swift, apps/ios/Sources/Push/ExecApprovalNotificationBridge.swift)
  • Solvely-Colin: Merged fix: route Android exec approvals to in-app inbox #95593 added Android gateway-backed exec approval handling, an adjacent mobile client surface protected by this coverage check. (role: adjacent Android approval contributor; confidence: medium; commits: e39249100ebb; files: apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt, apps/android/app/src/main/java/ai/openclaw/app/GatewayExecApprovals.kt)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (5 earlier review cycles)
  • reviewed 2026-07-05T10:45:54.208Z sha 52ec7de :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T11:14:22.152Z sha 4eb030b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T12:19:07.848Z sha 1e861d5 :: needs changes before merge. :: [P2] Skip quoted Swift constants during case resolution
  • reviewed 2026-07-05T13:03:11.173Z sha e088522 :: needs changes before merge. :: [P2] Skip quoted Swift constants during case resolution
  • reviewed 2026-07-05T16:04:15.457Z sha 753e4a9 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jul 5, 2026
@steipete
steipete force-pushed the codex/fix-mobile-protocol-coverage-gaps branch from 69cfec7 to 4eb030b Compare July 5, 2026 11:08
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: imessage Channel integration: imessage docker Docker and sandbox tooling channel: qqbot size: XL and removed size: S labels Jul 5, 2026
@steipete
steipete force-pushed the codex/fix-mobile-protocol-coverage-gaps branch from 2027864 to 1e861d5 Compare July 5, 2026 12:01
@openclaw-barnacle openclaw-barnacle Bot added size: S and removed docs Improvements or additions to documentation channel: imessage Channel integration: imessage docker Docker and sandbox tooling channel: qqbot size: XL labels Jul 5, 2026
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jul 5, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e861d5122

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/check-protocol-event-coverage.mjs Outdated
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
@steipete
steipete force-pushed the codex/fix-mobile-protocol-coverage-gaps branch 2 times, most recently from a48bc94 to e088522 Compare July 5, 2026 12:50
@steipete
steipete force-pushed the codex/fix-mobile-protocol-coverage-gaps branch from e088522 to 753e4a9 Compare July 5, 2026 15:55
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 5, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head land proof for 753e4a941df1e975a66992eff7a509827f70cfbb:

  • Blacksmith Testbox tbx_01kwset3wyjah8xrcgz0mx0dge: 45 focused protocol/workflow tests passed; scoped check:changed passed.
  • Live protocol scan: all 29 catalog events accounted for.
  • CI run 28746434462: all branch-related jobs passed. QA Smoke's first attempt had one unrelated Control UI scroll-timing flake; isolated rerun job 85238994560 passed all 81 scenarios.
  • Fresh autoreview: no accepted/actionable findings.
  • Review artifacts validated with recommendation READY FOR /prepare-pr.

No remaining proof gaps for this CI/tooling-only change.

@steipete
steipete merged commit e62cf76 into main Jul 5, 2026
157 of 164 checks passed
@steipete
steipete deleted the codex/fix-mobile-protocol-coverage-gaps branch July 5, 2026 16:23
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

The required scripts/pr prepare and merge path performed the squash merge. Its final commit-endpoint check raced GitHub propagation; the landed commit is now on main.

@steipete steipete mentioned this pull request Jul 5, 2026
Closed
@github-actions github-actions Bot mentioned this pull request Jul 5, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 6, 2026
@steipete
fix(ci): catch native-only mobile protocol drift (openclaw#100278)
4ff395f 
* fix(ci): cover native-only protocol event drift

* fix(ci): ignore quoted Swift event constants
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

@steipete steipete

Labels

maintainer Maintainer-authored PR merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CI: gateway-protocol event coverage check for mobile clients

1 participant

@steipete