CI: validate the dialect definitions on every pull request - #11
Open
TSC21 wants to merge 4 commits into
Open
Conversation
The repository documents an allocation policy (README.md, IDMAPPING.md, the military.xml header) and a wire contract (message IDs are stable once assigned; CRC_EXTRA is what a peer checks before accepting a message), but nothing machine-checks either one, so a pull request can allocate outside the 53xxx reservation, collide with the private 53900-53999 block, or change a message body without the wire impact being visible in review. check_dialect_policy.py enforces the documented allocation over both dialect files: military.xml draws messages and MAV_CMD entries from the shared 53000-53899 window, military_extensions.xml only from the private block, names follow MAVLink case conventions, and the template cannot collide with a shared ID or name. It runs on the Python standard library alone so it needs nothing checked out besides this repository. Structure beyond the policy (schema conformance, field types, duplicates against common.xml) stays owned by the schema check and by mavgen, which parse the full include tree. check_wire_compat.py parses two versions of military.xml with pymavlink's generator, the same CRC_EXTRA computation every consumer runs, and reports removals, renames and CRC changes per message ID. The report is advisory by default because the dialect is under active development and replacing a message body is a decision for review, not for a bot; WIRE_COMPAT_ENFORCE=1 turns findings into a failure once the dialect freezes. Changes confined to extension fields keep CRC_EXTRA stable and are wire-compatible by design, so they are not reported. Validation: the policy check passes on the current tree (26 messages and 4 MAV_CMD entries in the shared window, 2 template messages in the private block) and a synthetic tree with 11 planted violations reports all 11 and exits 1. The wire check on identical inputs reports zero changes; against the head of pull request #10 it reports exactly the eleven messages that PR touches, with FIRES at CRC_EXTRA 124, matching the message-entry table published in that PR's description.
A change to military.xml gets its first machine check after it merges: the header-regeneration workflow is the only one configured, it runs on push to main, and the open dialect pull requests show no checks at all. Schema errors, allocation mistakes and invisible wire breaks therefore reach review with no tooling behind the reviewer. validate_dialect.yml runs on every pull request and push to main, and every job in it reads: nothing writes to the repository, publishes an artifact, or exercises dialect behavior, keeping CI inside the same boundary the README draws for the dialect itself. The jobs: - schema: xmllint against pymavlink's mavschema.xsd for both dialect files. The extensions template is excluded from generation by design, so this is the one gate that keeps it well-formed. - policy: the documented 53xxx allocation and MAVLink naming conventions, via check_dialect_policy.py. - wire-compat (pull requests only): CRC_EXTRA diff against the PR base via check_wire_compat.py, advisory, written to the run's step summary. - generate-c: mavgen C with schema validation and --strict-units on, then a syntax-and-type pass over the generated header at -Wall -Wextra -Werror. No consumer or example code exists in the repository and none is written in CI: the compiler parses the header as an include, it does not construct or move a message. -Wno-address-of-packed-member is required by design of the generator's helpers and fires inside the generated common.xml headers, independent of this dialect. - generate-python: mavgen Python with the same validation, then an import that proves the module initializes with a complete message map. The setup-mavlink-tooling composite action is the single source of truth for the toolchain: mavlink/mavlink 92c81a5 for the upstream definitions military.xml includes, ArduPilot/pymavlink 839c381 for the generator. The two are a pair proven against each other, so generation runs with schema validation on. Actions are pinned by commit SHA, jobs carry timeouts, workflow permissions are contents: read, checkouts drop credentials, and superseded pull request runs are cancelled. Validation, local, with the pinned pair and the exact job commands: xmllint validates both dialect files; mavgen C and Python generate cleanly with validation and strict units (260 messages across 4 XML files); the C header type-check passes; the Python import reports 260 messages in the map; the C CRC table regenerated by the pinned pair is identical to the committed generated/ tree.
The regeneration workflow tracks mavlink/mavlink and ArduPilot/pymavlink at master, so its output and its ability to run at all depend on the day it fires, and generation carries --no-validate because the two moving heads drift out of schema agreement, which waives validation for the dialect itself along with them. Checkout actions float on major-version tags, the job holds a blanket contents: write, and nothing bounds its runtime. The workflow keeps its one purpose, holding generated/ in step with military.xml on main, and draws its toolchain through the same setup-mavlink-tooling composite action as the validation suite: one pinned mavlink/pymavlink pair, proven against each other, so schema validation and --strict-units stay on and a failure here is a real dialect defect, not upstream drift. The output tree is cleared before generation so a message removed in the dialect cannot leave a stale header behind. Permissions are contents: read at the workflow with contents: write scoped to the one job that pushes, actions are pinned by commit SHA, the job carries a timeout, and a concurrency group serializes regenerations without cancelling one mid-push. The self checkout alone keeps its credentials because the push step needs the job token; the zizmor waiver on that line records the reason. Validation: regenerating with the pinned pair reproduces the committed CRC table byte-identical, so the first run on main refreshes generator banner lines only, not wire content.
Workflow definitions are code that runs with credentials, and this repository reviews them by eye alone. Two linters close that gap on every change under .github/: actionlint for correctness (expression types, runner labels, shellcheck over run blocks) and zizmor for security posture (credential persistence, template injection, unpinned actions). actionlint is fetched as a pinned release binary and verified against a recorded sha256 before it runs; zizmor installs version-pinned from PyPI. Both gate pull requests and main. dependabot.yml keeps the SHA-pinned actions current with one grouped pull request a month behind a seven-day cooldown, the right noise floor for a definitions repository. The mavlink/pymavlink generator pins are checkout refs inside the composite action, outside dependabot's reach, and move by hand, together, behind a green validation run. Validation: actionlint 1.7.12 exits clean over the three workflows; zizmor 1.28.0 reports no findings across the workflows, the composite action and dependabot.yml, with the one artipacked waiver justified in-file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributed on behalf of RIIS, LLC.
This PR gives the repository a CI suite for the definitions themselves: that the XML validates, that the 53xxx allocation and naming rules the README and IDMAPPING.md document actually hold, that a change's wire impact is visible in review, and that the dialect still generates. Everything runs on the pull request, before merge, and everything is read-only.
What I ran into
While preparing the addressing RFC (#10) I noticed none of it had run through any check: the open dialect PRs show no checks at all, because the one workflow this repository has triggers on push to main, after merge. So a schema error, an ID outside the reservation, or a silent CRC change lands first and surfaces later, downstream.
The regeneration workflow itself also depends on the day it runs. It checks out mavlink/mavlink and ArduPilot/pymavlink at master, and it generates with
--no-validatebecause the two moving heads drift out of schema agreement, which waives validation for this dialect's own content along with upstream's. Its actions float on version tags, the job holds a blanket write permission, and nothing bounds its runtime.What this PR does
Three workflows, one composite action, two small scripts, all of it definition hygiene.
validate_dialect.yml runs five read-only jobs on every PR and push to main:
mavschema.xsd, for bothmilitary.xmlandmilitary_extensions.xml. The template is excluded from generation by design, so this is the one gate that keeps it well-formed.military.xmldraws messages and MAV_CMD entries from 53000-53899, the template only from the private 53900-53999 block, MAVLink naming conventions, no duplicate or colliding IDs and names. The script is stdlib-only and enforces nothing the README does not already say.military.xmlwith pymavlink's own generator and reports, per message ID, what a peer would notice: removals, renames, CRC_EXTRA changes. It is advisory, written to the run's step summary, and never fails the check. I chose report-only deliberately: the dialect is under development and replacing a message body is sometimes the right call, RFC: addressing for the MAVLink-M messages that direct a specific system #10 itself changes eleven CRCs on purpose, so this job's work is to make the wire impact impossible to miss in review, not to veto it. One env flag (WIRE_COMPAT_ENFORCE=1) turns it blocking once IDs freeze.--strict-unitson, then the generated header is parsed and type-checked at-Wall -Wextra -Werror. No consumer or example code is committed and none is written in CI: the compiler reads the header as an include, and nothing constructs, encodes or sends a message.generate_c_lib.yml keeps its one job, regenerating
generated/on main, rebuilt on the pinned toolchain. The pins live in one place, asetup-mavlink-toolingcomposite action used by every job: mavlink/mavlink92c81a5(master, 2026-07-29) and ArduPilot/pymavlink839c381(master, 2026-07-31), a pair proven against each other, so--no-validateis gone and validation is on everywhere. The output tree is cleared before regenerating so a removed message cannot leave a stale header behind. Permissions drop to read with write scoped to the one job that pushes, and runs are serialized and never cancelled mid-push.workflow_lint.yml makes the workflows police themselves: actionlint (pinned release binary, checksum verified before it runs) and zizmor (version-pinned from PyPI) on every change under
.github/. Adependabot.ymlkeeps the SHA-pinned actions current with one grouped PR a month behind a seven-day cooldown; the mavlink/pymavlink pins move by hand, together, behind a green validation run.Across all of it: every third-party action pinned by full commit SHA, a timeout and least-privilege permissions on every job, checkouts drop credentials except the one that pushes (waived in-file with the reason), no
pull_request_target, and no run block interpolates untrusted input.What this deliberately does not do
The README draws the dialect's boundary at carrying intent and observation, and the CI stays inside the same line. It validates and generates definitions; it does not exercise them. No message is ever constructed or sent, no consumer or example code enters the repository, no artifact is published beyond run logs and step summaries (generated bindings exist only inside the runner and are discarded with it), and nothing here touches firmware, vehicles or simulation. I also left out release pipelines, scorecard badges and similar machinery: for a definitions repository they add surface, not assurance.
Local verification
Every job ran locally with the exact pinned pair and the exact commands before opening this:
xmllint --schema mavschema.xsd:military.xmlandmilitary_extensions.xmlboth validate.-Wall -Wextra -Werror: passes.generated/with the pinned pair reproduces the committed CRC table byte-identical, so the first post-merge regeneration refreshes generator banner lines only, not wire content.Questions
WIRE_COMPAT_ENFORCEis a one-line change.auterion-ci <ci@auterion.com>as on main today; happy to switch it if the project prefers a Dronecode identity.