From e9fb2f3a07ce8fcd92e57ade0832f208189f450c Mon Sep 17 00:00:00 2001 From: forkwright Date: Sun, 16 Aug 2026 21:55:28 -0500 Subject: [PATCH 1/2] ci(gate-attestation): stop the caller-level concurrency group self-cancelling main-push gates hybrid-gate.yml already declares its own concurrency group (workflow-name + commit sha on push, so distinct commits never collide) and its own comment forbids callers duplicating that key. This file's caller-level block was ref-keyed on push (the PR-number expression falls back to the ref, constant for main), so it reintroduced the exact class the reusable workflow's sha-on-push branch exists to prevent -- one layer up, where the reusable workflow's own fix could not see it. Confirmed live: main-push runs at 7004d734/cf1dd2da/9e8778b4/11f0978f show zero jobs ("workflow file issue"), and three dependabot squash-merges landing seconds apart (60f97de/b5c8071/bebbcd1) show the shared group cancelling the first two runs' full-gate-build mid-run -- main commits merging with no completed compile check. sphragis's gate-attestation.yml already carries the fix (no caller-level block) with the identical WHY. Also drops the pull_request `branches: [main]` filter: that filter matches a PR's BASE branch, so a stacked PR (base = another open PR's branch, not main) would match nothing and get zero gate runs -- not pending, absent. push stays scoped to main, where the base is always resolvable. Documents why doctest_cmd stays empty: every fenced block in a doc comment across the workspace is ```text``` or ```ignore```, never a bare ```rust``` block, so a doctest stage would compile and run zero examples. Part of #262 --- .github/workflows/gate-attestation.yml | 59 ++++++++++++++++++++------ .kanon-lint-baseline.toml | 8 +++- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/.github/workflows/gate-attestation.yml b/.github/workflows/gate-attestation.yml index 6984c9a..a9eea32 100644 --- a/.github/workflows/gate-attestation.yml +++ b/.github/workflows/gate-attestation.yml @@ -8,15 +8,16 @@ # from that build; ai-attribution runs unconditionally via the reusable # workflow's own centralized job. # -# NOTE (#262 residual gap, tracked -- not closed by this change): the -# reusable workflow's check-trailer job waives dependabot[bot] PRs from -# full-gate-build unconditionally (fleet-wide behavior, not overridable -# per-caller), and this workflow triggers on pull_request only (no -# push-to-main compile check). #262's "Done when" (a PR that fails -# `cargo check --workspace` cannot auto-merge; main compiles in CI on every -# push) is therefore only partially met: dependabot bumps still bypass the -# build, and there is no push-triggered compile check. Left as an explicit -# comment on #262 rather than silently claimed done. +# NOTE (#262 status): the two residual gaps this comment used to track are +# both closed. check-trailer no longer waives dependabot[bot] PRs from +# full-gate-build (the reusable workflow's own trailer step routes ANY +# untrailered tip, bot or human, to a real build -- confirmed on PRs #367/ +# #369/#370, each a full-gate-build pass before merge); and #353 added the +# `push: branches: [main]` trigger below so a squash-merged, trailer-less +# commit still gets built. #262's "Done when" is therefore mechanically met +# at the PR/push-trigger level -- what remained live was the caller-level +# `concurrency:` block documented below, which could cancel a queued +# main-push run before full-gate-build ever started. # # fmt/check/clippy/nextest command strings mirror .kanon-ci.toml's stage # commands verbatim (the local `kanon gate --stamp` SSOT) so a Gate-Passed @@ -56,8 +57,14 @@ name: Gate Attestation on: + # WHY no `branches:` filter here: a `branches:` filter on `pull_request` + # matches the PR's BASE branch, not its head. A stacked PR (base = another + # open PR's branch, not main) would match nothing and get zero gate runs + # -- not a pending check, an absent one, so nothing blocks it from merging + # broken. Two PRs shipped non-viable code through exactly that shape this + # week (kanon#3475). `push:` stays scoped to main below; that one IS the + # PR's base, always resolvable, and scoping it prevents duplicate runs. pull_request: - branches: [main] # WHY a push trigger and not just pull_request: a squash merge does not # carry the source commits' trailers, so no commit on main has a # Gate-Passed trailer -- check-trailer finds none and routes to @@ -67,9 +74,29 @@ on: push: branches: [main] -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true +# WHY no concurrency: block here: hybrid-gate.yml itself already declares +# `concurrency: group: ${{ github.workflow }}-${{ github.event_name == +# 'push' && github.sha || github.ref }}` and its own comment states callers +# must NOT also set a concurrency group with that same key, or the shared +# group self-cancels. This file's PRIOR block used +# `${{ github.event.pull_request.number || github.ref }}` -- ref-keyed on +# push, same failure class the reusable workflow's sha-on-push branch exists +# to prevent, just reintroduced one layer up. Confirmed live on #262 +# (reopened 2026-08-05): main-push runs at 7004d734/cf1dd2da/9e8778b4/ +# 11f0978f all show zero jobs ("workflow file issue"), and three dependabot +# squash-merges landing seconds apart on 2026-08-16 (60f97de/b5c8071/ +# bebbcd1) show the same group cancelling the first two runs' full-gate-build +# mid-flight -- main commits merging with no completed compile check, the +# exact defect #262 exists to close. sphragis's gate-attestation.yml carries +# the identical fix with the identical WHY, having hit this first. +# +# WHY the pull_request-time behavior does not regress: without a caller-level +# group, cancellation authority passes to the reusable workflow's own block. +# On a pull_request event `github.ref` resolves to `refs/pull//merge`, +# stable per PR number across every push to it -- so a superseded push to +# the same PR still cancels that PR's own prior run, same as before, without +# touching any other PR or a push-to-main run (different key entirely, per +# the ternary's sha branch). permissions: contents: read @@ -82,6 +109,12 @@ jobs: check_cmd: "cargo check --workspace --all-targets --features syntonia/hardware-serial --jobs 8" clippy_cmd: "cargo clippy --workspace --all-targets --jobs 8 -- -D warnings" nextest_cmd: "cargo nextest run --workspace --features syntonia/hardware-serial --build-jobs 8 --test-threads 8" + # WHY empty: nextest does not run doctests (kanon#3486). Left empty + # here, not skipped by oversight -- every fenced block in a doc + # comment across the workspace is ```text``` (ASCII diagrams) or + # ```ignore``` (kerykeion::message.rs), never a bare ```rust``` block, + # so `cargo test --doc` would compile and run zero examples. Revisit + # if a future doc comment adds a runnable example. doctest_cmd: "" system_packages: "libusb-1.0-0-dev libudev-dev protobuf-compiler" needs_fleet_repo_token: false diff --git a/.kanon-lint-baseline.toml b/.kanon-lint-baseline.toml index 30e9e3c..d83af68 100644 --- a/.kanon-lint-baseline.toml +++ b/.kanon-lint-baseline.toml @@ -1,7 +1,7 @@ [baseline] created = "2026-08-03" remove_after = "2026-11-01" -reason = "akroasis#261 lint-debt burn-down — errors first (vault plain-string-secret, kerykeion crypto indexing), both resolved. Remaining entries are deliberate exceptions, not deferred mechanical work: RUST/no-arc-mutex-anti-pattern (kerykeion/collector.rs) already uses tokio::sync::Mutex — the rule's own recommended async-safe primitive; converting further to RwLock needs a per-callsite read/write classification across 5 files, an architecture change outside a lint-driven edit. VOCAB/crate-name-collision + NAMING/no-fleet-collision (koinon) are a cross-repo naming call deferred to a fleet naming decision (akroasis#264). NAMING/no-owner-prefix (akroasis-server) needs a GNOMON-reviewed rename, an identity decision outside a mechanical fix. ARCH/substrate-dead-dep (sphragis) is a deliberately staged dependency awaiting the pinax reference-store integration and a cryptographic review (akroasis#172). TOML/missing-trailing-comma (.gitleaks.toml), RUST/doc-promised-observability (delivery.rs), CI/release-yml-missing-attestation (release-please.yml builds no artifacts to attest — release.yml already attests), and RUST/plain-string-secret (ListEntryReport.credential_type, a JSON category label not a secret) are confirmed lint-rule false positives. Entries clear only when the rule is fixed upstream or the cited decision resolves." +reason = "akroasis#261 lint-debt burn-down — errors first (vault plain-string-secret, kerykeion crypto indexing), both resolved. Remaining entries are deliberate exceptions, not deferred mechanical work: RUST/no-arc-mutex-anti-pattern (kerykeion/collector.rs) already uses tokio::sync::Mutex — the rule's own recommended async-safe primitive; converting further to RwLock needs a per-callsite read/write classification across 5 files, an architecture change outside a lint-driven edit. VOCAB/crate-name-collision + NAMING/no-fleet-collision (koinon) are a cross-repo naming call deferred to a fleet naming decision (akroasis#264). NAMING/no-owner-prefix (akroasis-server) needs a GNOMON-reviewed rename, an identity decision outside a mechanical fix. ARCH/substrate-dead-dep (sphragis) is a deliberately staged dependency awaiting the pinax reference-store integration and a cryptographic review (akroasis#172). TOML/missing-trailing-comma (.gitleaks.toml), RUST/doc-promised-observability (delivery.rs), CI/release-yml-missing-attestation (release-please.yml builds no artifacts to attest — release.yml already attests), and RUST/plain-string-secret (ListEntryReport.credential_type, a JSON category label not a secret) are confirmed lint-rule false positives. YAML/missing-concurrency (gate-attestation.yml, added #262) is the same class: the file deliberately carries NO caller-level concurrency block because forkwright/.github/.github/workflows/hybrid-gate.yml already declares one and its own comment states a caller-level duplicate self-cancels the shared group — sphragis's gate-attestation.yml (the reusable workflow's other adopter) carries the identical no-block shape for the identical reason. Entries clear only when the rule is fixed upstream or the cited decision resolves." [[baseline.entry]] rule = "ARCH/substrate-dead-dep" @@ -152,3 +152,9 @@ rule = "VOCAB/crate-name-collision" file = "crates/koinon/Cargo.toml" line = 1 hash = "70acf00586aa7b90c3866278505be7b81fb7ee1f7e21c17c1a22ac239be3c72a" + +[[baseline.entry]] +rule = "YAML/missing-concurrency" +file = ".github/workflows/gate-attestation.yml" +line = 1 +hash = "b8e794237ee7d759994634dcd32d7aadd9f237ef78337fe432f10f23861dd14b" From d9f9d92e5db780f647340fcd245a7c913eb403af Mon Sep 17 00:00:00 2001 From: forkwright Date: Mon, 17 Aug 2026 10:28:40 -0500 Subject: [PATCH 2/2] fix(ci): sha-key security.yml's concurrency group on push, add a regression guard security.yml carried the identical self-cancelling concurrency-group defect this PR's prior commit fixed in gate-attestation.yml: on a push event github.event.pull_request.number is empty, so the group fell back to github.ref, constant for main. Two pushes inside the cancel window collided and cargo audit/cargo deny reported cancelled, not red, for the loser -- confirmed live on 60f97de and b5c8071, the same two commits already cited as this branch's own evidence. Fixed by sha-keying the group on push, matching hybrid-gate.yml's own key; this file has no reusable-workflow fallback to catch the collision, so the key itself (not a caller-level duplicate) was the defect here. Adds crates/syntonia/tests/workflow_concurrency_fitness.rs: a fitness test asserting gate-attestation.yml still declares no top-level concurrency block and security.yml's group is exactly the sha-on-push form -- the mechanical regression check this bug class previously had none of. --- .github/workflows/security.yml | 13 +- .../tests/workflow_concurrency_fitness.rs | 119 ++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 crates/syntonia/tests/workflow_concurrency_fitness.rs diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index c6d5bec..397f52e 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -21,8 +21,19 @@ on: permissions: contents: read +# WHY the SHA on a push (#375): `github.event.pull_request.number` is empty +# on a `push` event, so the old key here fell back to `github.ref`, constant +# for `main` -- two pushes to main inside the cancel window collided, and +# `cargo audit`/`cargo deny` reported `cancelled`, not red, for the loser. +# Confirmed live on 60f97de and b5c8071 (`gh api +# repos/forkwright/akroasis/commits//check-runs`), the same two commits +# `gate-attestation.yml`'s identical fix cites as its own evidence -- this +# file carries the exact same bug, standalone (no reusable-workflow fallback +# to catch it). `schedule`/`workflow_dispatch` keep falling back to +# `github.ref`: a superseding scheduled/manual run cancelling an older one of +# its own kind is the desired behavior, unchanged here. concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.sha || github.ref }} cancel-in-progress: true env: diff --git a/crates/syntonia/tests/workflow_concurrency_fitness.rs b/crates/syntonia/tests/workflow_concurrency_fitness.rs new file mode 100644 index 0000000..0fccc25 --- /dev/null +++ b/crates/syntonia/tests/workflow_concurrency_fitness.rs @@ -0,0 +1,119 @@ +//! Fitness test: the merge-gate workflows must not carry a concurrency group +//! that collides across distinct push-to-main commits. +//! +//! WHY(#375): `.github/workflows/gate-attestation.yml`'s prior caller-level +//! `concurrency:` block and `.github/workflows/security.yml`'s block both +//! used `${{ github.event.pull_request.number || github.ref }}` -- constant +//! for `main` on a `push` event (`pull_request.number` is empty), so any two +//! main-push commits landing inside the `cancel-in-progress` window collided +//! and the loser's job reported `cancelled`, not red, silently dropping the +//! check for that commit. Confirmed live: `gh api +//! repos/forkwright/akroasis/commits//check-runs` on `60f97de` and +//! `b5c8071` shows `cargo audit`/`cargo deny` `cancelled` on both. +//! `gate-attestation.yml` is fixed by removing its caller-level block +//! entirely (the reusable `hybrid-gate.yml` already supplies a sha-on-push +//! one, and a caller-level duplicate self-cancels that shared group); +//! `security.yml` has no reusable-workflow fallback to catch the same +//! defect, so it is fixed by sha-keying its own group on `push` instead. +//! +//! This asserts the shape stays fixed rather than merely documenting it in a +//! comment: before this test, "a second caller-level concurrency block +//! creeping back into `gate-attestation.yml`" or `security.yml`'s key +//! drifting off the sha-on-push form had no mechanical check -- only a +//! comment nobody re-reads under review pressure. + +#![expect( + clippy::expect_used, + clippy::panic, + reason = "integration test — panics are the correct failure mode" +)] + +use std::path::{Path, PathBuf}; + +/// Workspace root, derived from this crate's manifest directory. +fn workspace_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .ancestors() + .nth(2) + .expect("crates/ is two levels below the workspace root") + .to_path_buf() +} + +fn read(rel: &str) -> String { + let path = workspace_root().join(rel); + std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("reading {}: {e}", path.display())) +} + +/// The value of the `group:` line nested two spaces under a top-level +/// `concurrency:` key, or `None` if the file has no top-level `concurrency:` +/// block at all. +/// +/// WHY line-based, not a YAML parser: no YAML crate is a workspace +/// dependency, and both files' `concurrency:` blocks are a fixed two-line +/// shape -- adding a parser dependency for this would be more surface than +/// the thing it checks. +fn top_level_concurrency_group(text: &str) -> Option { + // WHY trim_end on the key line: a trailing space after `concurrency:` + // (invisible in a diff, easy to leave behind editing this by hand) must + // not make the guard blind to a real block -- fail-open on formatting + // noise is exactly the shape of guard that misses its own regression. + let mut lines = text.lines(); + lines.find(|l| l.trim_end() == "concurrency:")?; + for line in lines { + if let Some(rest) = line.strip_prefix(" group:") { + return Some(rest.trim().to_string()); + } + // WHY stop at the next top-level key: a `group:` line belonging to a + // later, unrelated top-level block must never be picked up as this + // block's value. + if !line.starts_with(' ') && !line.trim().is_empty() { + break; + } + } + None +} + +#[test] +fn gate_attestation_carries_no_caller_level_concurrency_block() { + let text = read(".github/workflows/gate-attestation.yml"); + assert!( + top_level_concurrency_group(&text).is_none(), + "gate-attestation.yml must not declare its own concurrency: block -- \ + hybrid-gate.yml already declares a sha-on-push one, and a \ + caller-level duplicate self-cancels the shared group (see the \ + file's own WHY comment above the `on:` block)" + ); +} + +#[test] +fn security_yml_concurrency_group_is_sha_keyed_on_push() { + let text = read(".github/workflows/security.yml"); + let group = top_level_concurrency_group(&text) + .expect("security.yml must declare a top-level concurrency: block"); + assert_eq!( + group, + "${{ github.workflow }}-${{ github.event_name == 'push' && github.sha || github.ref }}", + "security.yml's concurrency group must be sha-keyed on push (matching \ + hybrid-gate.yml's own key) -- a ref-only key (e.g. \ + `github.event.pull_request.number || github.ref`) is constant across \ + every push to main and collides, cancelling one of two commits' \ + cargo audit/cargo deny runs instead of running both. Confirmed live \ + on 60f97de and b5c8071: both showed cargo audit/cargo deny \ + cancelled under the old key." + ); +} + +#[test] +fn extraction_tolerates_a_trailing_space_on_the_key_line() { + // WHY this test exists: the extractor originally matched `concurrency:` + // by exact equality, so a trailing space on that line (invisible in a + // diff) made it silently report "no block" for a file that has one -- + // the guard blind to its own regression. Pins the `trim_end` fix above. + let text = + "on:\n push: {}\n\nconcurrency: \n group: some-value\n cancel-in-progress: true\n"; + assert_eq!( + top_level_concurrency_group(text).as_deref(), + Some("some-value"), + "a trailing space after `concurrency:` must not blind the guard to a real block" + ); +}