Conversation
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Issue fit
Partially — by design, and the PR is honest about it. #5554 §2.6's end state has three parts: "wire classes move under backends/flex/, m_flexBackend and the direct stream/connection dereferences go, Flex emits the seam data-plane signals." This lands part 1 only, and the AGENTS.md edit says so in the right words ("Relocation does not convert a touchpoint", "their existing model/UI consumers are not converted by that move"). RadioModel still holds the FlexBackend* alias and still dereferences m_connection/m_panStream directly; nothing here claims otherwise.
It also quietly closes #5554 §2.8's first bullet — "src/core/RadioConnection.cpp:3 (a Flex wire class) includes sim/SimBackend.h" — via DemoRadioConstants.h. That's a real fix, not just motion, and it isn't mentioned in the AGENTS.md rationale as an issue reference; worth naming in the PR body so the tracker can tick it.
Scope
| File / group | What it changes | Explained by the stated intent | Verdict |
|---|---|---|---|
5 × src/core/{…} → src/core/backends/flex/ |
Pure git mv (--stat -M shows 0 content lines on CommandParser.{h,cpp}, SmartLinkClient.h, WanConnection.h; the rest are include-line edits only) |
Yes | In scope |
CMakeLists.txt, tests/tests.cmake |
Source paths follow the move | Yes | In scope |
17 consumer #include rewrites (gui/, models/, core/, tests/) |
Mechanical | Yes | In scope |
src/core/backends/DemoRadioConstants.h (new) + SimBackend.{h,cpp} + RadioConnection.cpp |
Breaks the flex→sim include inversion so the moved file doesn't carry it into backends/flex/ |
Yes (and #5554 §2.8) | In scope |
docs/architecture/aetherd-touchpoint*.{json,md} |
Tag keys re-pathed; DemoRadioConstants.h tagged vendor(sim) |
Yes | In scope |
AGENTS.md |
EB3 section restated: ratchet-only → relocated-but-unconverted | Yes | In scope |
tools/check_engine_boundary.py |
Docstring examples only; no logic change | Yes | In scope |
tests/radio_connection_session_test.cpp, tests/sim_backend_test.cpp |
New characterization assertions pinning the Demo serial and waterfall rate independently of the production constants | Yes (commit 3) | In scope |
Nothing in the diff is unexplained. No new public/protocol surface, no changed defaults, no removed guards (the only deletions are the four relocated core/ rows in the generated manifest, which reappear under core/backends/flex/), no CHANGELOG entry.
Blockers
None.
Nits (non-blocking)
DemoRadioConstantsis the 34thvendor(*)stem in the audit but the 34th slot inVENDOR_STEMS_PINNED(tools/check_engine_boundary.py:125-145) was not opened — the vocabulary is now 34 stems against a pinned set of 33. Arming needs no edit there by design, so nothing breaks today; the gap is that this one header could later be silently retagged non-vendor without the EB3-load error the pinned set exists to raise. Inline comment on the new file.src/core/DvkWavTransfer.cpp:4keeps the../core/backends/flex/…form while all 16 other consumers use the root-relativecore/backends/flex/…. Compiles either way; inconsistent.
CodeGuard
All eleven CG-PATH-001 findings are false positives and I've dropped them: three are #include "../…" lines in DvkWavTransfer.cpp, one is a docstring example in check_engine_boundary.py:153, and the five MainWindow.cpp / two MemoryDialog.cpp hits are on lines this PR does not touch. The ../ in an include path is not a filesystem operation.
What I tried to break
- A consumer left pointing at the old path. Grepped every
#includeof the five headers across the whole head checkout: 40 sites, all on the new path, and no forwarding header left atsrc/core/RadioConnection.het al. Also grepped non-C++ files (CMake, CI, tools, packaging) for the old source paths — nothing stale. - A build that only works for
aethercore. Onlyaethercorecompiles the moved TUs, and it exportssrc/as aPUBLICinclude root (CMakeLists.txt:2269), so the rewritten"core/AppSettings.h"-style includes resolve. The two standalone targets that compileCommandParser.cpp(tests/tests.cmake:3170,:5502) passPRIVATE src, which itscore/RadioMessageTypes.hinclude needs. CI is green onbuild,check-macosandcheck-windowsforba7bd7a, which is the real proof here. - A silently loosened ratchet.
is_below_seam()exempts a file either bysrc/core/backends/prefix or by exact vendor-TU path — the moved files were already exempt by the second clause, so the move is a no-op for EB3. Rancheck_engine_boundary.py --stricton this head and onmain: 102 findings both, same rows,src/models/RadioModel.hstill tracked forRadioConnection.h/WanConnection.h/PanadapterStream.h. Also checked every key inKNOWN_VENDOR_INCLUDE_BASELINEandKNOWN_WIDGETS_LEGACYstill resolves to a real file — no row went dead under the rename. All ninetools/check_*.py --strictpass on the head, andgen_touchpoint_manifest.py --checkreports the manifest up to date (so the hand-looking table edits match what the generator produces). - Demo behaviour drifting during the constant extraction.
SimBackend::demoSerial()now delegates toDemoRadio::serial(), same"DEMO-0001";SimBackend::kWaterfallRatealiasesDemoRadio::kWaterfallRate, same100. The new tests deliberately retype both literals rather than importing the constant, which is the right call — deriving the expectation from the production constant would have made a serial change invisible. - The new tests being vacuous or touching a real socket.
isSyntheticDemo()andpanWaterfallLineDurationChangedboth exist and are reached; the demo path never touchesm_socket(connectToRadioreturns beforeconnectToHost), and the non-demo path can't reach the OS becauseMemorySocketoverrides the virtualconnectToHost(QString,…)that the address overload dispatches to, andRadioInfo's default null bind address skips them_socket->bind()branch. Both new cases run withinit()never called, and the nullm_heartbeatis guarded on the arm (RadioConnection.cpp:135) and on teardown (:312). - What I could not check: I did not build or run anything, so every claim above is reasoned from the code plus the green check runs, not reproduced at runtime. I also could not read the PR body (GitHub CLI auth failed here), so I reviewed against the title, the three commit subjects and the AGENTS.md text; if the body makes a stronger claim than "relocation only", that claim is untested by me.
Socket-test disclosure: tests/radio_connection_session_test.cpp gains two cases that instantiate a QTcpSocket subclass, but neither binds, listens, nor connects — the transport is stopped at the virtual boundary and the tests assert socketDescriptor() == -1. No new target, no new fake firmware peer, no registration change.
Recommendation
Approve with nits. This is a clean, mechanical relocation with the one piece of real design work (the flex→sim include inversion) handled by extracting shared data rather than by a forwarding header or a wider include path, and the ratchet is provably no looser than on main. The two nits are cosmetic and can ride in this PR or a follow-up. Next step for the maintainer: confirm that landing §2.6 in slices — motion now, m_flexBackend removal and data-plane emission later — is the intended sequencing, since #5554 lists all three as one end state.
🤖 aethersdr-agent · cost: $11.1084 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (11 finding(s))
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: $11.2641 · model: claude-opus-5 |
|
Both inline nits are fixed and verified in 999e7f2. The description now explicitly includes #5554 §2.8's first include-inversion item, and links the recorded §2.6 slice-1 sequencing: #5554 (comment). This remains the prerequisite-free relocation slice; alias retirement, M4 command conversion and audio routing remain separate. I also checked the CodeGuard advisory: the reported paths are compile-time include/docstring text, unchanged UI strings and existing executable-location code, not an introduced runtime path-traversal flow. No scanner bypass or unrelated runtime change was added. Fresh follow-up verification: native ARM64 app/aetherd builds, 13/13 focused tests, negative boundary probes (including removal of the new pin), all static gates and isolated authenticated Demo MCP pass. The task-owned Demo process is stopped; no live radio or TX was used. Prior Windows/Linux evidence remains attributed to ba7bd7a, not this follow-up. CI is running on the new signed, GitHub-verified head. |
Summary
Implements the independently approved #5554 §2.6 slice 1 for the aetherd migration, including the first §2.8 include-inversion item (
RadioConnectiondepending onSimBackend). This is the prerequisite-free mechanical slice in the recorded implementation order. Related to #3849; the broader trackers remain open.RadioConnection,PanadapterStream,SmartLinkClient,WanConnection, andCommandParserintosrc/core/backends/flex/, updating explicit includes and CMake/test source paths. No forwarding headers or include-directory shortcuts.DEMO-0001identity and waterfall rate100into backend-onlyDemoRadioConstants.h.SimBackendkeeps its existing public helpers;RadioConnectionno longer includes the concrete simulator implementation.This is physical relocation, not M4 command conversion, alias retirement, audio-route conversion, or Stage 5 binary streaming. Existing above-seam consumers remain accounted for. It is independent of #5830; whichever change lands second must integrate the other and rerun the affected checks.
Constitution principles honored
Principles VIII and XI: preservation claims are backed by source comparisons, native builds, runtime observations, and mutation-tested assertions. Principle X: coordination is scoped to this slice while existing tracker assignments remain intact. No settings, credentials, TX policy, or user-visible defaults change.
Scope and pre-publication review
Two self-review passes completed (scope/issue fit, then lifetime/failure paths). The review added independent checks of the simulator's typed initial waterfall publication. No outstanding actionable findings. No separate automated or independent-agent review is claimed.
Test plan
Base:
d3743a9ba4fe199e3aede0d37dd8e3c9c589da8e. The cross-platform results below were obtained atba7bd7a68df642c0e522064ee5783c7ff7eace2c.MCP checks use the unchanged production
tools/aether_mcp.pyserver over MCP stdio, one task-owned authenticated app per platform, freshAETHER_SETTINGS_DIR, saved-radio autoconnect off, andAETHER_AUTOMATION_NO_TX=1. Verified Demo connect, serial, waterfall rate 100, slice frequency/mode changes, pan center, positive spectrum/waterfall liveness, disconnect/reconnect, and final disconnect. All proof processes are stopped afterward. No real radio or RF transmission is used.Focused targets:
radio_connection_session_test,sim_backend_test,radio_status_ownership_test,transmit_inhibit_policy_test,flex_backend_lifecycle_test,backend_family_switch_test,backend_slice_lifecycle_test,pcm_compatibility_test,panadapter_dbm_range_test,demo_backend_swap_test,tx_operation_integration_test,aetherclock_engine_test,waterfall_rate_test.Additional checks:
DEMO-0001 → DEMO-0002: both regression targets fail. Rate mutation100 → 48: both fail. Restored source rebuilt and all focused tests pass.--check, strict test registration, frozen CI test gate, andgit diff --checkpass. No CI allowlist growth.Review follow-up
999e7f244a5309288f6c28f0e3e2e8dca7e9e701adds the missing Demo vendor pin and normalizes the DvkWavTransfer include. Fresh macOS ARM64 app/daemon builds, the same 13 focused tests, and isolated authenticated Demo MCP proof pass. In-memory negative probes verify that retagging the Demo header as universal/mixed or removing its audit entry now blocks; removing the pin reproduces the bypass. All static gates above still pass. Windows/Linux results are from the preceding revision, not a new VM run for these two follow-up edits.Validation limits: this mechanical change does not claim live SmartLink/WAN or real-radio firmware coverage. The selected tests are not the entire repository suite. Native Demo rendering/liveness is observed, not a performance benchmark; process cleanup is not proof of graceful shutdown. GitHub CI on the follow-up is pending.
Checklist