fix(radiocert): Fix Icom Persist freshness and TX evidence reporting

[jensenpat]

Icom Persist snapshots could present startup defaults as settled state, transaction histories lacked stable event identity, and TX reports could turn unsupported meters or a single-tone generator into misleading evidence. This follow-up to merged #5500 makes confirmation provenance explicit and fixes the test-process defects found during the IC-7300MK2 round.

• Track accepted confirmation age, pending intent and session/context for frequency, mode/data/filter tuple, SQL, AGC, RF power and PTT. Export bounded transaction history with event IDs and a backend-instance UUID through existing scheduler/Persist diagnostics. Readiness covers these six fields only and does not authorize TX.
• Report temperature/voltage with support and freshness; absent readings are null. Preserve native ALC units in TX reports. Refuse Icom txtest twotone before keying because its current TUNE generator produces one sine wave.
• Start TX freshness timing before keying, exclude prior-burst samples, require a fresh accepted Icom PTT-off confirmation, and distinguish a fresh zero-carrier CW gap from missing telemetry. A gap requires a valid SWR earlier in the same burst and fresh power/SWR updates; the 0.9-second initial deadline, 500 ms safety freshness and watt ceiling remain intact.
• Correct earlier waveform claims and record the hardware results, restoration, freshness baseline and unresolved observations in docs/research/persist-icom7300mk2-p1-followup-2026-09-08.md.

Validation:

• Main-based macOS app build with cmake --build build -j22; eight focused headless CTest selections passed, including 17 TX-harness safety cases. Engine-boundary, registration, bridge-doc, touchpoint-manifest and frozen CI-gate checks passed locally.
• Socket-free regressions inject the backend/dispatcher and decoded frames; no new socket-owning test or synthetic firmware peer. Mutation checks caught constant event IDs, disabled context invalidation and removal of the Icom two-tone refusal.
• Authorized IC-7300MK2 live tests at 7.200 MHz into ANT1 dummy load, maximum 10 W: CW 2.797 W, AM 1.399 W in the final repeat, DIGU 6.993 W, SWR 1.0; explicit unkey and actual gauge observations recorded. SQL 27/AGC Fast retained across normal process restart. Final RX-only reconnect readiness took about 2.4 seconds. Original settled state was restored and the radio disconnected.
• Hardware tests preceded the main transplant; exact tested executable hashes and stages are in the report. The final integrated build and additional aggregate-freshness tightening have local test evidence, not another TX run. No full post-fix Icom matrix, IC-705/IC-9700 hardware proof, physical front-panel latency or independent RF measurement is claimed.

AM at 2% still produced radio-meter zero and a guarded stop. The historical intermittent CW/missing-power and initial DIGU stop were not deterministically reproduced; successful repeats do not close those observations. Scheduler optimization and actual Icom two-tone generation remain follow-up work. The four earlier persistence repairs remain separate in #5514.

Generated with OpenAI Codex (GPT-6 Astra)

[aethersdr-agent]

Issue fit

No fixes/closes issue — this is a follow-up to the research report merged as #5500, driving the P1 items out of docs/research/persist-icom7300mk2-expanded-run-2026-09-08.md. Per GOVERNANCE.md "What does NOT require an RFC" (bug fixes with a clear root cause, documentation corrections), no RFC is owed. Against its own stated intent it delivers: the six-field freshness payload, lifetime-unique scheduler event IDs, the two-tone mislabeling correction, and the meter-provenance fix are all present and each has a socket-free test. The corrections to the two earlier research reports are honest — they retract the two-tone claim rather than quietly deleting it.

Two things the body does not claim and should: the two-tone refusal is scoped to Icom while the identical defect exists on HL2/RTL (blocker 1), and the harness's SWR/power ceiling windows were narrowed while TX_TEST_PROMPT.md says "the timing budgets are unchanged" (blocker 3).

Scope

File / group	What it changes	Claimed?	Verdict
src/core/backends/icom/IcomCivScheduler.{h,cpp}	TransactionEvent::eventId, monotonic counter	Yes	In scope
src/core/backends/icom/IcomCivBackend.{h,cpp} — confirmState/stateFreshness/instance UUID	Six-field freshness diagnostic	Yes	In scope
IcomCivBackend.cpp:2338 — AGC size()!=1 || v<1 || v>3	Drops out-of-range AGC replies entirely, not just for freshness	No — presented as freshness validation	Undisclosed behavior change (nit 3)
IcomCivBackend.cpp:2905 — PTT size()==1 && data[0]<=1	Narrows acceptance in a TX-safety path	No	Blocker 2
AutomationServer.cpp — meterObservation, temperature/voltage, paTemp/supplyVolts	New bridge surface + scalar semantics change	Yes	New public surface — maintainer decision
AutomationServer.cpp:6883 — Icom twotone refusal	Refuses a previously-accepted verb	Yes	In scope, but incomplete (blocker 1)
AutomationServer.cpp:8480 — backendDiagnostics in persist	New persist field	Yes	New public surface — maintainer decision
tools/tx_meter_test.py	Post-key deadline, CW gap, native ALC, Icom unkey gate	Yes	In scope; blocker 3 on the budget claim
tools/test_tx_meter_test.py	5 new cases	Yes	In scope
tests/automation_persist_diagnostics_test.cpp + tests.cmake	New socket-free target, registered in both AETHER_SETTINGS_CONSUMERS and AETHER_AUTOMATION_SERVER_TESTS (both loops at tests.cmake:4714/4739 run after the target definition — registration is correct)	Yes	In scope
tests/icom_civ_scheduler_test.cpp, tests/icom_incident_telemetry_test.cpp	Event-ID and freshness cases	Yes	In scope
docs/automation-bridge.md, docs/automation/TX_TEST_PROMPT.md, 3 research docs	Contract + retractions	Yes	In scope

No CHANGELOG.md edit. Nothing unrelated bundled. Everything else in the diff is explained by the stated intent.

Socket-test surfacing: the PR adds no socket-owning test. automation_persist_diagnostics_test.cpp constructs AutomationServer but never calls start() — the constructor at AutomationServer.cpp:2307 is empty and QLocalServer is only created inside start() — and the stub backend answers invokeExtension synchronously with no peer. Genuinely socket-free, as the header comment claims. Note that CI does not run any of the three tests: ci.yml's only ctest -R steps are the DV/cross_needle_meter_test/mac_nr_filter_test/asr_gpu_probe_test selections. Green CI here means "it compiles on three platforms", not "these tests pass" — the mutation evidence in the body is the only evidence they do.

Blockers

1. The two-tone refusal is family-shaped and leaves the identical defect on HL2 and RTL. (AutomationServer.cpp:6883, inline.) The argument for refusing is exactly right — TransmitModel::startTwoToneTune calls setTuneMode("two_tone"), which emits commandReady("transmit set tune_mode=two_tone"), and grepping the head checkout, tune_mode is read only by FlexBackend.cpp:922. But that means the same is true of every non-Flex backend, and Hl2Backend::setTune says so in its own comment: "The HL2 has no tune generator of its own, so it is the built-in test tone at ZERO offset — a carrier exactly on the TX NCO." So txtest twotone on an HL2 still returns {"ok":true,"txtest":"twotone"} over a single carrier — the exact mislabeled-IMD-evidence failure this PR exists to correct, still shipping. Either widen the guard to "no two-tone route on this backend" (a RadioCapabilities flag, which is also what Constitution II/III's capability-shaped-not-family-checked rule points at) or say explicitly in the body that HL2/RTL are knowingly left.

2. PTT frame acceptance narrowed in the fail-closed path, and untested. (IcomCivBackend.cpp:2905, inline.) !frame.data.empty() → frame.data.size() == 1 && frame.data[0] <= 1. A 1C 00 frame outside that shape is now dropped before reaching any of the block's logic — including the branch whose own comment reads "The radio says it is keyed while we asked it to stop. Publish it and say so — this is the fail-closed path" and "when the unkey was lost, refused, or overridden at the front panel, that report is the only thing that says so." Constitution VI wants a path that can transmit to fail closed; this makes an unexpected encoding fail silent. Reading it as reasoned-from-code: I have no evidence an Icom ever sends a 2-byte or >1 PTT payload, so this may never fire in practice — but the change is in a TX-safety path, is not mentioned in the body, and the new tests cover malformed SQL ({0xFA}) and out-of-range AGC ({0xFF}) while adding no malformed-PTT case. Either keep publishing data[0] != 0 as keyed and use the strict shape only to gate confirmState, or add the negative test that pins the intended drop.

3. The high-SWR and measured-watt aborts were narrowed, and TX_TEST_PROMPT.md says they weren't. (tools/tx_meter_test.py:197 and :184, inline.) The SWR ceiling check moved from 0 <= swr_age < FRESH_MS (1500 ms) to 0 <= swr_age < min(SAFETY_FRESH_MS, elapsed * 1000) — so a 600 ms-old SWR of 4.0 that previously set stop_reason is now discarded. fresh_peak gained fwd_age <= elapsed * 1000, doing the same to the measured-watt backstop inside the first ~500 ms. The 0.9 s deadline stop bounds the exposure, so this is a delay rather than a hole — but the doc's added line "Missing/stale telemetry and a missing ratio with positive power still stop the run; the timing budgets are unchanged" is not accurate for these two checks. The post-key rule is right for qualifying evidence; for an abort a stale-but-alarming reading is still a reason to stop. Either restore the wider window for the two ceiling comparisons (keeping the tight one for aggregation), or correct the doc.

Nits

• Icom detection in the harness sniffs the serial string (tools/tx_meter_test.py:77, inline). Fail-open shape in a TX gate.
• meterObservation's 1500 ms cut is a new magic number applied to low-rate vitals (AutomationServer.cpp:2224, inline), and it reimplements predicates MeterModel already has (hasPaTemp(), hasSupplyVoltage() — the latter's header comment describes this exact "definition landed, value didn't" trap).
• AGC behavior change is undisclosed (IcomCivBackend.cpp:2338, inline): a 16 12 reply of 0x00 previously published "med" via the else branch; it is now dropped. Every sibling case in that switch (kNoiseReduce, kPreamp, kMonitorFn) reads data[0] under only the group-level data.empty() guard, so AGC is now uniquely strict. Probably correct — worth one line in the body.
• pending has no expiry. stateFreshness() checks pending ahead of every other status, and it is cleared only by a successful confirmState or disconnectRadio(). A tracked write whose readback never lands — or is rejected by the stale-generation / PTT-intent guards — leaves that field pending and trackedStateReady false for the rest of the session. All six fields are polled today (the baseline table proves it), so it self-heals; a comment saying why it self-heals would keep a future poll-cadence change from silently breaking it.
• confirmState's header comment is slightly stronger than the code. "Called only after decode and stale-generation/PTT-intent rejection" — in the PTT block the observation == Stale early return sits in the else if after if (m_pendingPttIntent), so a stale-generation PTT frame arriving while an intent is pending and matching that intent reaches confirmState.
• micp = [] is now dead (tx_meter_test.py:166): the PR deletes its only append. It was already unread on main, so no behavior change — but the variable should go with the append.
• Header layout: m_diagnosticInstanceId / m_confirmedState / m_stateContext and the ConfirmedState struct are interleaved between stateFreshness() and schedulerTransactionTrace() in the private method block (IcomCivBackend.h:294–308), splitting the declarations.
• CodeGuard CG-PATH-001 (tools/tx_meter_test.py:28) is a false positive and I've dropped it: line 28 is the Usage: example inside the module docstring, not filesystem code. No path is constructed from untrusted input anywhere in the diff.

What I tried to break

• doCiv being async inside doRadioCert. If civ scheduler returned before the extension answered, backendDiagnostics.result would be empty on real hardware while the stub-backend test passed. It doesn't: doCiv connects extensionResult/extensionError with Qt::DirectConnection around a synchronous invokeExtension and reports answered (AutomationServer.cpp:8328–8355). Holds.
• Meter field-name drift. meterObservation reads name/source/has_value/age_ms/value/unit; MeterModel::meterToJson emits exactly those, and age_ms is -1 whenever has_value is false, so the fed = has_value && age >= 0 conjunction can't be satisfied by a defined-but-unfed meter. "PATEMP" and "+13.8A" are the real declared names (IcomMeters.cpp:119, Hl2Backend.cpp:5159, MeterSurfaces.h:104/107). Holds.
• Ring-eviction arithmetic in the new scheduler test. 140 enqueue/observe rounds against a 128-entry ring gives front().eventId == 13, back() == 140, and 141 after clearTransactionHistory() + reset(). The assertions are right, and event.eventId = ++m_transactionEventId is set before any early return in recordTransaction, so a timeout and its late reply do get distinct IDs as the doc claims.
• Whether the new test compiles. StubBackend overrides all eleven IRadioBackend pure virtuals (capabilities, connectRadio, disconnectRadio, isConnected, the four slice setters, setPanCenter, setKeying, invokeExtension); AutomationServerTestAccess is already a friend (AutomationServer.h:385); IcomCivBackendTestAccess already reaches m_lastIncident so the new m_confirmedState/queueWrite/m_civReported access is covered by the same friendship. The new members are in a plain private: section, not the private slots: block, so moc won't try to register them.
• Whether test_previous_burst_sample_cannot_satisfy_safety proves anything. It does, but by a narrow margin: with POWER_SAMPLE_DEADLINE_S patched to 0, the first post-settle sample lands near elapsed ≈ 0.2 s, where fwd_age = 400 > 200 fails fresh_peak and trips the stop before any sample can be aggregated. Had the first sample landed past elapsed = 0.4 s, the 400 ms age would have qualified and result["fwd"] is None would fail. Real, but wall-clock-sensitive.
• swr_gap_is_observed accepting a boolean. type(power) in (int, float) excludes bool (since type(True) is bool), and the gap requires power == 0 exactly, so a residual 0.001 W keeps the stop armed. Fail-closed as documented.
• The Icom serial gate actually firing. Grepping every RadioInfo construction with family == "icom", the only producer is ConnectionPanel.cpp:2921, and automationConnectByIp routes through onManualConnectClicked() to that same site — so today the prefix always matches and the gate does engage. That's the reason it's a nit and not a blocker; it's the coupling, not a live failure.

All findings are reasoned from the head checkout at /tmp/aetherclaude/pr-5516; I have no build and no hardware, so nothing here was reproduced at runtime. I could not test the live-evidence table in persist-icom7300mk2-p1-followup-2026-09-08.md (the wattages, the 2.3–2.5 s readiness times, the executable SHA-256s) — that needs the IC-7300MK2 on the bench, and the report is appropriately explicit that those repeats establish exercised paths rather than a root cause.

Recommendation

Request changes — on blocker 1 and blocker 3, both of which are small. Blocker 1 is either a capability flag or one sentence in the body admitting HL2/RTL still mislabel; blocker 3 is either two comparisons restored or one doc sentence corrected. Blocker 2 is a judgment call I'd like the maintainer to make explicitly rather than have it ride in as a side effect of a diagnostics PR. The bridge-surface additions (temperature/voltage, stateFreshness, transactions[].eventId, backendDiagnostics, and the paTemp/supplyVolts null semantics) are a maintainer decision on their own — third parties bind to these and the scalar change is a contract break for existing readers, documented but not versioned.

The engineering here is careful and the retraction of the earlier two-tone claims is exactly the right instinct. Thanks for writing the corrections into the old reports instead of quietly editing them.

---

_{🤖 aethersdr-agent · cost: $14.3601 · model: claude-opus-5}

[aethersdr-agent]

Blocker — the reasoning is right but the scope is Icom-only, and the sibling backends have the identical defect.

The premise checks out: TransmitModel::startTwoToneTune calls setTuneMode("two_tone") → commandReady("transmit set tune_mode=two_tone"), and grepping the head checkout, tune_mode is consumed only by FlexBackend.cpp:922. But that is a statement about every non-Flex backend, not about Icom. Hl2Backend::setTune documents its own single-tone nature:

The HL2 has no tune generator of its own, so it is the built-in test tone at ZERO offset — a carrier exactly on the TX NCO

So after this merges, txtest twotone on an HL2 (and on RTL) still returns {"ok":true,"txtest":"twotone"} while producing a single carrier — the exact "do not record ordinary TUNE output as two-tone or IMD proof" failure the PR is correcting, still live on two families.

A family == check is also the shape Constitution II/III steers away from. Suggest a RadioCapabilities flag (hasTwoToneTune, defaulting false, set true only by FlexBackend) so the guard is "this backend has no two-tone route" rather than "this radio is an Icom" — that fixes all three families at once and won't need editing when the next backend lands. If you'd rather keep the narrow fix, please say in the body that HL2/RTL are knowingly left, so the next reader of TX_TEST_PROMPT.md doesn't infer the verb is safe everywhere else.

[aethersdr-agent]

Blocker — this narrows a TX-safety path, is not mentioned in the body, and has no test.

!frame.data.empty() → frame.data.size() == 1 && frame.data[0] <= 1. A 1C 00 frame outside that shape is now dropped before reaching any of this block's logic — including the branch a few lines down whose own comment is:

The radio says it is keyed while we asked it to stop. Publish it and say so — this is the fail-closed path.

and

Swallowing an unexpected ON after an unkey request costs the operator any indication that the radio is still on the air — when the unkey was lost, refused, or overridden at the front panel, that report is the only thing that says so.

Constitution VI wants that path to fail closed; the new guard makes it fail silent for any payload shape we didn't anticipate. I have no evidence an Icom ever sends a 2-byte or >1 PTT payload — this is reasoned from the diff, not observed — but it's a change in the keying path made in service of a diagnostics feature, and the new tests cover malformed SQL ({0xFA}) and out-of-range AGC ({0xFF}) while adding nothing for malformed PTT.

Two ways out, either fine:

• keep publishing data[0] != 0 as keyed and use the strict shape only to gate the confirmState("ptt", …) call, so the diagnostic tightens without the safety path narrowing; or
• keep the drop and add the negative case that pins it, so the intent is recorded rather than inferred.

[aethersdr-agent]

Blocker (paired with line 184) — the SWR ceiling abort was narrowed, and TX_TEST_PROMPT.md says the budgets weren't touched.

Before: 0 <= swr_age < FRESH_MS (1500 ms). Now: < min(SAFETY_FRESH_MS, elapsed * 1000) (≤500 ms, and less than that early in the window). Because the swr_val > max_swr ceiling check lives inside this same if, an SWR of 4.0 with a 600 ms age that previously set stop_reason is now silently discarded. The same shape at line 184 (fwd_age <= elapsed * 1000 folded into fresh_peak) does it to the measured-watt backstop for the first few hundred ms after key.

The 0.9 s deadline stop bounds the exposure, so this is a delay rather than a hole — but the doc line added in this PR reads "Missing/stale telemetry and a missing ratio with positive power still stop the run; the timing budgets are unchanged," and for these two comparisons that isn't so.

The post-key rule is exactly right for qualifying evidence (a sample predating the key isn't this burst's telemetry). It's the wrong direction for an abort: a stale-but-alarming reading is still a reason to stop transmitting, not a reason to keep going. Suggest splitting the two concerns — keep min(SAFETY_FRESH_MS, elapsed * 1000) for what enters swr/fwd/peaks, and keep the original FRESH_MS window for the > max_swr and > max_watts stop conditions. If you'd rather keep one window, please correct the doc sentence.

[aethersdr-agent]

Fail-open shape in a TX gate. The Icom confirmation requirement — the one TX_TEST_PROMPT.md says model flags alone can't satisfy — only engages when radio.serial starts with "icom:". That string has exactly one producer in the tree: ConnectionPanel.cpp:2921, the manual/network connect path, where the comment calls it a fallback identity ("No discovery means no MAC and no reported serial, so the host is the only stable identity this radio has for us. It has to be SOMETHING"). The Icom backend never overwrites it.

Today that always matches — automationConnectByIp routes through onManualConnectClicked() to the same site — so the gate does fire, which is why this is a nit rather than a blocker. But the failure mode is silent and one-directional: any future Icom path that supplies a real chassis serial (or discovery growing an Icom entry) downgrades this to the model-flags-only check with no error and no log line, while the C++ side of the same feature keys off m_radioModel->family().

radioSnapshot() (AutomationServer.cpp:1862) doesn't expose family, which is presumably why the serial got used. Adding {"family", …} there would let this read self.g("radio", "family") == "icom" and match the authority the rest of the feature uses. Cheaper alternative with no C++ change: call cmd=civ action=scheduler unconditionally and treat ok:false as "not an Icom" — the verb already refuses non-Icom backends, so it can't fail open.

[aethersdr-agent]

Two things about this line, neither blocking.

The 1500 ms cut is a new magic number with no comment. MeterModel::kTxMeterStaleMs is 2000 and the header notes RigctlProtocol uses 1500 — so 1500 is at least precedented, but nothing here says which it's meant to match or why the same constant governs a 4 Hz TX meter and a low-rate PA-temperature vital. On a radio whose PATEMP/+13.8A cadence is slower than 1.5 s, paTemp and supplyVolts will flap between a number and null on successive get meters calls.

It also re-derives predicates the model already owns. MeterModel::hasPaTemp() and hasSupplyVoltage() exist for precisely this, and the latter's header comment describes the same trap this function is guarding against — "m_supplyIdx is set when the meter DEFINITION lands, while m_supplyVolts stays at its 0.0f initialiser until a '+13.8A' VALUE packet lands … exactly the fabricated reading this accessor exists to prevent." Reconstructing that from the all array means two implementations of "is this a real reading" that can drift.

Worth noting in the body that paTemp/supplyVolts now go null where they previously carried the last known value, and that this deliberately disagrees with the GUI, which uses hasPaTemp() and keeps displaying the last reading rather than blanking on age.

[aethersdr-agent]

Undisclosed behavior change, worth one line in the body.

This doesn't only gate confirmState — it returns, so a 16 12 reply outside 1..3 no longer publishes a SliceDelta at all. Previously v == 0 fell into the else and published "med". That was arguably wrong, so the tightening is probably an improvement, but it means the AGC control now holds its previous value where it used to change, and the PR presents this hunk as freshness validation.

It also makes AGC uniquely strict in this switch: the group already guards frame.data.empty() at line 2214, and every sibling (kNoiseReduce, kNoiseBlanker, kAutoNotch, kPreamp, kMonitorFn, kVox) reads data[0] with no length check of its own. If a 1-byte payload is the invariant for cmd::kFunction, that belongs at 2214 for all of them; if it isn't, this case needs a comment saying which radio motivated it.

[aethersdr-agent]

Cisco CodeGuard — static analysis of this PR (1 finding(s))

• [MEDIUM] CG-PATH-001 — Potential path traversal in tools/tx_meter_test.py /tmp/aetherclaude/pr-5516/tools/tx_meter_test.py:28

_{Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them.}

---

_{🤖 aethersdr-agent · cost: $15.7711 · model: claude-opus-5}