ci: adopt the org-wide reusable Rust workflow by JustinKovacich · Pull Request #13 · luminartech/simple_doip

JustinKovacich · 2026-09-09T18:00:29Z

Stacked on #12 — merge order: #11 → #12 → this. --base is
fix/server-tracks-tester-logical-address, so the diff here is only this PR's
two commits.

What

Replaces this repo's seven hand-rolled CI jobs with a thin caller for
luminartech/rust_workflow/.github/workflows/rust-ci.yml@v1 — the same shape
uds_protocol and automotive_wire_codec use.

What the crate gains over the old ci.yml: pre-commit, a security audit
(cargo audit + cargo deny), coverage, miri, and
cargo-semver-checks — the last being the gate that stops an accidental
breaking release once the crate is public.

Publishing stays out of it

release.yml (from #11) owns publishing, through trusted publishing rather
than the registry token this workflow expects, so publish-crate: false. The
publish dry run stays on — it needs no credentials and checks the crate still
packages.

The four inputs that are off, and when each expires

Input	Why	Flips when
`run-semver-checks: false`	Diffs against the published baseline; there is none yet	The change that enables publishing
`run-fuzz-tests: false`	`cargo fuzz build` needs a `fuzz/` directory	#1 lands (it adds four targets)
`run-property-tests: false`	No `prop_` tests, so the filter selects nothing — and `cargo nextest` exits 4 on an empty selection, so the job would fail rather than skip	A property test exists
`publish-crate: false`	See above	Not intended to flip

Two overrides that are not temporary:

no-std-target: thumbv7em-none-eabihf rather than the workflow's
thumbv6m default — it's the target this crate's bare-metal support is
written against and examples/bare_metal_codec is built for. (thumbv6m
does build, for what it's worth; I checked.)
miri-args: '--lib' — tests/golden_vectors.rs reads its .hex
fixtures off disk, and miri's isolation refuses open. The library tests are
where the zero-copy decode paths worth checking for UB live.

MSRV is left unset so the job reads rust-version from Cargo.toml and cannot
drift from what the manifest promises.

Commit 1 is the prerequisite: 12 lints and 2 typos

The shared workflow lints with --all-targets and with
--no-default-features; this repo's CI ran clippy over the library with all
features on. Twelve findings sat in that gap, all in test code or behind a
feature combination nothing ever linted — six assert!(a == b) → assert_eq!,
two truncating as casts in test fixtures, a missing #[must_use], an
unchecked Duration subtraction, two doc comments needing backticks on DoIP.

typos found two real ones, and one matters: diagnotics was in a
user-visible warning string in logical_address.rs, so it would have shipped
to anyone using the crate. recieve was in a doc comment.

It also caught something my earlier audit missed: a test doc comment named an
internal MicroVision application as its example of a tester holding the
TCP_DATA slot. This repository is public, so it now says "a diagnostic tool
already polling the same ECU" — which is what the test actually exercises. (My
branding sweep grepped for the company and sensor names but not the
application's, which is how it slipped through.)

How it was tested

Every gate the caller enables, run locally:


`cargo fmt --all -- --check`	ok
clippy `--all-targets --all-features` (pedantic)	ok
clippy `--no-default-features` (pedantic)	ok
clippy `--no-default-features --features alloc`	ok
`cargo build --release --all-features`	ok
no-std build, `thumbv7em-none-eabihf` (± `alloc`)	ok
`cargo doc` with `RUSTDOCFLAGS=-D warnings` + doctests	ok
`cargo +1.88 build --all-features` (MSRV)	ok
`cargo test --all-features`	ok
`cargo publish --dry-run`	ok
miri, exactly as the job runs it (`--lib`)	ok — 21 tests, no UB
`pre-commit run --all-files`	ok
`typos`	ok

cargo deny and cargo audit aren't installed on this machine, so this PR's
own run is their first check — the deny.toml is uds_protocol's, and its
allow-list may need widening for a dependency it doesn't share.

Scaffolding

.pre-commit-config.yaml, .typos.toml and deny.toml, copied from
uds_protocol. Two hooks the siblings run are deliberately absent, with the
reasons in the config: mdformat would mangle this crate's markdown tables
and reference-style links, and check-json has no strict JSON to check here
(the only JSON is .vscode/launch.json, which is JSONC — and excluding it
instead makes check-hooks-apply fail a hook that matches nothing).

First run failed three jobs — and finding out was the point

The three that failed are exactly the three that could not be checked locally,
and two shared one root cause I had missed.

rust-toolchain.toml pinned channel = "stable", and a directory-local
toolchain file overrides whatever toolchain CI installs — rustup default
does not beat it. So every job whose purpose is to run a different toolchain
was either failing or lying:

Miri failed: the job installs nightly with the miri component, then the
bare cargo miri test resolved to stable, which has no miri.
The MSRV check never checked the MSRV. It installs 1.88 and runs
cargo build, which the file redirected to stable. That is not a regression
from this PR — the previous hand-rolled ci.yml used the same pattern, so
the MSRV gate has been decorative for as long as the file existed. 1.88
does hold; cargo +1.88 verifies it explicitly.
Pre-commit failed on the file's components = ["clippy", "rustfmt"]:
rustup tried to add clippy to a runner whose stable toolchain already ships
bin/cargo-clippy and refused with a file conflict.

Neither uds_protocol nor automotive_wire_codec carries a toolchain file, so
removing it also stops this repo being the odd one out. CONTRIBUTING.md now
says the absence is deliberate and why.

Worth being explicit about my own verification miss: every local check
passed because cargo +nightly miri and cargo +1.88 build name a toolchain
explicitly, which does beat the file. The workflow runs bare cargo. I was
testing a different thing than CI was.

Security Audit was a genuine finding, not a config problem. cargo audit
reported three vulnerabilities in the committed lockfile, all from versions
years behind what the manifest already permits:

Crate	Locked	Advisory	Patched
`bytes`	1.4.0	RUSTSEC-2026-0007	>= 1.11.1
`mio`	0.8.8	RUSTSEC-2024-0019	>= 0.8.11
`tracing-subscriber`	0.3.19	RUSTSEC-2025-0055	>= 0.3.20

cargo update takes them to 1.12.1, 1.2.3 and 0.3.23 and moves 53 other
packages — tokio 1.30.0 → 1.53.1, anyhow 1.0.75 → 1.0.104 among them,
clearing three unsound advisories that were only warnings. The changelog gets
a Security entry, since a consumer deciding whether to upgrade should see it.

The real risk in a 56-package update is the MSRV, which is now actually
enforced. cargo +1.88 builds the refreshed graph with --all-features and
with --no-default-features.

Full battery re-run green after the fixes, including miri through
rustup run nightly (what the job now does), pre-commit run --all-files, and
cargo publish --dry-run.

One commit is mislabeled. chore(deps) also carries the
rust-toolchain.toml deletion — the git rm was already staged when I wrote
its message, which is why that message points at a "next commit" that does not
contain it. Left as-is rather than rewritten; the reason it was removed is in
the following commit and in CONTRIBUTING.md.

Relationship to #1

This overlaps @gavin-dunlap-luminar's #1, which proposed its own 297-line
main.yml — written before rust_workflow was tagged v1. The scaffolding
that PR adds is still wanted, the fuzz targets especially, since they're
what unblocks run-fuzz-tests here. Its workflow file is superseded by this
caller. Worth a conversation rather than a close.

Review status

Not reviewed by anyone yet. Draft.

Release automation (added after the original review pass)

This PR now also flips use-release-plz: true, so the reusable workflow's
Release-plz PR and Release-plz Release jobs take over versioning, the
changelog, tags, GitHub releases and the crates.io publish. That input also
disables the workflow's tag-gated Release & Publish job, so the two paths
cannot both fire. #11 carries the matching release-plz.toml and deletes the
old cargo-release tooling.

Wiring copied from automotive_wire_codec:

contents: write + pull-requests: write on the called workflow — the test
jobs downscope themselves back to contents: read; this grant exists only
so release-plz can push the release-PR branch and the release tag.
cargo-registry-token, release-plz-app-id, release-plz-app-private-key
passed through as secrets.
publish-repository: luminartech/simple_doip, so a fork cannot publish
under this crate's name.

Repo configuration this needs, none of it in this diff


`CARGO_REGISTRY_TOKEN`	secret; does not exist yet
`RELEASE_PLZ_APP_ID` / `RELEASE_PLZ_APP_PRIVATE_KEY`	secrets; do not exist yet
`crates-io` environment	this repo has no environments at all; both siblings have this one, and the reusable workflow gates each release job on it

Until those exist the release-plz jobs run and no-op rather than publishing,
which is why merging this is safe.

run-semver-checks stays off — there is still no published baseline to diff
against. The comment now also records what it will not do once enabled: it
reads the public API surface, so a changed signature is caught and a changed
behavior is not.

`luminartech/rust_workflow` lints with `--all-targets` and with `--no-default-features`, where this repo's own CI ran clippy over the library with all features on. Twelve findings sat in the gap, all in test code or behind a feature combination that was never linted: - Six `assert!(a == b)` in `src/messages/mod.rs` become `assert_eq!`, which also means a failure prints both values instead of just `false`. - Two truncating `as` casts in `bare_metal_entity.rs` test fixtures become `try_from(..).expect(..)`. Neither can fail at the sizes the fixtures use -- which is the point: a truncating cast would hide it if that changed. - `LogicalAddress::is_valid_client_address` gains `#[must_use]`. Only the no-default-features lint reached it, so this repo never saw it. - A `Duration` subtraction in the interleaving test becomes `saturating_sub`. The instants are recorded in order so it cannot underflow, but an underflow would panic the test rather than fail its assertion, which reads as a hang. - Two doc comments get backticks on `DoIP`, which rustdoc otherwise reads as an unlinked item. `typos` found two real ones, both now fixed. `diagnotics` was in a **user-visible warning string** in `logical_address.rs`, so it would have shipped to anyone using the crate; `recieve` was in a doc comment. The three it flagged that are deliberate -- `pendings` (a run of DoIP "response pending" messages), `catch-alls`, `mis-mapped` -- are allowed in `.typos.toml`. One test doc comment named an internal MicroVision application as its example of a tester holding the `TCP_DATA` slot. This repository is public, so it now says "a diagnostic tool already polling the same ECU" -- which is also what the test actually exercises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Replaces this repo's seven hand-rolled jobs with a thin caller for `luminartech/rust_workflow/.github/workflows/rust-ci.yml@v1`, the same shape `uds_protocol` and `automotive_wire_codec` use. What the crate gains over the old `ci.yml`: pre-commit, a security audit (`cargo audit` + `cargo deny`), coverage, miri, and `cargo-semver-checks` -- the last being the gate that stops an accidental breaking release once the crate is published. Four inputs are deliberately off or overridden, each for a reason that will expire: - `publish-crate: false`. `release.yml` owns publishing, through trusted publishing rather than the registry token this workflow expects. The publish *dry run* stays on -- it needs no credentials and checks the crate still packages. - `run-semver-checks: false`. It diffs against the published baseline, and there is none until the first release. Flip it in the change that enables publishing. - `run-fuzz-tests: false`. `cargo fuzz build` needs a `fuzz/` directory. #1 adds four targets; this flips when that lands. - `run-property-tests: false` with `unit-test-filter: 'all()'`. There are no `prop_` tests, the filter would select nothing, and `cargo nextest` exits 4 on an empty selection -- so the job would fail rather than skip. And two overrides that are not temporary: `no-std-target: thumbv7em-none-eabihf`, the target this crate's bare-metal support is written against and `examples/bare_metal_codec` is built for, rather than the workflow's `thumbv6m` default; and `miri-args: '--lib'`, because `tests/golden_vectors.rs` reads its fixtures off disk and miri's isolation refuses `open`. The library tests are where the zero-copy decode paths worth checking for UB live, and 21 of them pass under miri. MSRV is left unset so the job reads `rust-version` from Cargo.toml and cannot drift from what the manifest promises. Scaffolding the shared workflow expects: `.pre-commit-config.yaml`, `.typos.toml`, and `deny.toml` copied from `uds_protocol`. Two hooks the siblings run are deliberately absent, with the reasons in the config -- mdformat would mangle this crate's markdown tables and reference-style links, and check-json has no strict JSON to check here. Every enabled gate was verified locally, including miri and pre-commit; `cargo deny` and `cargo audit` are not installed on this machine, so this PR's own run is their first check. This overlaps #1, which proposed its own 297-line `main.yml` before `rust_workflow` was tagged. The scaffolding that PR adds -- fuzz targets especially -- is still wanted; its workflow file is superseded by this caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`cargo audit` fails on the committed lockfile with three vulnerabilities, all from versions that had drifted years behind what the manifest allows: - `bytes 1.4.0` -- RUSTSEC-2026-0007 (patched in >= 1.11.1) - `mio 0.8.8` -- RUSTSEC-2024-0019 (patched in >= 0.8.11) - `tracing-subscriber 0.3.19` -- RUSTSEC-2025-0055 (patched in >= 0.3.20) `cargo update` takes them to 1.12.1, 1.2.3 and 0.3.23, and moves 53 other packages -- `tokio 1.30.0` to 1.53.1 and `anyhow 1.0.75` to 1.0.104 among them, which clears three `unsound` advisories that were only warnings. No manifest requirement changes; every one of these was already permitted. The risk in a 56-package update is the MSRV, which this repo has never actually verified (see the next commit). `cargo +1.88` builds the refreshed graph with `--all-features` and with `--no-default-features`, so 1.88 still holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two things the previous commit changed without saying so. Its message describes only the lockfile refresh, but the `rust-toolchain.toml` deletion was already staged when I wrote it, so both landed together and the reason for the removal went unrecorded. Rather than rewrite the commit, the reason goes where someone will look for it. `CONTRIBUTING.md` now states that the absence of a toolchain file is deliberate: a directory-local `rust-toolchain.toml` overrides whatever toolchain CI installs, which turned the miri job into a failure and the MSRV job into a no-op that built with stable. It also says to name a toolchain explicitly when checking the floor, which is the trap I fell into -- `cargo +nightly` and `cargo +1.88` beat the file, so every local check passed while CI could not. The lockfile refresh gets a `Security` entry in the changelog, because three of the versions it moved off had RUSTSEC advisories against them and a consumer reading the changelog to decide whether to upgrade should see that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The repository merges by squash with `squash_merge_commit_title: PR_TITLE`, so the PR title is the commit subject that lands on `main` -- the individual commits in a branch never appear there. The commit history is the changelog, so an unconventional PR title becomes a permanent unconventional changelog entry, and nothing was checking it. Both `uds_protocol` and `automotive_wire_codec` have carried this workflow; this repo did not. Only the title job is ported. The sibling repos pair it with a description lint that requires `## Issue URL` and `## Testing` sections from a pull-request template, and a `No Issue` label as the escape hatch. This repo has neither, so that job would fail every open PR. simple_doip#1 adds the templates -- the description lint belongs with them. The action is pinned by commit SHA rather than tag, matching how the shared workflow pins third-party actions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Turns on `use-release-plz`, which activates the reusable workflow's release-plz PR and release jobs and disables its tag-gated `Release & Publish` job, so only one release path can fire. Wiring matches `automotive_wire_codec` exactly: `contents: write` + `pull-requests: write` on the called workflow (the test jobs downscope themselves back to read), the registry token and the release-plz GitHub App credentials passed as secrets, and `publish-repository` pinned so a fork cannot publish under this name. Repo configuration this needs before it can actually release, none of which is in this diff: `CARGO_REGISTRY_TOKEN`, `RELEASE_PLZ_APP_ID` and `RELEASE_PLZ_APP_PRIVATE_KEY` as secrets, and a `crates-io` environment -- the reusable workflow gates both release jobs on one, and this repo has no environments at all today while both siblings do. Until then the release-plz jobs run and no-op rather than publishing. `run-semver-checks` stays off, but the comment now says what it will and will not do once the first publish gives it a baseline: it diffs the public API surface, so a changed signature is caught and a changed behavior is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@gavin-dunlap-luminar

…mation (#11) ## What The two things standing between `simple_doip` and a first crates.io release: the license texts, and a way to cut a release. ## Why The crate turned out to be much closer to publishable than the Phase A notes suggested. Checked against the live repo: - `Cargo.toml` on `main` already carries the **full publish metadata** — `description`, `license`, `repository`, `readme`, `keywords`, `categories`, `exclude`. No `publish = false`. - CI already runs **`cargo publish --dry-run`** in the `package` job, so publishability is verified on every run. - The only first-party dependency, **`automotive-wire-codec 0.3.0`, is already on crates.io** (published 2026-07-17). That was the hard prerequisite — `cargo publish` refuses a crate whose dependencies aren't on a registry. - The name **`simple_doip` is unclaimed** on crates.io (both spellings 404). - DFT's submodule pointer is **identical** to `main` (0 ahead / 0 behind, both at 0.5.2), so there's no fork to reconcile first. What was actually missing was the license files and the release plumbing. ## Commits 1. **`docs(license)`** — adds `LICENSE-MIT` + `LICENSE-APACHE`, copied verbatim from `uds_protocol` so the protocol libraries carry identical wording. The manifest has declared `MIT OR Apache-2.0` for a while with no text in the repo; GitHub's API reported `license: null` for exactly that reason. Both files land inside the published `.crate`. 2. **`build(release)`** — originally cargo-release plus a hand-rolled tag-driven `release.yml`. **Superseded:** see "Release tooling" below. Both files are deleted; `release-plz.toml` replaces them. ## Release tooling: release-plz, matching the sibling repos `uds_protocol` and `automotive_wire_codec` both release with release-plz, off a `release-plz.toml` that is **byte-identical between them**, and neither has a release workflow of its own — the jobs live in `luminartech/rust_workflow`. This repo was the odd one out. It now carries the same config verbatim, and `release.toml` + `.github/workflows/release.yml` are gone. The switch itself (`use-release-plz: true`, permissions, secrets) lands one layer up in **#13**, which owns `main.yml`. What this changes in practice: **no version is bumped by hand and no tag is pushed by hand.** A push to `main` maintains an open release PR; merging that PR publishes, tags, and cuts the GitHub release. ### The one real regression The deleted `release.yml` published through **crates.io trusted publishing** — OIDC, no stored registry credential anywhere. The reusable workflow's release-plz job takes a `CARGO_REGISTRY_TOKEN` secret instead. Matching the org is the point of this change, so that's the trade accepted here, but it is a step backwards on that one axis and the right place to fix it is `rust_workflow`, not this repo: one `id-token: write` plus a `crates-io-auth-action` step would give trusted publishing to every repo on the shared workflow at once. **Merging this still publishes nothing** — the release-plz jobs need secrets and a `crates-io` environment this repo does not have yet (listed in #13). ## `CONTRIBUTING.md` gains a Releases section Because the repo squash-merges with `squash_merge_commit_title: PR_TITLE`, the **PR title** is the commit subject on `main` — so PR titles, not branch commits, are what release-plz computes the version from. The new section spells that out with the type-to-bump table, and calls out the trap that bit this stack: pre-1.0 a breaking change is a *minor* bump, `!` is the only thing that produces one from a `fix:`, and `cargo-semver-checks` reads the API surface so it will not catch a behavioral break for you. ## How it was tested - `cargo publish --dry-run` — packages 77 files / 106.6 KiB compressed, verifies the packaged crate, resolves `automotive-wire-codec 0.3.0` from crates.io. **This is the proof that 0.5.2 is publishable as-is.** - `cargo package --list` — confirms `LICENSE-APACHE` and `LICENSE-MIT` are in the `.crate`. - `cargo fmt -- --check` and `cargo clippy --all-features -- -D warnings -Dclippy::pedantic` — both clean. - `release-plz.toml` parses, and its values `diff` clean against `uds_protocol`'s and `automotive_wire_codec`'s copies. - `cargo package --list` confirms `release-plz.toml` and `rust-toolchain.toml` stay out of the packaged crate (the `exclude` entry was updated with the rename). - The three cargo-release 0.25 behaviors this depends on were verified empirically during Phase A: the `publish` subcommand overrides `publish = false`; `--allow-branch '*'` is required on a tag-push (detached-HEAD) run; and `--version '^0.25'` is needed because cargo rejects a bare `--version 0.25`. ## Still needs a decision (not in this PR) 1. **crates.io owner + token.** `simple_doip` has no repo secrets at all. The precedent next door is a personal account — both `uds_protocol` and `automotive-wire-codec` are owned on crates.io by `zheylmun`. A crates.io **team owner** (`github:luminartech:<team>`) would be the durable answer. `CARGO_REGISTRY_TOKEN` then goes in this repo's secrets. (Worth noting: `uds_protocol` is at 0.1.0 on `main` but still 0.0.2 on crates.io, and has no `CARGO_REGISTRY_TOKEN` in its secrets — its merged release tooling has never actually published either.) 2. **First-publish version.** 0.5.2 as-is works. Worth being deliberate, because the version number is spent permanently once published. 3. **`luminartech/rust_workflow`.** The org-wide reusable workflow (`rust-ci.yml@v1`) already does tag-gated publish, a `publish-repository` fork guard, a `publish-environment` approval gate and `cargo semver-checks` — `uds_protocol` and `automotive_wire_codec` are both thin callers of it. Adopting it here would replace this repo's hand-rolled `ci.yml` *and* this `release.yml`, and it needs scaffolding this repo doesn't have yet (pre-commit config, `deny.toml`, `.typos.toml`, `.config/nextest.toml`, fuzz targets) — much of which is what #1 adds. That migration is worth doing, but it's a bigger change than unblocking a first publish, so this PR deliberately doesn't touch `ci.yml`. 4. **#1 and #2.** #2 (mine) is superseded by this PR — `main` grew the publish metadata it was adding. #1 (@gavin-dunlap-luminar) is a different question: its hand-rolled 297-line `main.yml` is superseded by `rust_workflow`, but its scaffolding is a prerequisite for adopting it. Both are currently `CONFLICTING` against `main`. ## Pre-publication audit (commits 3-10) A pass over the whole repo for things that would ship visibly wrong. Commits 3-5 needed no decisions: 3. **`build(cargo)`** — **docs.rs would have published a nearly empty API reference.** `default = []`, and docs.rs builds default features only, so `client`, `server`, `codec`, `alloc` and `std` — most of what the README points a reader at — would have been absent from the docs page. Fixed with `[package.metadata.docs.rs] all-features = true`; verified that the `client`, `server` and `message_codec` modules now render. The same commit stops shipping `release.toml` and `rust-toolchain.toml` inside the `.crate` (77 files → 75). 4. **`chore(vscode)`** — all six debug configurations passed `--package=doip`, the crate's pre-rename name, so every one of them failed. 5. **`chore(deps)`** — `futures-util 0.3.28` is yanked on crates.io, so every `cargo publish` run warned about it; moved to 0.3.34. Lockfile only. Commits 6-8 then closed the documentation gaps the audit turned up, and 9-10 settled how the crate actually gets published: 6. **`docs`** — adds `CHANGELOG.md` (110 lines), reconstructed from the release history, so the crate does not arrive on crates.io with the 0.2.0 zero-copy core, the 0.4.0 server API break and the 0.5.x client fixes behind it and no record of any of them. 7. **`docs`** — adds `SECURITY.md` (a stated way to report a vulnerability in an automotive diagnostics library) and `CONTRIBUTING.md` (a stated position on outside contributions, plus the Releases section described above). 8. **`docs(readme)`** — replaces the 45-line "Status" known-gaps inventory with a "Scope and limitations" section. The README is the crate's front page on crates.io; it is now 144 lines. 9. **`ci(release)`** — moved the hand-rolled `release.yml` onto crates.io trusted publishing. **Superseded** by commit 10, which deletes that file. 10. **`build(release)`** — hands versioning and publishing to release-plz. See "Release tooling" above, including the trusted-publishing regression this trade accepts. Also updated the repo's GitHub **description** (was "Crate for Rust DoIP") to match the manifest, and added **topics** (`doip`, `iso13400`, `automotive`, `diagnostics`, `no-std`, `rust`). ### Clean Worth recording, since the audit went looking: no Luminar branding anywhere except the org name in the repository URL; no internal hostnames, IP addresses, ticket numbers or names in any file; no `dbg!`/`println!`/`#[allow(...)]` in `src/`; `#![warn(missing_docs, missing_debug_implementations)]` is on; all 16 symbols the README names exist; examples are clean. ### Still open, needs a decision Four bullets that stood here are now closed: `CHANGELOG.md`, `SECURITY.md` and `CONTRIBUTING.md` are added by commits 6-7, the README's known-gaps inventory is replaced by commit 8, and **`v0.5.2` is now tagged and pushed at `304d014`**, so the tag history no longer has a hole in front of a first publish. What is left: - **Six naked TODOs in `src/`** (`connection.rs`, `routing_activation_request.rs`, and four in `server.rs`), notably `server.rs:631` (`LogicalAddress(0x0000), // TODO fix this constant`) — the same defect the README documents as `ClientConnectionInfo::logical_address` always being `0x0000`. Fixed one layer up in #12; the rest are fix-or-drop. - **`strum` is a major behind** (0.27 vs 0.28.0); bumping likely forces a matching bump in dft's workspace. - **CI has no cargo-audit, cargo-deny, cargo-semver-checks or typos check.** semver-checks is the one that matters once published — it's what stops an accidental breaking release. All four come free with `rust_workflow`, which #13 adopts. One audit finding was **withdrawn**: `ARCHITECTURE.md` §7.1 is marked "RESOLVED in 0.4.0" while sitting under "Known issues and deferred work", which looked stale — but §7's own preamble states the policy deliberately ("Resolved entries are kept because the analysis that led to the fix is still the fastest way to understand the shape the API ended up with"). Left alone. Renumbering would also have broken the `§7.2` references in `src/client_inner.rs` and `tests/integration_test.rs`. ## Review status Marked ready for review; not reviewed by anyone yet. CI is green — 7 pass. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

@gavin-dunlap-luminar

**Stacked on #13 — merge order: #11 → #12 → #13 → this.** ## Issue URL Closes #1 (supersedes it — see below). Files #15. ## What Brings simple_doip#1's test suites forward instead of rebasing that branch, and turns on the two CI gates that were off for want of them. #1's merge base is 2026-04-03, before the `no_std` migration, the error-taxonomy refactor and the 0.4.0 server break. Its 26 property tests and 3 of its 4 fuzz targets drive `Message::read`, `Message::write` and `Payload::read` — **none of which exist on `main`**, all removed in 0.2.0. A rebase resolves textually (12 conflicts on the first of 7 commits) and then fails to compile. The properties were sound; only the calls were stale. | | | |---|---| | **`tests/property.rs`** | 24 properties ported to `Encode`/`Decode`, all passing on the first run | | **`fuzz/`** | 4 cargo-fuzz targets, 3 rewritten and 1 unchanged | | **`.github/` templates** | PR + bug/epic/task, verbatim from #1 — no API coupling, no porting needed | | **`main.yml`** | `run-property-tests` and `run-fuzz-tests` both on | **24 of 26 properties land.** The two that don't are byte round trips through serde, and this crate has no serde dependency. They live in `tests/property.rs` rather than `#[cfg(test)]` modules inside `src/` as the original did: proptest needs `std`, the library is `no_std`, and an integration target gets `std` with no conditional-compilation gymnastics. The cost is only reaching the public API, which is all these properties touch. ## What they add over `golden_vectors.rs` The golden fixtures pin the exact bytes the crate emits, so they catch the wire format changing. These check `encode` and `decode` agree with **each other** across the whole input space, which catches a field written in one order and read in another. Neither finds a misreading of the standard that both directions share symmetrically — that's what the fixtures are for. ## `fuzz_roundtrip` found a real bug in under a second **#15**: `Message::encode` can emit a frame that `Message::decode` rejects. `decode` takes exactly `header.payload_length` bytes and lets `Payload::decode` consume fewer without complaint; the decoded `Message` keeps the declared length; `encode` then writes that stale length beside a payload of its real size. A NACK frame declaring 5 body bytes and carrying 1 decodes fine, re-encodes to 9 bytes with the header still claiming 5, and fails to re-decode with `Incomplete { needed: 5, available: 1 }`. The target **skips that specific shape**, with #15 referenced at the check, so it keeps hunting field-order asymmetries without asserting a property the crate violates today. The skip comes out with the fix, which is stacked on this PR. ## Testing | | | |---|---| | `cargo test --all-features` / `--no-default-features` | pass | | the 24 properties | pass | | `cargo nextest run -E 'test(~prop_)'` | **selects exactly 24**, not zero — the `exit 4` failure mode | | `cargo fuzz build` (real cargo-fuzz, nightly) | pass | | 8s per fuzz target | 5.4M / 6.1M / 1.6M / 4.1M execs, all clean | | `clippy --all-targets --all-features -Dclippy::pedantic` | clean | | `clippy --no-default-features -Dclippy::pedantic` | clean | | `cargo fmt --all --check`, `pre-commit run --all-files` | clean | | `cargo publish --dry-run` | pass — `fuzz/` does not enter the packaged crate | `unit-test-filter` stays `all()` rather than excluding `prop_`: the unit job measures coverage, and coverage should describe the whole suite. The property job re-runs the same 24 under their own name for a readable signal. ## Not included The **PR description lint** the sibling repos pair with the templates. It requires `## Issue URL` and `## Testing` sections and would fail every currently open PR, including this one's stack-mates. A `No Issue` label now exists as its escape hatch; the job belongs in a follow-up once bodies conform. ## On #1 Its `main.yml` predates `rust_workflow@v1` by six weeks, so it wasn't a misjudgment — the better option didn't exist. Its scaffolding was independently written in #13 before I'd read his closely enough, which is on me. What was uniquely valuable was the tests, and they're here. Recommend closing #1 with a pointer to this PR rather than leaving it to rot; @gavin-dunlap-luminar is credited in the commit and in `tests/property.rs`. ## Note on the commit This is one commit, not the four its message describes — everything was already staged when I wrote the first one, so it swept the lot. Since the repo squash-merges with the body taken from the PR description, the rationale that would have been in those messages is above instead. ## Review status Not reviewed by anyone yet. Draft. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…sage (#17) **Stacked on #16 — merge order: #11 → #12 → #13 → #16 → this.** ## Issue URL Closes #15. ## What `Message::encode` could emit a frame that `Message::decode` rejects. `decode` takes exactly `header.payload_length` bytes and hands them to `Payload::decode`, which is not required to consume all of them. The decoded `Message` keeps the header verbatim, declared length included, and `encode` wrote that stale field beside a payload of its real size. Note that `encoded_size()` already disagreed with the header being written — it returns `Header::SIZE + payload.encoded_size()`, not the declared length. ```rust // A NACK body is one byte. This header claims five. let framed = [0x02, 0xFD, 0, 0, 0, 0, 0, 0x05, 0x03, 0, 0, 0, 0]; let (msg, _) = Message::decode(&framed).unwrap(); // accepted assert_eq!(msg.header.payload_length, 5); // preserved // re-encode -> 9 bytes, header still claims 5 Message::decode(&encode(&msg)); // Err(Incomplete { needed: 5, available: 1 }) ``` Anything that decodes a frame and re-emits it — a proxy, a replay tool, a logging fake, a test harness echoing what it received — was turning a malformed-but-accepted frame into a corrupt one on the wire. This crate's own `MessageCodec` encoder is on that path. ## The fix `encode` builds its header from `payload.encoded_size()` rather than trusting `self.header.payload_length`, so an encoded frame is always self-consistent regardless of how lenient `decode` is. **No input that is accepted today starts being rejected.** The consequence, documented on the method: for a frame that arrived with a mismatched declared length, `decode(encode(m)).header.payload_length` is the payload's real size rather than the length it arrived with. That is the point — but it is a visible behavior change, so it's in the changelog. A **well-formed frame is byte-identical**, which is why all 11 golden vectors still pass unchanged. That's also pinned as a test. `MessageError::PayloadTooLarge` covers the one fallible step — a payload too large for the `u32` length field, unreachable for a frame off the wire whose length was itself a `u32`. `MessageError` is `#[non_exhaustive]`, so adding it breaks nothing. ## Why not make `decode` strict instead That is the standards-correct complement — ISO 13400-2 has an entity answer an invalid payload length with NACK `0x04`, and `MessageError::PayloadLengthTooShort` sits unused for exactly this. But it's a redesign, not a fix: `Payload::decode` would have to report unconsumed bytes, and the identification requests **deliberately** discard their EID/VIN body (`ARCHITECTURE.md` §7.6), so a `0x0002` request carrying its six EID bytes would start being rejected outright rather than declined — breaking the UDP responder path. Recorded in `ARCHITECTURE.md` §7.6 as deferred rather than dropped. ## Testing | | | |---|---| | `tests/encode_consistency.rs` | 3 regression cases: overlong length on a fixed payload, nonzero length on a unit payload, and a well-formed frame encoding to the bytes it came from | | golden vectors | 11/11 pass **unchanged** — the fix cannot touch a frame whose declared length was already right | | `fuzz_roundtrip`, skip removed, idempotence asserted | **21,033,263 executions clean** | | other three fuzz targets | 6.4M / 8.1M / 1.7M clean | | full test suite, all features and none | pass | | clippy `--all-targets --all-features -Dclippy::pedantic`, and `--no-default-features` | clean | | `cargo doc` with `-D warnings`, `cargo fmt`, `pre-commit`, `cargo publish --dry-run`, MSRV 1.88 | pass | The fuzz target now asserts payload and payload-type equality plus **idempotence** (encode, decode, encode again → identical bytes) rather than whole-`Message` equality. Asserting full equality would assert the bug back into existence, since the declared length legitimately normalizes; idempotence buys back the field-order asymmetry detection that equality was providing. ## Also Corrects `PayloadLengthTooShort`'s message, which read "does match" where it meant "does not match" — a user-visible error string. ## Release bump: 0.6.0 This PR sits at the top of the stack, so it also carries `chore(release): v0.6.0` — the version the whole stack (#11 → #17) publishes as. It follows the same pattern as 0.5.2, whose bump was made inside #10's branch rather than by `cargo release` on main. Nothing in the stack breaks a signature: `MessageError` is `#[non_exhaustive]`, so `PayloadTooLarge` is additive, and `ClientConnectionInfo::logical_address` keeps its type and only starts carrying a real value. The bump is for the encode change in this PR — a caller that set a mismatched `payload_length` deliberately (a negative-test fake, a corpus generator, a proxy replaying what it saw) stops being able to emit that frame, with no compiler diagnostic anywhere. The CHANGELOG entry is marked **Breaking:** so the version and the section header tell the same story. `v0.5.2` is now tagged at the #10 merge on `main` (`304d014`), so that section has a comparison range and the release links run `v0.5.2...v0.6.0`. ## Review status Not reviewed by anyone yet. Draft. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

JustinKovacich self-assigned this Sep 9, 2026

JustinKovacich added the enhancement New feature or request label Sep 9, 2026

JustinKovacich added this pull request to stack #14 September 9, 2026 18:24

JustinKovacich marked this pull request as ready for review September 10, 2026 13:25

JustinKovacich requested a review from gavin-dunlap-luminar September 10, 2026 13:25

zheylmun approved these changes Sep 10, 2026

View reviewed changes

JustinKovacich force-pushed the ci/adopt-org-rust-workflow branch from 00f5868 to 2d820b1 Compare September 10, 2026 13:52

JustinKovacich and others added 6 commits September 10, 2026 09:54

JustinKovacich force-pushed the ci/adopt-org-rust-workflow branch from 2d820b1 to 27d5903 Compare September 10, 2026 13:54

JustinKovacich mentioned this pull request Sep 10, 2026

build(release): license texts, contributor docs, and release-plz automation #11

Merged

JustinKovacich merged commit 2b7f9fe into main Sep 10, 2026
19 checks passed

JustinKovacich deleted the ci/adopt-org-rust-workflow branch September 10, 2026 16:50

JustinKovacich mentioned this pull request Sep 10, 2026

chore(standardization): codec migration luminartech/simple_someip#153

Merged

Navigation Menu

Uh oh!

ci: adopt the org-wide reusable Rust workflow - #13

ci: adopt the org-wide reusable Rust workflow#13
JustinKovacich merged 6 commits into
fix/server-tracks-tester-logical-addressfrom
ci/adopt-org-rust-workflow

JustinKovacich commented Sep 9, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JustinKovacich commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Publishing stays out of it

The four inputs that are off, and when each expires

Commit 1 is the prerequisite: 12 lints and 2 typos

How it was tested

Scaffolding

First run failed three jobs — and finding out was the point

Relationship to #1

Review status

Release automation (added after the original review pass)

Repo configuration this needs, none of it in this diff

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JustinKovacich commented Sep 9, 2026 •

edited

Loading