diff --git a/crates/openpulse-ardop/tests/ptt_keys_every_transmit.rs b/crates/openpulse-ardop/tests/ptt_keys_every_transmit.rs index 7b17edf1..fd89e567 100644 --- a/crates/openpulse-ardop/tests/ptt_keys_every_transmit.rs +++ b/crates/openpulse-ardop/tests/ptt_keys_every_transmit.rs @@ -178,6 +178,12 @@ fn the_transmitter_is_released_before_each_ack_listen() { /// /// Validated against a planted bare call, so a scan that matched nothing would fail this test rather /// than pass it — the vacuous-gate trap this repo has hit three times. +// VERIFIES: REQ-PTT-04 +// +// The property had no id until #1411: REQ-PHY-07/08 say which PTT backends must EXIST, and +// nothing said the configured one is USED on every emission. It shipped broken independently in +// three front-ends with the same silent symptom — audio emitted with the transmitter unkeyed — +// which is why the binding lives in each front-end rather than only at the SharedPtt seam. #[test] fn every_transmit_in_the_bridge_is_inside_the_keyed_helper() { let src = include_str!("../src/bridge.rs"); diff --git a/crates/openpulse-daemon/tests/ptt_keys_every_daemon_transmit.rs b/crates/openpulse-daemon/tests/ptt_keys_every_daemon_transmit.rs index 9039d57e..b725fe13 100644 --- a/crates/openpulse-daemon/tests/ptt_keys_every_daemon_transmit.rs +++ b/crates/openpulse-daemon/tests/ptt_keys_every_daemon_transmit.rs @@ -136,6 +136,12 @@ async fn a_non_ota_send_message_keys_the_transmitter() { /// truncates each at its first `#[cfg(test)]`, since test modules legitimately transmit bare. Both /// the truncation and the pattern are validated against planted inputs, so a scan that matched /// nothing would fail rather than pass. +// VERIFIES: REQ-PTT-04 +// +// The property had no id until #1411: REQ-PHY-07/08 say which PTT backends must EXIST, and +// nothing said the configured one is USED on every emission. It shipped broken independently in +// three front-ends with the same silent symptom — audio emitted with the transmitter unkeyed — +// which is why the binding lives in each front-end rather than only at the SharedPtt seam. #[test] fn every_daemon_transmit_is_keyed() { let files: Vec<(&str, &str)> = vec![ diff --git a/crates/openpulse-kiss/tests/ptt_keys_every_transmit.rs b/crates/openpulse-kiss/tests/ptt_keys_every_transmit.rs index 775e0e55..08302945 100644 --- a/crates/openpulse-kiss/tests/ptt_keys_every_transmit.rs +++ b/crates/openpulse-kiss/tests/ptt_keys_every_transmit.rs @@ -135,6 +135,12 @@ fn the_watchdog_force_releases_a_key_that_outlives_its_deadline() { /// writes its calls as multi-line chains where `engine` and `.transmit(` are five lines apart, so /// the copied pattern would match **nothing** — a vacuous gate whose planted single-line control /// still passes. That is why the control below is deliberately multi-line. +// VERIFIES: REQ-PTT-04 +// +// The property had no id until #1411: REQ-PHY-07/08 say which PTT backends must EXIST, and +// nothing said the configured one is USED on every emission. It shipped broken independently in +// three front-ends with the same silent symptom — audio emitted with the transmitter unkeyed — +// which is why the binding lives in each front-end rather than only at the SharedPtt seam. #[test] fn every_transmit_in_the_bridge_is_keyed() { let src = include_str!("../src/bridge.rs"); diff --git a/docs/dev/project/requirements.yaml b/docs/dev/project/requirements.yaml index f0a47a28..8bae251a 100644 --- a/docs/dev/project/requirements.yaml +++ b/docs/dev/project/requirements.yaml @@ -855,13 +855,16 @@ capabilities: - REQ-PHY-05 - REQ-PHY-07 - REQ-PHY-08 + - REQ-PTT-01 + - REQ-PTT-04 tests: - - crates/openpulse-radio/src/noop.rs - - crates/openpulse-radio/tests/generic_cat_integration.rs - - crates/openpulse-radio/tests/rigctld_integration.rs - crates/openpulse-ardop/tests/ptt_keys_every_transmit.rs - crates/openpulse-daemon/tests/ptt_keys_every_daemon_transmit.rs + - crates/openpulse-kiss/tests/ptt_keys_every_transmit.rs - crates/openpulse-mesh/tests/no_real_audio.rs + - crates/openpulse-radio/src/noop.rs + - crates/openpulse-radio/tests/generic_cat_integration.rs + - crates/openpulse-radio/tests/rigctld_integration.rs traceability: baseline CAP-60: code: @@ -1108,7 +1111,6 @@ capabilities: - crates/openpulse-radio/src/rigctld.rs name: PTT backends satisfies: - - REQ-PTT-01 - REQ-PTT-02 - REQ-PTT-03 tests: @@ -2150,8 +2152,8 @@ requirements: REQ-PTT-01: category: PTT covered_by: - - CAP-74 - statement: PTT assert/release within 50 ms + - CAP-59 + statement: Every PTT-keyed transmit scope releases the transmitter deterministically on scope exit - early return, error, and panic/unwind - via an RAII guard, so an unexpected key-down is bounded by the scope rather than by ptt_max_duration; the watchdog remains the backstop for a transmit that blocks. status: ratified traceability: baseline REQ-PTT-02: @@ -2168,6 +2170,12 @@ requirements: statement: GPIO PTT backend status: ratified traceability: baseline + REQ-PTT-04: + covered_by: + - CAP-59 + statement: 'Every transmission the station makes - data, ARQ/ACK, handshake, QSY, relay, station ID, discovery beacon and file-transfer bursts - keys the transmitter through the configured PTT backend before audio is emitted, and holds the key for the burst. Emitting audio unkeyed is a fault, never a fallback: VOX-only operation is an explicit operator configuration, not what an unusable backend degrades to.' + status: ratified + traceability: enforced REQ-QRM-01: category: Interference covered_by: diff --git a/docs/dev/project/traceability.md b/docs/dev/project/traceability.md index e9265806..4323cfdd 100644 --- a/docs/dev/project/traceability.md +++ b/docs/dev/project/traceability.md @@ -15,6 +15,70 @@ and the actually-observed results per change. --- +## 2026-09-19 — REQ-PTT-01's registered statement was a paraphrase that changed its meaning; REQ-PTT-04 added; #1411 + +**Change.** REQ-PTT-01's yaml statement restored to its ratified prose and re-pointed +`covered_by: CAP-74 -> CAP-59`; new **REQ-PTT-04** (enforced, CAP-59) for the keying property that +had no id; three `// VERIFIES: REQ-PTT-04` bindings; `openpulse-kiss`'s keying test added to +CAP-59's `tests:`; prose added to `requirements.md`. + +**The issue half-dissolved, and the reason is a registry defect.** #1411 claimed *no* requirement +states the PTT keying property. That is true for one half and **false for the other**, because my +sweep searched the yaml `statement:` fields and the **ratified prose says something else**. +`docs/dev/requirements.md:398-402`: + +> Every PTT-keyed transmit scope shall release the transmitter **deterministically on scope exit** — +> including on an early return or a panic/unwind — via an RAII guard … (REQ-PTT-01) + +The yaml carries `PTT assert/release within 50 ms`. Provenance, by `git log -S`: `f4c10467` (#872) +created REQ-PTT-01 as the RAII guard; `daa1676e` (#1098) wrote a **traceability-matrix row** +paraphrasing it as the 50 ms clause and citing the wrong test; `1da27abd` (#1117) imported that row +into `requirements.yaml` as the statement. The importer was later deleted (#1223), so the yaml became +source of truth **carrying the paraphrase**. The code agrees with the prose, not the yaml — +`shared_ptt.rs:221,413,869` all cite REQ-PTT-01 for the RAII guard. **Nothing checks a yaml statement +against its prose.** + +So the stuck-key failure mode was registered all along, and several trailers #1402 called +"defensible but pointing at a timing requirement" are simply **right**. + +**A live #1405 instance, fixed here.** REQ-PTT-01 was `covered_by: [CAP-74]` ("PTT backends"), which +does **not** own `shared_ptt.rs` — the file its own acceptance test lives in, owned by CAP-59. Its +coverage was bound to the paraphrase's evidence rather than to its requirement. + +**What was genuinely missing, and is now REQ-PTT-04.** REQ-PHY-07/08 say which PTT backends must +*exist*; nothing said the configured one is *used* on every emission. Swept `unkeyed|stuck key|keys +the transmitter|every emission|dead RF|jammed` across `requirements.md`, `regulatory.md`, the 1.0 +criteria, `AGENTS.md` and the roadmap: zero hits (positive control: 3 in CLAUDE.md). It shipped +broken independently in three front-ends with the same silent symptom, which is why the binding sits +in each front-end and not only at the seam. + +**Capability: CAP-59, not a new one — and the registry chose this already.** The emission paths span +five capabilities, and a "keying discipline" capability owning `bridge.rs` would recreate exactly +what #1399's maintainer decision measured and rejected (extending CAP-59 into its importers turned +two ARDOP/KISS *receive* mislabels into passes, because file-level overlap cannot separate two +concerns in one file) and would rebuild the CAP-68 shape flagged the day before. CAP-59 is the +registry's own precedent: the author twice added these very tests to `CAP-59.tests`, and it mirrors +REQ-SEC-13/CAP-77 — the capability owns the *seam*, source scans hold the sites. + +**The atomicity argument was dropped as imported.** I proposed splitting on +`iterative-delivery`'s "requirements must be atomic". **Compound statements are this registry's +norm** (REQ-CTL-02, REQ-SEC-14, REQ-CTL-04, REQ-DISC-04, REQ-QRM-01 …). The split stands on +different grounds: different provenance (2026-07 vs never), different evidence tier (a hardware +1.0-criterion vs a source scan plus a counting spy), different mechanism (RAII/watchdog vs routing +through the configured backend). + +**Corrections to #1411's own text:** ten commits carry `Implements: REQ-PTT-01`, not eight; "PTT-01 +is the only PTT requirement" is false (PTT-02/03, PHY-05/07/08); and CLAUDE.md's 50 ms row names +REQ-PHY-05 only — it never paired PTT-01 with it, so the "control-path and audio-path halves" framing +was mine, not the repo's. + +**Gates.** trace check ok · trace self-test ok · reachability ok · fmt ok · the three bound suites +8 passed / 0 failed. + +**Review.** `docs/dev/reviews/review-1411-keying-requirement.md`. + +--- + ## 2026-09-19 — nine merged commits carry the wrong trailer, and most carry the wrong trailer TYPE; #1402 **Change.** This entry. The nine commits below are merged, so the ledger is the only place their diff --git a/docs/dev/requirements.md b/docs/dev/requirements.md index 6f01d45f..06f396a1 100644 --- a/docs/dev/requirements.md +++ b/docs/dev/requirements.md @@ -2,7 +2,7 @@ project: openpulsehf doc: docs/dev/requirements.md status: living -last_updated: 2026-09-16 +last_updated: 2026-09-19 --- # Requirements @@ -400,6 +400,14 @@ in the roadmap; each is a candidate, not a committed deliverable. max-duration watchdog (REQ-REG-10 / #863). This bounds an unexpected key-down to the current stack scope instead of up to `ptt_max_duration`. Acceptance: a test that panics inside a keyed transmit scope and asserts the transmitter was released without waiting for the watchdog timer. (REQ-PTT-01) +- Every transmission the station makes — data, ARQ/ACK, handshake, QSY, relay, station ID, discovery + beacon and file-transfer bursts — shall key the transmitter through the configured PTT backend + before audio is emitted, and hold the key for the burst. Emitting audio unkeyed is a **fault, never + a fallback**: VOX-only operation is an explicit operator configuration, not what an unusable + backend degrades to. REQ-PHY-07/08 say which backends must *exist*; this says the configured one is + *used*. Acceptance: per front-end, a source scan requiring every transmit call site to sit inside + the keyed helper — itself validated against a planted bare call — plus a counting backend asserting + one keying per emission. (REQ-PTT-04) - `openpulse-radio` shall support keying via the **CM108/CM119 sound-chip GPIO over USB-HID** (the common cheap-interface PTT path), selectable from config like the existing backends. Acceptance: unit tests for the HID output-report encoding; documented in the PTT backend list. (REQ-PTT-02) diff --git a/docs/dev/reviews/review-1411-keying-requirement.md b/docs/dev/reviews/review-1411-keying-requirement.md new file mode 100644 index 00000000..afa2c2c3 --- /dev/null +++ b/docs/dev/reviews/review-1411-keying-requirement.md @@ -0,0 +1,144 @@ +--- +project: openpulsehf +doc: docs/dev/reviews/review-1411-keying-requirement.md +status: resolved +last_updated: 2026-09-19 +--- + +# Design review — register the PTT keying property that eight commits point at and no id carries (#1411) + +Nothing is built. This is a proposal to add one or two requirements to `requirements.yaml`, which is +a decision site, reviewed before implementation. + +## Consumer + +- `docs/dev/project/requirements.yaml` — the registry. `trace.py` reads it for REQ-GAP, EMPTY-CAP + and the dormancy join; `req-mutation.sh` derives mutation scope from the capabilities a + requirement is `covered_by`. +- `scripts/check-trailer.sh` — the ids become legal trailer targets. Eight merged commits currently + point at REQ-PTT-01 for want of a better id (#1402's adjudication; corrected set in #1410). +- CLAUDE.md's acceptance table already carries the property in **three rows**, with tests. + +## Prior art + +- **No existing requirement states it.** Swept every statement for `key|PTT|transmitter`: the + matches are REQ-PTT-01 ("PTT assert/release within 50 ms"), REQ-PHY-05 ("Transmitter release must + occur within 50 ms of the last transmitted sample"), REQ-PHY-07/08 (which *backends* must exist), + REQ-FX-06 (airtime-bounded bursts within the radio watchdog), REQ-CTL-02 (TX-keying fails closed + for an unauthenticated client). **All are timing, backend inventory, or auth gating. None says a + transmission must be keyed at all, or that no path may leave it keyed.** +- Note PTT-01 and PHY-05 are *both* 50 ms clauses — the control-path and audio-path halves, a split + CLAUDE.md states explicitly. PTT-01 is therefore already narrow; this is not a case of an id being + deliberately broad. +- The tests exist and run: `ptt_keys_every_transmit` (ardop 3, kiss 3), `ptt_keys_every_daemon_transmit` + (2), `abnormal_exit_release` (1), `shared_ptt` (23) — **32 tests, all listed under the gate's + `--no-default-features`**, so `enforced` is feasible rather than aspirational. + +## Twins + +- **REQ-PHY-05 is the twin to keep distinct.** It is deferred (#1112) because its audio half needs + the rig. This proposal must not absorb it, or a deferred-and-honest gap becomes silently "covered". +- **REQ-CTL-02** already says TX-keying fails closed for an unauthenticated client — an adjacent + *refusal* property. The new requirement is about keying when transmission is legitimate. +- **`openpulse-mesh`** had its audio route *removed* rather than guarded, precisely because it had + no keying discipline and no station-ID timer. That is this property asserted by deletion. + +## Prompt + +Test this rather than confirm it. Two of my last three issue framings were too strong and were +corrected by looking at structure; assume this one is too until it survives. + +### A. One requirement or two? + +The eight commits cover two different failure modes: + +1. **An emission is not keyed** → dead RF. Wasteful, not dangerous. +2. **An exit path leaves the transmitter keyed** → a stuck transmitter. Unsafe, and a §97.221 + problem on an unattended station. + +`iterative-delivery`'s requirement-quality rule says *atomic — one testable assertion; compound +requirements hide gaps; split them*, and the acceptance table already treats them as separate rows. +So I lean **two**. Argue me out of it if one requirement with two clauses is how this registry +actually behaves elsewhere — I did not survey that. + +### B. The capability question, which is the real problem + +The emission paths span **five capabilities in five crates**: CAP-39 (ardop `bridge.rs`), CAP-40 +(kiss), CAP-55 (daemon `server.rs`), CAP-47 (repeater), CAP-59 (`shared_ptt.rs`). Options: + +1. `covered_by: [CAP-39, CAP-40, CAP-47, CAP-55, CAP-59]` — honest about where the code is, but + gives the requirement a mutation scope spanning five crates, and #1405 has just shown what a + scope wider than its bound tests produces. +2. `covered_by: [CAP-59]` alone — the property's *seam* is `SharedPtt`, and `cross-cutting-seams` + says a cross-cutting concern belongs at the single shared seam. Narrow and checkable, but it + would be false: the front-ends are where the property is actually violated, and four of the + eight commits touched no `openpulse-radio` file. +3. **A new capability** — "PTT keying discipline on every emission path" — owning the seam plus the + emission sites, satisfying the new requirement(s). Most honest, one more capability, and it + overlaps files that CAP-39/40/47/55 already own (multi-ownership rises). + +I lean (3) but I am not confident. Which of these does the registry's grain actually favour? + +### C. Does registering it change any verdict today? + +If `enforced`, `req-mutation.sh --all-enforced` gains it, and its scope under option (1) or (3) +includes `daemon/server.rs` (218 mutants) and `ardop/bridge.rs`. Given #1279 concluded the scheduled +job is not viable, that cost is currently theoretical — but the enforced set is also the thing +#1405 measures. **Does adding this make anything worse before it makes anything better?** + +### D. Is this a requirement at all? + +The sceptical reading: "every emission keys the transmitter" is an *implementation invariant* of +having a PTT at all, not a product requirement, and the right home is the acceptance table where it +already lives. I do not believe that — an unkeyed emission is externally observable as silence on +the air, and a stuck key is observable as a jammed channel — but the argument deserves a hearing +before the registry grows by two. + +## Verdict + +Reviewed 2026-09-19. **Add ONE requirement (keying-at-all), CORRECT one that already exists +(REQ-PTT-01), create NO new capability.** + +**My central factual claim was false for half the property, and the falsity is a registry defect.** +I swept the yaml `statement:` fields; the **ratified prose** at `requirements.md:398-402` makes +REQ-PTT-01 the RAII release-on-scope-exit requirement. `f4c10467` (#872) created it that way; +`daa1676e` (#1098) wrote a traceability-matrix row paraphrasing it as "PTT assert/release within +50 ms" and citing the wrong test; `1da27abd` (#1117) imported that row into the yaml as the +statement. The importer was deleted (#1223), so the yaml became source of truth carrying the +paraphrase. The code agrees with the prose (`shared_ptt.rs:221,413,869`). Nothing checks a yaml +statement against its prose. **So the stuck-key half was registered all along.** + +**Also live: a #1405 instance.** REQ-PTT-01 was `covered_by: [CAP-74]`, which does not own +`shared_ptt.rs` — the file its acceptance test lives in. Re-pointed to CAP-59. + +**D — is it a requirement? ADOPT, with the argument replaced.** The survey I skipped says the +registry is *not* consistently externally-specified behaviour: every post-bright-line entry is +invariant-shaped (REQ-DCD-01 names the seam, REQ-SEC-13 names the clippy gate, REQ-RX-02/03 name the +mechanism). So the "implementation invariant, wrong home" objection has no footing. But my +"externally observable" argument is worthless — *every* defect is observable as silence. What +survives is the **hole between two existing ids**: PHY-07/08 say which backends must exist; nothing +said the configured one is used. + +**A — two ids, but one is PTT-01 corrected, and NOT on atomicity.** Compound statements are this +registry's norm (CTL-02, SEC-14, CTL-04, DISC-04, QRM-01). My atomicity argument was imported from +`iterative-delivery` rather than drawn from this repo. The split stands on provenance, evidence tier +and mechanism instead. + +**B — option (2), CAP-59. Options (1) and (3) rejected.** A new "keying discipline" capability owning +`bridge.rs` recreates precisely what #1399's maintainer decision measured and rejected, and rebuilds +the CAP-68 shape. CAP-59 is the registry's own precedent — the author twice added these tests to +`CAP-59.tests` — and mirrors REQ-SEC-13/CAP-77: the capability owns the seam, source scans hold the +sites. + +**C — `enforced`, and the question was partly moot.** A new id *cannot* be `baseline` +(`NOT-GRANDFATHERED`, `trace.py:682`). My feasibility claim was overstated in the other direction: +the 32 tests pass, but none carried a `// VERIFIES:` — enforced is feasible *after* adding bindings, +which this change does. + +**Corrections to the issue text:** ten commits carry `Implements: REQ-PTT-01`, not eight; "PTT-01 is +the only PTT requirement" is false; and CLAUDE.md's 50 ms row names REQ-PHY-05 only, so the +"control-path and audio-path halves" pairing was mine. + +**Not done here, recorded instead:** CAP-59 mixes PTT and CAT and shares `rigctld.rs` with CAP-74 — +a curation item. And `68887bd5` (OTA send reports `PttFault` rather than `Delivered`) belongs to +neither id; it is about delivery reporting.