Skip to content

ci(validate_dialect): validate the dialect on every pull request - #11

Merged
mrpollo merged 17 commits into
mainfrom
ci/dialect-validation-suite
Oct 9, 2026
Merged

mrpollo merged 17 commits into
mainfrom
ci/dialect-validation-suite

Conversation

@TSC21

@TSC21 TSC21 commented Aug 1, 2026 •

Copy link
Copy Markdown
Member

On main, a dialect change gets one check before it merges: the C headers still generate. This adds the checks a reviewer needs on every pull request. Both XML files are validated against the MAVLink schema, the documented ID allocation and naming rules are checked with each finding annotated on its line, and IDMAPPING.md has to match the XML. A wire-compatibility report shows every CRC_EXTRA change with the generated entry row of each changed message, and mavlink's own check_api_break.py adds what CRC_EXTRA can't see, such as changed enum values; it only reports until message IDs are frozen, when setting WIRE_COMPAT_ENFORCE to "1" makes it blocking. C and Python are generated with strict units and the headers compiled at -Werror, the built docs site is checked for broken links, anchors and sitemap URLs, and the workflows themselves are linted with actionlint, shellcheck and zizmor. Their scripts live in a folder per workflow under .github/scripts/, with tests that the same workflow runs, so every check runs locally the way CI runs it.

It also changes how the C library and the site are published. The publish job pushes the headers the generate job built, so no generator code runs next to the deploy key, and it runs in the c-library environment, which holds that key and is limited to main. Pull requests rehearse the publish against a local copy of the C library; the first rehearsal found that rsync skipped a regenerated version.h written in the same second as the checkout, so the publish script compares contents, and it skips a publish whose only change is the build date. The workflow runs only when its inputs change. Each docs deploy is followed by a check of the live site. The mavlink and pymavlink pins live in .github/mavlink-pins.env, which the checks, the publish and the docs reference all read, so a bump touches one file. Every action is pinned by SHA, the commit-checks workflow included, jobs have timeouts and least-privilege permissions, and dependabot groups action updates monthly.

There is no change to the dialect or the generated headers.

With the pinned mavlink 87da370 and pymavlink 19880422 every check passes on main's dialect, 83 script tests and 17 site tests pass with full line and branch coverage of the four Python checkers, and the linters report nothing.

Assisted-by: Claude-Code:claude-opus-5-5


RIIS


@TSC21
TSC21 requested a review from mrpollo August 1, 2026 18:36
@TSC21
TSC21 force-pushed the ci/dialect-validation-suite branch from c337498 to e9a8481 Compare October 2, 2026 14:11
@TSC21 TSC21 changed the title CI: validate the dialect definitions on every pull request ci: validate the dialect definitions on every pull request Oct 2, 2026
@TSC21
TSC21 requested review from mrpollo and removed request for mrpollo October 2, 2026 14:15
@TSC21
TSC21 force-pushed the ci/dialect-validation-suite branch 2 times, most recently from caea0ae to 3797060 Compare October 2, 2026 16:23
@TSC21 TSC21 changed the title ci: validate the dialect definitions on every pull request ci(validate_dialect): validate the dialect on every pull request Oct 2, 2026
@TSC21
TSC21 force-pushed the ci/dialect-validation-suite branch from 3797060 to 157da87 Compare October 2, 2026 16:31
@mrpollo

mrpollo commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Could this PR also skip publishing when only the build date changed, and narrow the generate triggers? Both touch files it already rewrites.

Merging #15 (the contributing guidelines) published 3c168c6, which only bumps MAVLINK_BUILD_DATE. With --checksum that would happen on every rerun, so the publish check could ignore that line with git diff --cached --quiet -I '^#define MAVLINK_BUILD_DATE ', and the same-second test would then use a real header change. Narrowing the triggers to generate_c_headers.sh, publish_c_library.sh and the setup action would keep unrelated script changes from rerunning generation.

Assisted-by: Claude-Code:claude-opus-5-5

@TSC21

TSC21 commented Oct 2, 2026 •

Copy link
Copy Markdown
Member Author

@mrpollo done, in c3515c4 and 70337f8.

The publish check is git diff --cached --quiet -I '^#define MAVLINK_BUILD_DATE ', so a change made only of build-date lines publishes nothing, while any other change still carries the new date with it. The same-second test uses a same-size CRC define instead, and a new test covers the date-only case; with -I taken out, that test is the one that fails.

The generate triggers are military.xml, the workflow itself, the pins file, the setup action and .github/scripts/generate_c_lib/, which holds its scripts and their tests, and the workflow runs those tests itself.

@mrpollo mrpollo 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.

General rule for the workflows and the setup action: a run: block longer than 3 lines goes into .github/scripts/ as its own script, following generate_c_headers.sh and publish_c_library.sh: POSIX sh, a usage header, inputs as arguments. A single command wrapped over several lines, like the mavgen and gcc calls, can stay inline. Scripts can be shellchecked, tested like the Python checkers, and run locally. I've marked each block that needs it.

The c-library environment is set up, holds C_LIBRARY_DEPLOY_KEY and is limited to main.

Assisted-by: Claude-Code:claude-opus-5-5

Comment thread .github/actions/setup-mavlink-tooling/action.yml Outdated
Comment thread .github/workflows/docs.yml Outdated
Comment thread .github/workflows/generate_c_lib.yml Outdated
Comment thread .github/workflows/generate_c_lib.yml Outdated
Comment thread .github/workflows/validate_dialect.yml Outdated
Comment thread .github/workflows/validate_dialect.yml Outdated
Comment thread .github/workflows/workflow_lint.yml
@TSC21
TSC21 force-pushed the ci/dialect-validation-suite branch from 2ef6cb6 to 16c9417 Compare October 2, 2026 17:23
@TSC21

TSC21 commented Oct 2, 2026 •

Copy link
Copy Markdown
Member Author

@mrpollo thanks for setting up c-library, I took that ask out of the description. Every run block is now three lines or less, or a single wrapped command, the xmllint install and the gcc and Python mavgen steps included.

TSC21 added 14 commits October 2, 2026 19:09
.github/scripts holds every helper in one flat folder, and this series
adds a dozen scripts and their tests. A flat folder hides which
workflow runs which script, while CONTRIBUTING.md scopes a script
change by the workflow that runs it.

Each workflow's scripts go to .github/scripts/<workflow>/:
check_commits.sh to commit_checks/, generate_c_headers.sh and
publish_c_library.sh to generate_c_lib/. Their tests go to a tests/
folder beside them, which the same workflow runs. The files move byte
for byte; the workflows, the CONTRIBUTING.md examples and the commit
and pr skills name the new paths, and CONTRIBUTING.md states the
layout. generate_c_lib.yml keeps triggering on .github/scripts/**,
which matches the new folders too.

Validation: git records the three moves as 100% renames, actionlint
1.7.12 reports nothing, shellcheck 0.11.0 finds nothing in the moved
scripts, and check_commits.sh passes every subject of this series
from its new path.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
The conventions job checks out the pull request head with
actions/checkout@v4, a tag that can move, and keeps the token in
.git/config for the rest of the job, although nothing after the
checkout pushes. It runs without a timeout, so a hung step holds a
runner for the default six hours.

The checkout is pinned to v4.4.0 by SHA with persist-credentials
false, and the job is bounded at five minutes. The job keeps its id
and gets no name, so a required status check that names it still
matches.

Validation: actionlint 1.7.12 reports nothing, and zizmor 1.30.1
finds nothing in commit_checks.yml.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
MAVLINK_REF and PYMAVLINK_REF sit in generate_c_lib.yml's env, and
docs/scripts/generate_messages.py digs MAVLINK_REF out of that YAML
with a regular expression, so the docs depend on the workflow's
layout, and docs.yml misses a pin bump unless it also triggers on the
generate workflow. The generate job sets up the generator in four
steps of its own, which every other job that generates code would
repeat.

The pins live in .github/mavlink-pins.env as plain KEY=value lines.
read_pins.sh checks that each is set once, to a full 40-character
commit SHA, and prints them in the form GITHUB_OUTPUT takes. The
setup-mavlink-tooling action reads them through it, checks out
mavlink/mavlink (definitions only) and pymavlink at those commits with
pinned actions and no persisted credentials, installs the generator's
dependencies and places the dialect beside the upstream definitions;
the generate job runs it from a root checkout. publish_c_library.sh
records the pins in its commit from the same file, generate_messages.py
parses the file, and docs.yml and generate_c_lib.yml trigger on it,
so a bump touches one file. The CONTRIBUTING.md example for a pin
bump names it.

A test job runs the tests of the generate_c_lib scripts:
test_read_pins.py, and test_publish_c_library.py, which runs the
publish script against local git repositories. .gitignore skips the
__pycache__ folders that test runs leave.

Validation: 14 tests pass on Python 3.10.12 and 3.14.5: for
read_pins.sh valid pins, the repository's own file, a short SHA, a
missing pin, a pin set twice, a missing file and the usage error; for
the publish script unchanged headers, removed headers, the commit
message with both pins, merge commit titles, manual runs and the
usage error. generate_messages.py reads 87da370 from the new file,
and actionlint 1.7.12 and shellcheck 0.11.0 report nothing.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
publish_c_library.sh syncs the headers into the C library checkout
with rsync -a, whose quick check skips a file whose size and mtime, in
whole seconds, match. A regenerated version.h keeps its size, since
only the build date in it changes, so headers written in the same
second as the checkout are never published. This pull request's CI
rehearsal of the publish reported no change for a version.h that
differed.

rsync runs with --checksum and copies every file whose content
differs, at the cost of reading both copies of each of the 288
generated headers.

Validation: a new test gives a generated and a published header of
the same size the same mtime; it passes with --checksum and fails
without it. 15 tests pass on Python 3.10.12 and 3.14.5, and
shellcheck 0.11.0 reports nothing.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
mavgen stamps the generation date into every version.h, so headers
regenerated on another day differ from the published ones in that
line alone, and with --checksum each such run publishes a commit
that changes nothing else. Merging the contributing guidelines
published 3c168c6 to the C library, which only bumps
MAVLINK_BUILD_DATE.

The publish check is git diff --cached --quiet -I
'^#define MAVLINK_BUILD_DATE ', so a change made only of build-date
lines publishes nothing, while any other change carries the new date
with it.

Validation: a new test publishes headers that differ from the
library's only in the build date and expects no commit; it fails
without -I. The same-second test rests on a same-size CRC change, so
it keeps covering --checksum. 16 tests pass on Python 3.10.12 and
3.14.5.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
The generate job checks out the C library with the deploy key and
pushes to it after running the generator: pip installs future and
lxml from PyPI at whatever version is current, and pymavlink's code
runs, all in the job that holds a key able to push to the C library.
Its actions are tags that can move, the checkout keeps its token in
.git/config, and the job has no timeout.

generate builds the headers and uploads its out/ directory as the
c-headers artifact, kept for a day, since it only has to outlive the
run. publish runs only on main, in the c-library environment, which
holds the deploy key and is limited to main: it checks out this
repository, downloads the artifact, checks out the C library with the
key and runs publish_c_library.sh, so no generator and no third-party
Python package runs there, and the published headers are the ones
generate built. It waits for the test job, so a publish script whose
tests fail never runs with the key. Every action is pinned by SHA,
download-artifact at v8.0.1, which fails on a digest mismatch by
default; the checkouts drop their credentials except the C library's,
which the push needs; and the jobs are bounded at 15, 5 and 10
minutes.

Validation: actionlint 1.7.12 reports nothing, and zizmor 1.30.1
reports no unpinned action and no persisted credential in the
workflow beyond the C library checkout's, waived inline for the push.
This pull request's generate job runs the upload; the download and
the push run on main.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
The workflow triggers on .github/scripts/**, so a change to any script
under it, whichever workflow runs it, regenerates the C headers and
reruns the publish tests for nothing.

It triggers on what its jobs read: military.xml, the workflow itself,
.github/mavlink-pins.env, the setup-mavlink-tooling action and
.github/scripts/generate_c_lib/**, which holds the scripts it runs
and their tests.

Validation: actionlint 1.7.12 reports nothing, and every file the
generate, test and publish jobs read lies under one of those paths.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
The site's build and deploy jobs use actions by tags that can move,
keep the checkout token in .git/config, have neither timeouts nor
names, and grant pages: write and id-token: write without saying
what needs them.

Every action keeps its major version, pinned by SHA: checkout v4.4.0,
setup-python v5.6.0, setup-node v4.4.0, upload-pages-artifact v3.0.1
and deploy-pages v4.0.5. The checkout drops its credentials, the jobs
are named and bounded at 15 and 10 minutes, and each permission says
what uses it.

Validation: actionlint 1.7.12 reports nothing, and zizmor 1.30.1
finds nothing in docs.yml.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
Nothing checks the workflows themselves: a wrong expression type, a
shell mistake in a run block, an unpinned action or a template
injection shows only when the workflow runs, if at all.

workflow_lint.yml runs on changes under .github, in two jobs. The
first fetches actionlint 1.7.12 and shellcheck 0.11.0 from their
releases with fetch_release.sh, which checks each archive against its
SHA-256 before extracting it, so every runner gets the same versions.
actionlint checks every workflow and runs shellcheck on each run
block, shellcheck checks .github/scripts/*/*.sh, and the job runs
fetch_release.sh's tests. The second runs zizmor 1.30.1 from PyPI
with the job token for its online audits (impostor commits,
known-vulnerable versions). .github/zizmor.yml disables the
self-repository rule: actionlint 1.7.12 rejects the $/ form that rule
asks for, and would stop checking those steps.

Validation: actionlint, shellcheck on the 5 scripts and zizmor,
online with the regular persona, report nothing on this commit. The 6
fetch_release.sh tests, gzip and xz archives, a checksum mismatch and
a missing release that extract nothing, the download removed either
way, and the usage error, pass on Python 3.10.12 and 3.14.5. Run as
the workflow runs them, against the real releases, both installs
verify and extract, and actionlint passes with the pinned shellcheck.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
An action pinned by SHA stays at that commit until someone bumps it,
including past a release that fixes a vulnerability.

dependabot.yml checks the github-actions ecosystem monthly, for the
workflows and for the setup-mavlink-tooling composite action, whose
directory dependabot scans on its own. Each opens one grouped pull
request, at least seven days after a release. The mavlink and
pymavlink pins in .github/mavlink-pins.env and the release pins in
workflow_lint.yml are not action references, so they move by hand.

Validation: GitHub validates dependabot.yml once it lands on the
default branch; nothing in this repository can run it.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
The publish job runs only on main, so a change to publish_c_library.sh
or to what it publishes shows only after merge, as a failed or wrong
publish.

On pull requests a rehearse-publish job downloads the c-headers
artifact and runs rehearse_publish.sh, which clones the C library
anonymously into a local bare repository, runs publish_c_library.sh
through a clone of that, and prints a Markdown summary for the step
summary: the commit a publish would push, with its files, or that the
library already holds these headers. The publish log goes to stderr.
The job holds no credentials, and fails if the C library cannot be
cloned.

Validation: test_rehearse_publish.py covers a change, whose summary
names the commit and its files while the library itself stays
untouched, headers the library already holds, an unreachable library
and the usage error; the 20 generate_c_lib tests pass on Python
3.10.12 and 3.14.5. Against the live C library at 3c168c6, with
headers generated from this tree at the pinned commits, it reports no
change. shellcheck 0.11.0 and actionlint 1.7.12 report nothing.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
The docs workflow deploys whatever VitePress builds and never looks at
the result: a dead link, a dead anchor or a sitemap URL outside the
site's base path goes live, and nothing confirms that the deployed
site serves its pages.

Before the upload, npm run docs:check runs docs/scripts/check_site.py
on the built site: every sitemap URL lies under the site address, and
no hand-written page has a dead link or anchor; dead anchors in the
generated message reference only warn, since the upstream generator
writes them. After the deploy, the deploy job checks out
.github/scripts/docs sparsely and without credentials, which needs
contents: read, and runs check_deployed_site.sh on the page URL: the
main pages and the sitemap load, with retries while the CDN catches
up, and the sitemap's first URL lies under the site and loads too.
The build job runs both scripts' tests, and the workflow triggers on
.github/scripts/docs/**.

Validation: 17 check_site.py tests, at 100% line and branch coverage,
and 5 check_deployed_site.sh tests against a site served from a
temporary directory pass on Python 3.10.12 and 3.14.5.
check_deployed_site.sh passes against
https://dronecode.github.io/mavlink-military/, and actionlint 1.7.12,
shellcheck 0.11.0 and zizmor 1.30.1 report nothing.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
A dialect change gets one check before merge: the C headers still
generate. Nothing validates military_extensions.xml, which is never
generated, nothing compiles the headers, and nothing loads the
Python bindings.

validate_dialect.yml runs on every pull request and push to main, and
writes and publishes nothing. Its jobs validate both XML files
against pymavlink's mavschema.xsd with xmllint, generate C with
--strict-units and type-check the headers with gcc at -Wall -Wextra
-Werror, and generate Python with --strict-units and check the module
with check_python_bindings.py: it loads, and every message
military.xml defines is in its mavlink_map, without which a frame
decodes as MAVLink_unknown. A script-tests job runs the tests of the
workflow's scripts. xmllint installs only when the runner lacks it,
through sudo sh -c, so a failed apt-get stops the step under bash -e.

Validation: on this tree both files validate, the headers compile at
-Werror, and check_python_bindings.py reports "261 messages in the
generated map, all 26 dialect messages present" with the pinned
pymavlink 19880422. Its 7 tests pass at 100% line and branch coverage
on Python 3.10.12 and 3.14.5. With stub sudo and apt-get, the xmllint
install runs only when xmllint is missing and exits 100 when apt-get
fails. actionlint 1.7.12 and zizmor 1.30.1 report nothing.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
The allocation rules live in README.md, IDMAPPING.md and the
military.xml header, and nothing enforces them: a message outside the
shared window, a shared ID in the private block, a misnamed field or
an IDMAPPING.md that disagrees with the XML passes unless a reviewer
spots it.

check_dialect_policy.py checks the documented policy on the Python
standard library alone: the 53000-53899 shared window and the
53900-53999 private block, MAVLink naming conventions, duplicate IDs
and names, collisions between the template and the shared dialect,
and IDMAPPING.md against military.xml. Every message is listed once,
in "Shared messages" or in a family's own "## <family> messages"
table, at the ID military.xml gives it (New ID in a table that maps
development IDs, ID in a table of new messages); every MAV_CMD entry
is in "MAV_CMD entries"; nothing listed is missing from the XML; and
no ID in use lies inside a reserved block. Each finding names its
file and line, and under GitHub Actions it is an error annotation on
that line.

Validation: 24 tests cover each rule and its findings, a family
table, a message listed twice and a family table without its columns
among them, at 100% line and branch coverage on Python 3.10.12 and
3.14.5. The check passes on this repository, and on a dialect whose
IDMAPPING.md lists a family of new messages in its own table.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
TSC21 added 2 commits October 2, 2026 19:09
A peer whose CRC_EXTRA for a message differs from the sender's drops
that message without an error, and nothing shows a reviewer which
messages a change breaks.

check_wire_compat.py parses the base and head military.xml with
pymavlink's generator and compares, per message ID, the entry row
mavgen writes into MAVLINK_MESSAGE_CRCS: {msgid, CRC_EXTRA, min and
max length, flags, target_system and target_component offsets}.
Removed and renamed messages and CRC_EXTRA changes are breaking; row
changes with the same CRC_EXTRA, which extension fields make, and
additions are listed for information, each with its old and new row,
in the log and the step summary. The report only informs until
message IDs are frozen; WIRE_COMPAT_ENFORCE=1 makes it fail. The
script-tests job sets up pymavlink, which these tests import.

Validation: 13 tests at 100% line and branch coverage pass on Python
3.10.12 and 3.14.5. Against main this tree reports 0 changes and 26
messages untouched; against a revision that adds target fields to
ESAD_ARMING it reports CRC_EXTRA 153 -> 3 with the rows
{53031, 153, 15, 15, 0, 0, 0} -> {53031, 3, 33, 33, 3, 12, 13}.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
CRC_EXTRA leaves out changed enum values, removed enum entries and
extension fields that are removed, retyped or moved, so the wire
report cannot see them.

The wire-compat job runs mavlink's own scripts/check_api_break.py at
the pinned MAVLINK_REF, which setup-mavlink-tooling checks out with
the definitions, through run_api_break_check.sh: a break is a
warning, or an error under WIRE_COMPAT_ENFORCE=1, and an exit without
the checker's "Name removals/changes detected" line is the tool
failing, an error either way. The checker's output goes to the log
and the step summary.

Validation: 8 tests with stand-in checkers, a clean run, a break with
and without enforcement, a crash under both settings, stderr outside
GitHub Actions, the summary and its pointer, and the usage error,
pass on Python 3.10.12 and 3.14.5. The real checker at 87da370, on
one enum value moved 4 -> 40, gives the warning with exit 0, and the
error with exit 1 when enforcing; on this pull request it prints "No
XML files changed.". shellcheck 0.11.0, actionlint 1.7.12 and zizmor
1.30.1 report nothing.

Assisted-by: Claude-Code:claude-opus-5-5
Signed-off-by: TSC21 <nmarques@riis.com>
@TSC21
TSC21 force-pushed the ci/dialect-validation-suite branch from 3668d62 to b2b9176 Compare October 2, 2026 18:10
@TSC21

TSC21 commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

@mrpollo I regrouped the history so each commit is either a fix to what main has or one new check, and moved the scripts into a folder per workflow, with their tests in tests/, which that workflow runs. The IDMAPPING.md check also accepts a family's own ## <family> messages table. The SHAs in my replies point at the new commits.

@TSC21
TSC21 requested a review from mrpollo October 3, 2026 00:12
Comment thread .github/scripts/workflow_lint/fetch_release.sh Outdated
@mrpollo
mrpollo merged commit 1b83985 into main Oct 9, 2026
14 of 16 checks passed
@mrpollo
mrpollo deleted the ci/dialect-validation-suite branch October 9, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants