feat(governance): route review by what a diff does — TX-path ack, red-main attribution, intent-first review (RFC #6435)

.claude/skills/pr-review/SKILL.md

@@ -85,6 +85,15 @@ clean report stated plainly and without hedging. Severity stays calibrated
 what you actually observed — file:line, command output, failing scenario.
 Break the code, not the author.
 
+Nor does it mean **asking for defense the code does not need.** Principle VII
+puts validation at the boundary "so the interior can treat parsed values as
+sound": a request to re-check, inside our own code, a value that already
+crossed a boundary or that our own code produced is over-engineering, not
+rigor. So is an abstraction, option or fallback the PR's stated intent does
+not call for. Raise either as a nit at most, never a blocker, unless you can
+name the concrete input that reaches the interior unvalidated. That input is
+the finding; the missing check is not.
+
 ## 0. Test-boundary preflight — before parallel work
 
 Complete this preflight before building or running any test. It overrides the
@@ -178,6 +187,13 @@ the shape the bullet above routes a simulator closed loop to — see
 
 ## 2. Linked issue → does the PR actually solve it?
 
+- **Start with intent.** Read the PR body's `## Intent` section (the PR
+  template asks for it) and state in one line what the PR is for. Check that
+  against the linked issue before reading any hunk: a correct diff for the
+  wrong goal is the most expensive finding a review can miss, and the one
+  line-by-line reading misses most. If the section is absent, reconstruct the
+  intent from the body and issue, say you did, and note the missing section
+  as a nit.
 - Linked issues = `closingIssuesReferences` plus any `#NNNN` referenced in
   the PR title/body as "fixes/closes/resolves". Read each with
   `gh issue view <N> --json title,body,comments` (skim comments for accepted
@@ -711,6 +727,7 @@ round trip even for a nit. One body is read once and answered once.
 ## PR #NNNN — <title> (@author)
 
 **Issue:** #MMMM — one-paragraph summary of the problem as reported.
+**Intent:** the PR's stated goal in one line, and whether it matches the issue.
 **Proposed fix:** one-paragraph summary of the approach the diff takes.
 **Does it solve the issue?** Yes / Partially / No — with the requirement→hunk
 mapping and anything unaddressed.

.github/CODEOWNERS

@@ -151,6 +151,12 @@ docs/                        @aethersdr/reviewers
 resources/                   @aethersdr/reviewers
 tests/                       @aethersdr/reviewers
 
+# Transmit-path acknowledgements (Principle VI, tools/check_tx_surface.py). A PR
+# whose diff reaches .github/tx-surface.txt adds .github/tx-acks/<PR>.md; any
+# code owner's approval covers it, so the directory is Tier 3. Its files are
+# markdown, so like docs/ this line must sit below the *.md glob.
+.github/tx-acks/             @aethersdr/reviewers
+
 # AI-instruction files. AGENTS.md / CLAUDE.md / GEMINI.md /
 # .github/copilot-instructions.md all land here via the *.md glob above —
 # they are listed in the Tier 2 header comment rather than repeated as
@@ -252,6 +258,10 @@ SECURITY*                    @aethersdr/maintainers
 # diff rather than one more "rides along" step in ci.yml (which is Tier 2).
 # See tools/check_ci_test_gate.py.
 .github/ci-test-gate.txt     @aethersdr/maintainers
+# The transmit surface that tools/check_tx_surface.py gates on. Tier 1 for
+# the same reason: shrinking it is how a keying change would slip past the
+# TX acknowledgement, so the diff to it is the review signal.
+.github/tx-surface.txt       @aethersdr/maintainers
 .github/codeql/              @aethersdr/maintainers
 docs/RELEASE-SIGNING-KEY.pub.asc  @aethersdr/maintainers
 

.github/PULL_REQUEST_TEMPLATE.md

@@ -18,6 +18,13 @@ Before opening the PR, please:
 <!-- One-paragraph description of what changes and why.  If this fixes
 an issue, lead with "Fixes #N" or "Closes #N". -->
 
+## Intent
+
+<!-- What will be true after this merges that is not true now, in one or two
+sentences, and anything deliberately left out of scope.  Review starts here:
+the diff is judged against this before it is read line by line, so a correct
+diff for the wrong goal is caught first. -->
+
 ## Constitution principle honored
 
 <!-- Cite the principle by Roman numeral when relevant.  E.g.

.github/tx-surface.txt

@@ -0,0 +1,40 @@
+# The transmit surface. Principle VI: AetherSDR never transmits without
+# operator intent. A PR whose diff reaches anything listed here must say so:
+# tools/check_tx_surface.py fails Static checks until the PR adds
+# .github/tx-acks/<PR number>.md stating what changes on the TX path and how it
+# was verified. That directory is owned by @aethersdr/reviewers, so any code
+# owner's approval covers it. See GOVERNANCE.md §Transmit-path changes.
+#
+# This list is Tier 1: shrinking it is how the gate would be bypassed, so the
+# diff to this file is the review signal, the same as .github/ci-test-gate.txt.
+#
+# Two kinds of entry, one per line, `kind<space>regex`, trailing `# reason`
+# not parsed:
+#   path   — Python regex matched against the repo-relative path of any file
+#            the PR changes in a line that is not comment-only. The whole
+#            file is TX machinery.
+#   token  — Python regex searched in every added or removed line under src/
+#            that is not comment-only. The token is TX machinery wherever it
+#            appears, including inside RadioModel.cpp and MainWindow*.cpp,
+#            which are too large to gate by path.
+#
+# Comment-only lines never trigger either kind: a reworded comment is not a
+# keying change. A user-visible string that merely names a token is not one
+# either, which is why the env-var entry matches the read, not the name.
+
+path   ^src/models/TxController\.(h|cpp)$                # the TX permission object
+path   ^src/core/TxCoordinator\.(h|cpp)$                 # arbitration between TX producers
+path   ^src/models/RadioModel_TxCoordinator\.cpp$        # RadioModel's half of the coordinator
+path   ^src/core/backends/flex/FlexPttWireSession\.(h|cpp)$  # writes `xmit 1` / `xmit 0`
+path   ^src/core/TxKeyingMarker\.h$                      # which controls key, and the bridge's refusal
+
+token  "xmit\b                                          # Flex MOX on the wire
+token  "transmit tune\b                                 # Flex TUNE carrier
+token  "atu start\b                                     # ATU tune keys a carrier
+token  \bcwx (macro )?send\b                            # CWX keys CW
+token  \bsetKeying\b                                    # IRadioBackend keying entry point
+token  \b(setMox|setPtt|requestMox)\b                   # backend and model keying
+token  \b(markTxKeying|registerTxKeyingAction|registerReceiveControlAction)\b  # keying-control registration
+token  \b(transmitControlMatch|requiresTxPermission)\b  # keyboard and bridge TX guards
+token  \b(qEnvironmentVariable\w*|qgetenv|getenv)\(\s*"AETHER_AUTOMATION_ALLOW_TX"  # reading the bridge's TX opt-in
+token  \bm_txAllowed\b                                # the bridge's cached TX opt-in

.github/workflows/full-suite.yml

@@ -30,9 +30,13 @@ on:
 # `github.run_id` in the group, which gave every run a group of its own and
 # made the whole thing inert — the same behaviour, reached confusingly.
 
+# actions: read and pull-requests: read are for the tracking issue's
+# attribution lines (the parent commit's verdict, the commit's source PR).
 permissions:
   contents: read
   issues: write
+  actions: read
+  pull-requests: read
 
 jobs:
   full-suite:
@@ -318,13 +322,53 @@ jobs:
             const createLabels = [...labels, 'bug', 'priority: high'];
             const sha    = context.sha.substring(0, 7);
             const date   = new Date().toISOString().split('T')[0];
+            const repo   = { owner: context.repo.owner, repo: context.repo.repo };
+
+            // Attribution, so a human can decide fix-forward vs revert from the
+            // issue alone (docs/PR-WORKFLOW.md §Recovering from a red main).
+            // Squash merges make the pushed commit the suspect, unless main was
+            // already red one commit earlier. Neither lookup may stop the issue
+            // from being filed, so each degrades to "unknown".
+            let sourcePr = 'unknown';
+            try {
+              const prs = await github.rest.repos.listPullRequestsAssociatedWithCommit({
+                ...repo, commit_sha: context.sha,
+              });
+              const pr = prs.data.find(p => p.merged_at) || prs.data[0];
+              sourcePr = pr ? `#${pr.number} by @${pr.user.login}` : 'none (direct push)';
+            } catch (e) {
+              core.warning(`source PR lookup failed: ${e.message}`);
+            }
+            let parentVerdict = 'unknown';
+            try {
+              const commit = await github.rest.repos.getCommit({ ...repo, ref: context.sha });
+              const parent = commit.data.parents[0]?.sha;
+              if (parent) {
+                const runs = await github.rest.actions.listWorkflowRuns({
+                  ...repo, workflow_id: 'full-suite.yml', head_sha: parent, per_page: 10,
+                });
+                const done = runs.data.workflow_runs.find(r => r.status === 'completed');
+                const p7 = parent.substring(0, 7);
+                parentVerdict =
+                  !done                          ? `no completed run for parent \`${p7}\``
+                  : done.conclusion === 'success' ? `green at parent \`${p7}\` — this commit is the first red one`
+                  : `already ${done.conclusion} at parent \`${p7}\` (${done.html_url}) — look earlier`;
+              }
+            } catch (e) {
+              core.warning(`parent verdict lookup failed: ${e.message}`);
+            }
 
             const body = [
               `**Run:** ${process.env.RUN_URL}`,
               `**Commit:** \`${sha}\``,
+              `**Source PR:** ${sourcePr}`,
+              `**Main before this commit:** ${parentVerdict}`,
               `**Date:** ${date}`,
               `**Failed:** ${process.env.FAILED}`,
               '',
+              'Before reverting, check the failing tests against the known-flake umbrella #4703 ' +
+                'and re-run once; a revert is a human call (docs/PR-WORKFLOW.md §Recovering from a red main).',
+              '',
               '<details><summary>ctest report</summary>',
               '',
               '```',

.github/workflows/static-checks.yml

@@ -343,6 +343,22 @@ jobs:
         if: ${{ !cancelled() }}
         run: python tools/check_ci_test_gate.py --strict
 
+      # ── Transmit-path acknowledgement (Principle VI). Keying code is spread
+      # across Tier-3 files too large to gate by path (RadioModel.cpp,
+      # MainWindow*.cpp, every backend), so this gates on what the diff does:
+      # a PR whose non-comment lines reach .github/tx-surface.txt must add
+      # .github/tx-acks/<PR>.md saying what changes on the TX path and how it
+      # was verified. The checkout is the merge ref, so HEAD^1 is the base.
+      - name: Transmit-path acknowledgement
+        if: ${{ !cancelled() }}
+        env:
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+        run: python tools/check_tx_surface.py --base HEAD^1 --pr "$PR_NUMBER"
+
+      - name: Transmit-path checker self-test
+        if: ${{ !cancelled() }}
+        run: python tools/test_check_tx_surface.py
+
       # ── Network transfer-timeout ratchet (#4688 §6). A QNetworkAccessManager
       # with no transfer timeout turns a half-open connection into a reply that
       # never finishes and never errors — the surface that issued it just stops,

AGENTS.md

@@ -319,6 +319,10 @@ runs in Docker image `ghcr.io/aethersdr/aethersdr-ci:latest`; a new
 `find_package(...)` needs its `-dev` package in `.github/docker/Dockerfile`.
 Test registration, the frozen per-PR test gate, and gate integrity:
 [`docs/agents/tests-ci.md`](docs/agents/tests-ci.md).
+A PR whose code reaches `.github/tx-surface.txt` (keying commands, keying entry
+points, TX permission guards) must add `.github/tx-acks/<PR number>.md` stating
+what changes on the TX path and how it was verified — Static checks fails
+without it (GOVERNANCE.md §Transmit-path changes, Principle VI).
 
 ## Architecture Overview
 

CONTRIBUTING.md

@@ -154,6 +154,13 @@ tier simply means changes to the repo's own scaffolding (dependency bumps,
 issue templates, CI config, build config) need an infrastructure owner rather
 than a maintainer.
 
+One gate is not by path. A PR whose code reaches the transmit surface listed
+in `.github/tx-surface.txt` (wire keying commands, keying entry points, TX
+permission guards) must add `.github/tx-acks/<PR number>.md` saying what
+changes on the TX path and how it was verified; Static checks fails until it
+does, and any code owner can approve it. See
+[`GOVERNANCE.md`](GOVERNANCE.md) §Transmit-path changes.
+
 For the mechanics around this gate — draft-PR conventions, the stale-branch
 policy, and how to recover a red `main` — see
 [`docs/PR-WORKFLOW.md`](docs/PR-WORKFLOW.md).

GOVERNANCE.md

@@ -202,6 +202,27 @@ review from someone else. The Tier-1 paths are hard gates: no merge without
 maintainer sign-off (an admin override is the only bypass). Team rosters live
 in the GitHub org; `.github/CODEOWNERS` carries the exact patterns.
 
+### Transmit-path changes
+
+CODEOWNERS routes review by path, but the code that keys the transmitter is
+spread through Tier-3 files too large to gate that way (`RadioModel.cpp`,
+`MainWindow*.cpp`, every backend). A keying change there would otherwise
+arrive looking like any other source change. Principle VI makes this the one
+place where review is routed by **what a diff does**, not where it lands:
+
+- `.github/tx-surface.txt` lists the transmit surface: files that are wholly
+  TX machinery, and tokens (wire commands, keying entry points, TX permission
+  guards) that are TX machinery wherever they appear.
+- When a PR's added or removed lines under `src/` reach that surface, Static
+  checks fails until the PR adds `.github/tx-acks/<PR number>.md` stating what
+  changes on the transmit path and how it was verified. Comment-only lines
+  never trigger it.
+- Any code owner (`@aethersdr/reviewers`) can approve the acknowledgement. Its
+  value is that the approval covers an explicit TX disclosure, and stale-review
+  dismissal makes it cover the head that merges.
+- The surface list itself is Tier 1. Growing it is routine; shrinking it is
+  how a keying change would slip through, so that diff needs the maintainer.
+
 ---
 
 ## AI Contributors