[w5jwp]

Merge order: #6008 has merged (4d5b9893). This PR is replayed onto main (tip 5fae2f86 when pushed) and no longer carries any #6008 commits.

Summary

Closes #6026. Radio Setup → Peripherals shows a persisted device list and a per-device detail page, with Add, Remove, and a per-device automatic-connection toggle (Connect directly when available on TGXL and PGXL, Connect automatically on Antenna Genius and ShackSwitch). This implements the ruling on #6026 ("approved, with conditions"); the checklist below maps each condition to its commit.

Ruling checklist

#	Condition	Where
1	Per-device builders, no grid, no reparenting, no itemAtPosition; list wording DIRECT / RADIO / OFFLINE	1d96a707
2	Add creates and selects the row, nothing else	feaaefe8
3	Remove asks for confirmation; outcome table in this body and in the user doc	feaaefe8, a8300ace
4	Per-device automatic-connection toggle; Remove does not dismiss; no DiscoveryDismissed key	feaaefe8, da7df5c7
5	PeripheralSettings.h note corrected	feaaefe8
6	AG/ShackSwitch endpoint-scoped teardown and tests kept	replayed in 06a20738
7	VisibleDevices seeding, AG reconciliation and refresh follow signals; the 250 ms timer is gone	1d96a707
a	The detail page's status line is always visible and is the accessible state	1d96a707
b	A disabled Add entry shows and speaks its reason ("(already added)")	1d96a707
c	Docs describe the feature as it is	a8300ace
d	Status is carried explicitly (PeripheralDeviceStatus); nothing searches text for Error: or a dash	1d96a707
e	The #6026 reply and this PR's state agree: ready for review	this body

Also from the 2026-10-02 and 2026-10-03 reviews: the removal lease now stops every route to a socket (explicit, automatic, alternate, reconnect timer), checked before attempt state changes (06a20738, 38931177); a blocked recovery row shows the endpoint that refused the code; an address saved from the AG applet while a deletion is pending survives the Remove; an AG or ShackSwitch row with no address can be removed beside the other device's stored code; a disabled Remove button reads as disabled (37baf0e1).

Connecting automatically

TGXL, PGXL, Antenna Genius and ShackSwitch each have a toggle, on by default, stored as Peripherals.<id>.AutoConnect. Off blocks startup, discovery/radio-presence, alternate-address and reconnect attempts for that device. An explicit Connect always works and does not change the toggle; a session it starts is not reconnected after it drops.

• TGXL and PGXL: Connect directly when available. Off, the device is controlled through the radio's relay whenever the radio offers one (the applet shows RADIO); a live direct session continues until it drops.
• Antenna Genius and ShackSwitch: Connect automatically. No relay exists, so off leaves the device offline until Connect.

Every path that opens a socket emits attemptStarted() on TgxlConnection, PgxlConnection and AntennaGeniusModel, so Setup shows Connecting… for automatic attempts as well. The authorization-blocked recovery row shows the endpoint that refused the code unless the operator has edited the field.

Remove outcomes

Outcome	Configuration	Credential	Reconnection	Recovery
Success	Settings and row removed; toggle back to default	Stored code deleted	Guard released; discovery may connect the device again unless its row is kept with its toggle off	Add re-creates the row
Failed (denied or error)	Kept; row stays	Saved code remains; error shown on the row	Connection stopped, guard released; normal reconnect events may connect it again	Retry Remove
Unconfirmed (15 s timeout), then late completion	Kept; the row's pending field edits are not saved	Deletion unconfirmed	Guard held until the request returns, then released; a late completion changes no row or newer setting	The Peripherals page stays disabled in that Setup window. Close Setup and reopen it to retry; restart the app if the backend never returns
No credential backend	Same as success	Session copy cleared; stored-code deletion unconfirmed, with a notice saying so	Same as success	Remove the code from the OS vault once it is available
Unknown owner (Antenna Genius, ShackSwitch)	Kept	Untouched; the owning endpoint is not known	Connection stopped, guard released	Connect the device to establish its address, then retry Remove
An earlier removal still pending	Unchanged	Unchanged; the earlier request owns the slot	Remove and Connect are refused with a notice	Retry after the keychain request finishes

Scope

Peripherals UI and the persistence, credential-store and connection-lifecycle changes it needs. No authentication protocol, dependency, thread, radio family or TX command is added. TGXL/PGXL/AG/ShackSwitch endpoints keep their flat keys; no migration. The three applets now take their DIRECT / RADIO / OFFLINE strings from PeripheralConnectionSource.h (strings unchanged). Touchpoint manifest regenerated.

Test plan

• Desktop build on Linux; peripheral_auth_dialog_test, peripheral_auth_handshake_test, peripheral_auth_keychain_test, both automation-redaction tests, tgxl_*, amp_applet_test and the radio_setup_* tests pass. pgxl_panel_test fails on unchanged main too (a layout height assertion) and is unrelated.
• Mutations fail their tests, then pass restored: the removal guard removed from the socket-opening path; the Connect automatically gate removed from ShackSwitch discovery; Remove no longer resetting the toggle; the compare-and-clear of a newer endpoint; the shared-slot ownership check.
• gen_touchpoint_manifest.py --check, check_engine_boundary.py --strict (0 blocking), check_a11y.py on the new file (0 findings); grep itemAtPosition src/gui/RadioSetupDialog*.cpp is empty.
• CI on the final commit.
• Driven on the demo backend (isolated settings, DEMO-0001): Add persists across a restart; the toggles read "Connect directly when available" (TGXL, PGXL) and "Connect automatically" (AG, ShackSwitch); with the AG toggle off a discovered Antenna Genius stays OFFLINE through repeated beacons (with it on, it connects); Remove asks first and Cancel keeps the row.
• Not verified: a real OS vault, ShackSwitch/AG hardware, native accessibility on Windows/macOS. The focused tests are not in any ci.yml -R filter.

CI: the focused tests above are not in any ci.yml -R filter, so green CI does not mean they ran; that filter set is frozen per PR and is left to the maintainer.

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

Partially — against RFC #6026 and #6026's acceptance criteria, the UI-only commits (0063b943, d8aad51e) do what they claim, and I could not break the two hardest parts: the reconnect lease and the bounded close. Every connectTo* entry point is gated (three funnels, no bypass — see "What I tried to break"), and checkRemovalTimeout is a real regression, not a tautology. One acceptance criterion does not hold as written: "Remove reports pending, successful, failed and unconfirmed credential cleanup accurately" and "Removing ShackSwitch preserves a live AG session" — the ShackSwitch row never asks the vault to delete anything, and the AG row deletes a slot the ShackSwitch may own (finding 1).

The layout/workflow decision itself is correctly routed: #6026 carries maintainer-review and states approval is outstanding. I am not treating the redesign of the existing Peripherals table as smuggled preference — it went through an RFC, which is exactly what GOVERNANCE.md asks for. It is a maintainer call, not a blocker.

Scope

Group	What it changes	Claimed by this PR?	Verdict
docs/peripherals-settings-ui.md, docs/architecture/aetherd-touchpoint*	new workflow doc; manifest 230→232, AppSettings 103→105, ThemeManager 156→157	yes	In scope; counts match the tree
src/core/PeripheralSettings.h	VisibleDevices, DiscoveryDismissed, clearDeviceConnection in the existing nested Peripherals doc	yes	In scope. No new flat keys — Principle V satisfied
src/core/PeripheralRemovalGuard.h, guard checks in TgxlConnection.cpp:82 / PgxlConnection.cpp:81 / AntennaGeniusModel.cpp:316, m_connectTransport seams	transient reconnect lease + socket-free injection	yes	In scope
src/gui/RadioSetupDialog.{cpp,h} (+1139 lines)	list/detail, Add/Remove, pending + 15 s timeout, AG reconciliation	yes	In scope, pending the #6026 ruling
src/gui/PeripheralAuthStore.*	delete-completion / cache-retention handling	yes	In scope
src/models/AntennaGeniusModel.*	deferred ShackSwitch request/retry	yes	In scope
src/gui/MainWindow_Wiring.cpp	discoveredTarget() on the three discovery fallbacks	yes	In scope
tests/peripheral_auth_dialog_test.cpp (+1349), peripheral_auth_keychain_test.cpp, fakes/PeripheralAuthStoreFake.*	socket-free coverage	yes	In scope
Inherited #6008 commits 120ed439…2e58e6f4: .gitignore, CMakeLists.txt, AutomationSensitiveLineEdit.h, AutomationServer.cpp, LogManager.cpp, PeripheralAuthCode*.h, the TGXL/PGXL/AG auth state machines, AmpApplet/AntennaGeniusApplet/TunerApplet/ShackSwitchApplet, tools/probe_4o3a_auth.py, 6 test files	no — disclosed in the body as the prerequisite	Needs maintainer sequencing. 7,179 insertions / 49 files land if this branch merges as-is. The disclosure is honest and unusually clear, but the unbundling is "merge #6008 first", not something the diff does
AmpApplet source-indicator moved to bottom-right + tests/pgxl_panel_test.cpp assertions rewritten	visual relocation of an existing applet widget	neither title claims it	Belongs to #6008's review, not this one. Flagging so it does not slip through on a UI-titled PR

No CHANGELOG.md edit — correct.

Socket tests (surfaced, not a finding): this PR adds no new socket-owning test; the eight focused tests are all injection-based (I grepped the added sources for QTcpServer/QUdpSocket/listen/connectToHost — clean). It does modify one existing loopback test, tests/pgxl_panel_test.cpp (binds QTcpServer::listen(QHostAddress::LocalHost, 0) at line 121), inside the inherited auth commits; the change is layout assertions only. Neither that test nor any of the eight focused targets appears in ci.yml's -R filters, so see the CI note below.

Blockers

None that I can confirm at this severity. One finding is a genuine contradiction of a stated invariant but its blast radius depends on firmware I cannot observe, so I have labelled it plausible rather than certain — see inline comment 1 and the request below.

Findings

1. Remove is asymmetric across the shared AG credential slot (inline, RadioSetupDialog.cpp:10749). device.row >= 1 && device.row <= 3 puts ShackSwitch (row 4) on the else { finishRemoval(device); } path — no PeripheralAuthStore::clear, no PeripheralRemovalGuard. There is exactly one AG slot (SettingsCredentialPolicy.h:39, antenna_genius_auth_code) and MainWindow_Wiring.cpp:6524 saves authCodeAccepted under Device::AntennaGenius regardless of target, while the AUTH trigger at AntennaGeniusModel.cpp:581 keys only on the greeting, not the device name. So removing ShackSwitch leaves its code in the vault, and removing AG deletes whichever target owns the slot. The #6026 invariant holds for the socket, not for the credential. Reasoned from code — I cannot confirm ShackSwitch firmware sends AG AUTH. Either gate row 4 the same way, or state in the doc that ShackSwitch never authenticates.

2. The removal timeout discards unrelated pending edits (inline, RadioSetupDialog.cpp:10789). m_peripheralRowSavers.clear() drops every row's saver, not the removed row's. Clear the ACOM address, then Remove TGXL, then time out and close: the ACOM clear is silently lost. The doc says "closing does not save abandoned field edits" — this drops edits that were never part of the abandoned Remove.

Nits (non-blocking)

• listGroup->setFixedWidth(250) / deviceList->setFixedHeight(340) (RadioSetupDialog.cpp:10244, :10266) are hard pixels. The evidence doc records a prior clipped-control problem at the default dialog size; fixed extents at 125 % scaling or a larger UI font are how that comes back. setMinimumWidth + a stretch would be safer.
• docs/peripherals-settings-ui.md:7 lists seven supported pages and omits LP-100A Meter, which devices includes (row 9, RadioSetupDialog.cpp:10232) and which finishRemoval handles.
• Two id conventions now coexist in the same Peripherals JSON root: VisibleDevices/DiscoveryDismissed use lowercase ("tgxl", "acom"), while clearDeviceConnection/deviceString use "Acom"/"Vkamp"/"Lp100a". Functional, but a future endpoint migration will have to reconcile them.
• PeripheralRemovalGuard.h:33: std::array<unsigned, 3> non-atomic, with the main-thread requirement carried only in a comment. Nothing enforces it and the header sits in core/.

What I tried to break

• Guard bypass. Grepped every connect path: connectToTgxl, connectToPgxl, AntennaGeniusModel::connectToDevice are the only three places reaching connectToHost, and both connectToAddress overloads funnel through connectToDevice. The gate is at the funnel, not at optional call-site wrappers. No bypass found.
• Guard leak on teardown. The clear() callback is registered against qApp, captures removal by value, and releases on lambda destruction on every exit — including if (!self || *timedOut) return; and the request.context-null branch in startNextWrite. Destroying the parent window mid-delete does not free the lease early. Held.
• Double Remove after timeout. The timeout sets m_peripheralRemovalPending = false, which looked like it re-opened Remove — but removeButton is a child of content, which stays setEnabled(false), and PeripheralRemovalGuard::pending() early-returns anyway. Two independent stops. Held.
• Test quality. checkRemovalTimeout (tests/peripheral_auth_dialog_test.cpp:433) asserts connects == 0 while the lease is held and connects == 1 after late completion, across 3 devices × success/failure × dialog-alive/destroyed, with the deadline stopped and its timeout injected rather than slept. Emptying the gate or the release would fail it. It is not asserting the implementation back to itself.
• finishRemoval ordering. Checked that SS_ManualIp is removed before disconnectFromDevice(), so the MainWindow.cpp:2266 handler sees an empty key and hides the SS button. Correct order. Also verified kPortKeys[4] == "SS_ControlPort" matches buildRow(4, …) at :9297 — no stale-port leak.
• Could not check: anything at runtime. No build, no test run, no GUI. Every finding above is reasoned from the head checkout at /tmp/aetherclaude/pr-6027, not reproduced. Windows/Linux behavior, native accessibility, real OS-vault semantics and ShackSwitch AUTH behavior are all unverified here — as the PR body itself states.

CI note: 5/5 green on d8aad51e (build, check-macos, check-windows, sanitizer configure, static checks). But none of the eight focused tests — peripheral_auth_dialog_test, peripheral_auth_handshake_test, peripheral_auth_keychain_test, the two automation-redaction tests, tgxl_docked_parity_test, tgxl_applet_ports_test, amp_applet_test — appears in any ctest -R filter in ci.yml. Green means the tree compiles on three platforms; it does not mean this PR's coverage ran. The contributor's local run is currently the only evidence they pass.

Recommendation

Needs maintainer decision, on two independent axes, then request changes on finding 1.

The sequencing is the first call: as it stands this branch carries all of #6008, including an AmpApplet indicator relocation that neither PR title mentions. The contributor has been scrupulous about disclosing this and is not asking for a merge — the ask is to review 0063b943 + d8aad51e on top of #6008, which is a reasonable request and what I did. The second call is #6026 itself: layout, Remove semantics, TGXL/PGXL discovery dismissal, and the blocking-close behavior are all yours to approve.

Concrete next step: rule on #6026, then ask for finding 1 (gate row 4 the same as rows 1–3, or document that ShackSwitch never authenticates) and finding 2 (scope the saver clear to the removed row) before the post-#6008 replay. Findings 3–6 are cleanup that can ride along. This is careful, well-instrumented work with an unusually honest evidence boundary — the credential asymmetry is the one place the implementation and the stated invariant have drifted apart.

---

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

[aethersdr-agent]

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

• [MEDIUM] CG-PATH-001 — Potential path traversal in src/core/TgxlConnection.h /tmp/aetherclaude/pr-6027/src/core/TgxlConnection.h:20
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6027/src/gui/MainWindow.cpp:9549
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6027/src/gui/MainWindow.cpp:9661
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6027/src/gui/MainWindow.cpp:9780
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6027/src/gui/MainWindow.cpp:9781
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6027/src/gui/MainWindow.cpp:9782

_{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: $13.9818 · model: claude-opus-5}

[w5jwp]

@aetherbot @aethersdr-agent — pushed 5c07925 addressing this review.

1. Shared AG/ShackSwitch credential ownership: both Remove paths now acquire the shared reconnect lease and use the bounded deletion workflow. clearForEndpoint reads the existing slot and deletes only a matching endpoint; the other endpoint's code survives. A queued conditional delete rechecks endpoint/revision before it starts, preserving a replacement save. An unknown offline hostname fails closed with an explicit instruction to connect the selected device to establish its peer address before retrying. This makes no claim about whether ShackSwitch firmware emits AUTH. Tests cover each row, owned/other credentials, unknown identity, queued replacement and the live shared-model isolation already covered.
2. Unrelated pending edits: timeout now suppresses only the selected row's pending clear-on-close edit. It no longer clears the saver vector. The regression clears an ACOM endpoint before timing out another device's Remove and confirms the ACOM clear still persists.
3. Layout: replaced the fixed list width/height with minimum extents and expanding layout space. Dialog tests pass at 125% and 150%; a 125% offscreen demo screenshot shows the TGXL auth fields and Show/Clear/Add/Remove controls fitting without clipping.
4. Documentation/contracts: added LP-100A Meter to the user guide. Explicitly documented stable lowercase UI IDs versus the existing case-sensitive connection-object names, preserving compatibility instead of silently migrating operator data. Sharpened the removal guard contract: every non-atomic acquire, release and query must run on the main thread.
5. CodeGuard advisories checked: the TgxlConnection.h location is a protocol-format comment; MainWindow.cpp:9549 and :9661 are fixed widget/status text. The three :9780–9782 locations walk upward from Qt's own application executable path to its bundle for the existing relaunch code. Those locations contain no untrusted path component reaching a file read/write operation. I found no actionable path-traversal defect and made no unrelated relaunch changes or scanner suppressions.

Validation on the restored final code: desktop build, all eight focused tests (17.81 s), scaled dialog runs, registration, generated manifest and strict engine/capability/command-plane checks passed. Mutations that restored whole-page saver clearing, bypassed endpoint ownership, or excluded ShackSwitch from removal guarding each failed their regression test. The demo reported one slice, one panadapter, TX disabled/transmitting=false; the review-owned instance was stopped. No live firmware or actual OS-vault evidence is claimed.

#6027 remains ready for review, must merge after #6008, and still needs the maintainer's #6026 UI ruling. The final post-#6008 replay and CI are still required; these fixes do not imply approval or request automatic merge.

[ten9876]

Issue fit

Partially. This was reviewed as the increment over #6008 (git diff 71d5bcd7 0ef90af5, 22 files), against RFC #6026. The list/detail page, Add/Remove, the bounded close, and the reconnect lease all work as described. The previous review's two findings are fixed in 5c079250: the shared AG/ShackSwitch credential is now endpoint-scoped, and the timeout no longer drops unrelated edits.

Two #6026 acceptance points fail at this head:

• "Remove reports … failed and unconfirmed credential cleanup accurately" fails for AG and ShackSwitch when no keychain backend is available. I reproduced it (blocker 1).
• 5c079250 regressed ShackSwitch removal (blocker 2).

The RFC itself has no maintainer ruling yet, and #6008 has not merged. The body discloses both, so I list them as sequencing, not code findings.

Scope

File / group	What it changes	Claimed?	Verdict
src/gui/RadioSetupDialog.{cpp,h}	list/detail page, Add/Remove, 15 s bounded wait, AG reconciliation	yes (#6026)	in scope, pending the #6026 ruling
src/core/PeripheralRemovalGuard.h + gates in Tgxl/PgxlConnection.cpp, AntennaGeniusModel.cpp	transient reconnect lease	yes	in scope
src/core/PeripheralSettings.h	VisibleDevices, per-device DiscoveryDismissed in the existing Peripherals blob	yes	in scope; no new flat keys
src/gui/PeripheralAuthStore.*, PeripheralAuthConnectFlow.h	endpoint-scoped delete, configuredEndpoint, delete-completion handling	yes	in scope; see blocker 1
src/gui/MainWindow_Wiring.cpp	discoveredTarget() honours dismissal	yes	in scope; see M1
docs/peripherals-settings-ui.md, touchpoint manifest (+PeripheralRemovalGuard.h, peripheral(4o3a); AppSettings 104→105)	doc + manifest	yes	in scope; additions only
docs/4o3a-remote-auth-hardware-evidence.md (+181/−10)	rewrites part of #6008's evidence doc	partly	nit N5
tests: peripheral_auth_dialog_test (+1430), peripheral_auth_keychain_test, fake store	socket-free coverage	yes	in scope; see blocker 1 on the fake
everything else (≈28 files)	inherited from #6008	disclosed	reviewed on #6008

No CHANGELOG.md edit. Against main this branch is still 50 files, +7586/−268, until #6008 lands and it is replayed.

Socket tests: none added. The dialog test's Connect clicks are all refused before connecting (invalid code, empty address), and lifecycle tests inject m_connectTransport. The two pre-existing loopback PGXL tests are inherited from #6008, and the operator approved running them for this review.

Blockers

1. AG and ShackSwitch can never be removed when the keychain backend is unavailable, and the error says a code "remains in keychain" when none may exist.

src/gui/PeripheralAuthStore.cpp:397, in clearForEndpoint:

if (!entry.loaded || entry.status == LoadStatus::Unavailable) {
    callback(ClearResult::Failed);

load() leaves loaded = false and sets Unavailable for every read error except EntryNotFound (:270-283), including NoBackendAvailable and NotImplemented.

TGXL and PGXL go through clear(), which maps the same backend loss to SessionCleared and lets removal proceed. Your own keychain test states the invariant: "No OS backend must not prevent local removal, even if this device never had a stored code."

Reproduced. I added a temporary probe to tests/peripheral_auth_keychain_test.cpp, which uses the production adapter with the QtKeychain double. It calls clearForEndpoint(AntennaGenius, …) and fails the read with NoBackendAvailable. Then it retries the same call.

aether.peripheral.auth: keychain read failed: "no backend"
PROBE called=1 result=2 (Cleared=0 SessionCleared=1 Failed=2)
aether.peripheral.auth: keychain read failed: "no backend"
PROBE retry called=1 result=2

So Linux without Secret Service/KWallet, on a QtKeychain build, can never remove AG or ShackSwitch. "Retry Remove" fails forever.

The dialog test can't see this. Its fake (tests/fakes/PeripheralAuthStoreFake.cpp:144-155) falls through to clear() whenever the endpoint matches, so production's Unavailable → Failed branch is never exercised.

Fix: on Unavailable with NoBackendAvailable/NotImplemented, return SessionCleared as clear() does (keep Failed for AccessDenied). Add a keychain-test case through clearForEndpoint, plus a fake that models the read failure.

2. Removing a connected ShackSwitch leaves its applet button on screen (regression from 5c079250).

• RadioSetupDialog.cpp:10789 calls m_ag->disconnectFromDevice() when Remove starts, while SS_ManualIp is still saved.
• disconnectFromDevice() emits disconnected synchronously. The handler at MainWindow.cpp:2276-2285 hides the SS button only if SS_ManualIp is already empty, so it keeps the button.
• finishRemoval case 4 (:10661-10665) now only calls setAuthCode({}). At d8aad51e it called disconnectFromDevice() after the keys were removed, and that second disconnected is what hid the button. Nothing emits it now.
• The button for a removed device stays until restart or rediscovery.

Traced in source, not driven: the demo has no ShackSwitch. A fix is to hide the SS button explicitly in finishRemoval for row 4, or to re-emit through a model call once the keys are gone.

Maintainer decisions (#6026)

• M1. Remove used as "undo" permanently dismisses discovery.
  • Bridge evidence: on this head, in demo mode with an isolated store, I ran Add → Tuner Genius XL, then Remove without entering anything. The store then held:

    Peripherals|{"VisibleDevices":[],"tgxl":{"DiscoveryDismissed":true}}
    

  • RadioSetupDialog.cpp:10639 sets the flag unconditionally for TGXL/PGXL. From then on, discoveredTarget() suppresses direct auto-connect to a discovered TGXL/PGXL on every radio connect, across restarts. The applets stay visible on the relay, so nothing tells the operator.
  • TGXL loses manual relay control (#469) and PGXL loses direct telemetry (Vac/Vdd/HL/fan/MEffA). Add or an explicit Connect undoes it, but only an operator who knows the flag exists will think to.
  • At minimum, only dismiss when the device was actually configured or discovered.
• M2. The RFC ruling itself: the layout, Remove semantics, discovery dismissal, and the blocking close.

Nits (non-blocking)

• N1. ShackSwitch credential can be orphaned while Remove reports success. The SS connect path saves under port 9007, but offline Remove computes configuredEndpoint(SS_ManualIp, SS_ControlPort) (:10772-10776). If the port spin ever differed, the endpoints don't match and clearForEndpoint returns Cleared without deleting (PeripheralAuthStore.cpp:405-407). The doc's "fails closed" covers only an empty host. Reasoned from code.
• N2. a11y (#4896).
  • None of the Removing / timed-out / failed / succeeded notices calls QAccessible::updateAccessibility, so screen readers don't announce them (docs/a11y.md, live value updates).
  • Observed on the bridge: after typing a code, the field's accessibleDescription still reads "Code blank. A saved code will be checked when connecting." :11092-11098 keys only on the saved status, never on the field's text.
  • Accessible names are inconsistent on one page (TGXL authorization code / Tuner Genius XL show authorization code / TGXL clear saved authorization code), and the port spin box has no accessible name.
• N3. i18n. :11026 compares detailStatus.startsWith(tr("Connecting")) against the untranslated setText("Connecting...") at :8871. In a translated build that shows "Offline" while connecting. Accessible names are also built by concatenation (device.label + tr(" address")), which breaks word order in translation.
• N4. After a timeout, close and reopen, Remove and Connect do nothing while the lease is still held. Remove returns silently at :10759, and Connect leaves "Connecting..." stuck. A message naming the pending deletion would help.
• N5. This PR rewrites #6008's own evidence-doc sentences (+181/−10), which will conflict with any further #6008 edit. It also still says Setup "prevents editing or closing until the vault completes", which contradicts the 15 s bound. A separate evidence doc for #6027 would avoid both.

What I verified vs. read

• Built 0ef90af5 on Linux and ran the 13 focused targets: 13/13 pass, including peripheral_auth_dialog_test (6.7 s) and the two inherited loopback PGXL tests (operator-approved).
• Reproduced blocker 1 with the production adapter (output above). The temporary probe was reverted.
• Bridge: offscreen, isolated AETHER_SETTINGS_DIR/HOME, explicit socket, AETHER_AUTOMATION_NO_TX=1, attached to DEMO-0001 only (get radio → "model":"AetherSDR Demo","serial":"DEMO-0001"). LAN discovery listed the operator's FLEX, which was never connected.
  • Verbs: menu list, invoke "Radio Setup..." trigger, invoke radioSetupNavigation setCurrentText Peripherals, invoke peripheralAddButton click, invoke "Tuner Genius XL" trigger, invoke peripheralAuth_tgxl_4 setText, Show, peripheralRemoveButton click, dumpTree, grab, then a sqlite read of the store.
  • The empty-state page, Add menu (8 devices), detail page, redaction and Remove all behaved as described, apart from M1 and the N2 description. The instance was closed.
• Gates: all four checks are flat versus #6008 (102 findings; EB3 rows in RadioSetupDialog.cpp shift lines only). The manifest check passes, and the tag sidecar has one addition.
• Attacks that held (reasoned from code, read-only second pass):
  • every connectTo* funnel is gated by the lease;
  • the lease is released on every path, including owner teardown;
  • a dialog destroyed mid-delete is a safe no-op;
  • Remove twice, and Add during a pending delete, are both blocked;
  • late completion after a timeout never applies the abandoned Remove;
  • keychain callbacks stay on the main thread, as the non-atomic guard requires.
• CI is stale: the earliest run on this head was created 2026-10-01 22:26Z, before main's 2026-10-02 19:07Z tip. The branch now conflicts with main in the same three files as #6008, all inherited. The focused tests are not in any ci.yml -R filter.
• Not verifiable here: a real OS vault, ShackSwitch/AG hardware, and Windows/macOS accessibility.

[w5jwp]

Thanks for the review. Pushed 74bf2e89. This branch merges #6008 (and through it origin/main) with no force-push.

Blocker 1 (d0780723): clearForEndpoint records the read error on the entry. On NoBackendAvailable / NotImplemented it clears the session value and returns SessionCleared, as clear() does; AccessDenied and other errors still return Failed. A later load retries the vault. A new peripheral_auth_keychain_test case goes through clearForEndpoint with a NoBackendAvailable read, including the retry. The fake store can now model a read failure, and a dialog case uses it. With the fix disabled, the new keychain case fails.

Blocker 2 (b1fa3ef1): RadioSetupDialog emits peripheralRemoved(id) at the end of finishRemoval. MainWindow hides the ShackSwitch button for that row, and hides the AG button for the AG row unless another AG is connected or discovered. A dialog test asserts the signal fires.

M1 mitigation (b1fa3ef1): Remove captures, before it disconnects, whether the device had a saved manual IP or a live or connecting session, and finishRemoval sets DiscoveryDismissed only in that case. Add → Remove of a never-configured TGXL/PGXL leaves it unset. Left for your ruling (M2/#6026): the layout, Remove semantics, whether Remove should ever dismiss discovery, and the blocking close.

Nits

• N1 (b1fa3ef1): one kShackSwitchControlPort (9007) is used by the connect path and the removal endpoint.
• N2 (b1fa3ef1): removal notices update the accessibility tree. The code field's description follows whether it has text. Row names use one tr("%1 …").arg(label) pattern, and the port spin boxes are named.
• N3 (b1fa3ef1): the connecting state is a property instead of startsWith(tr("Connecting")); the status text goes through tr(); names use tr("%1 address").arg(label).
• N4 (b1fa3ef1): Remove and Connect show a notice naming the pending deletion when a lease is still held after a timeout.
• N5 (f41882ba): this PR's evidence moved to the new docs/peripherals-removal-evidence.md, feat(peripherals): authenticate direct TGXL, PGXL, and Antenna Genius connections #6008's sentences are restored, and the "until the vault completes" sentence describes the 15 s bound.
• Mixed id casing (tgxl vs Acom, from the earlier bot review): known, left unchanged.

One test change you should look at: #6008's N1 means a keychain outage no longer blocks, so peripheral_auth_dialog_test modelled the blocked-device case wrong. 74bf2e89 makes it use a missing saved code instead. checkRemovedDiscovery also now sets a saved manual IP first, since a never-configured device no longer dismisses.

Built and ran the focused targets locally; peripheral_auth_* pass. Not verified: a real OS vault, ShackSwitch/AG hardware, native accessibility on Windows/macOS.

[w5jwp]

@ten9876 the updates from your review are pushed (74bf2e89). The itemized reply is above. Ready for another look when you have time.

[ten9876]

Issue fit

Partially. #6026 is approved with conditions (ruling). The list/detail direction, the 15 s bound and lease, the excluded flat-key migration and AG/ShackSwitch isolation all stand.

At 74bf2e89 the branch doesn't meet the ruling's conditions, and it can't merge as is:

• #6008 has landed on main (4d5b9893), with a fallback rework that this branch hasn't seen.
• git merge-tree origin/main 74bf2e89 conflicts in about 15 files.
• CI ran at 15:25Z, before #6008 merged, so the green describes a merge that no longer exists.

Reviewed as the increment over #6008's old head: git diff 22b1496f...74bf2e89, 24 files.

Scope

File / group	What it changes	Claimed?	Verdict
src/gui/RadioSetupDialog.{h,cpp} (+1123/−157)	device list, detail stack, Add/Remove, bounded close, recovery rows	yes	in scope; layout does not meet ruling item 1
src/core/PeripheralSettings.h	VisibleDevices, DiscoveryDismissed in the Peripherals document	yes	in scope; DiscoveryDismissed is superseded by ruling item 4
src/core/PeripheralRemovalGuard.h, {Tgxl,Pgxl}Connection.*, AntennaGeniusModel.*	removal lease checked at connect, m_connectTransport test seam	yes	in scope; see blocker 2
src/gui/PeripheralAuthStore.*, PeripheralAuthConnectFlow.h, MainWindow{,_Wiring}.cpp	configuredEndpoint, endpoint-scoped clear, deletion completion, discovery wiring	yes	in scope
docs/architecture/aetherd-touchpoint*	+PeripheralRemovalGuard.h (2 GUI includers), AppSettings.h 105→106 via PeripheralAuthConnectFlow.h	yes (numbers stale: body says 231→232, now 235→236)	in scope; additions only, no retag
docs/peripherals-settings-ui.md, docs/peripherals-removal-evidence.md	user doc + evidence	yes	in scope; see nit N3
tests/peripheral_auth_{dialog,keychain}_test.cpp, tests/fakes/PeripheralAuthStoreFake.*	1,690 test lines	yes	in scope

Nothing in the diff is unexplained. CHANGELOG.md is untouched.

Socket preflight: the new dialog cases call connectToTgxl("192.0.2.9x"), connectToPgxl and connectToDevice, but every one goes through the injected m_connectTransport (installed via PeripheralConnectionTestAccess::injectConnect). No socket opens, and the tests are socket-free as the body says.

Blockers

1. Replay onto main, and keep the removal lease on every connect path.
On main, TgxlConnection::connectToTgxl is one of three thin wrappers. autoConnectToTgxl and tryAlternateTgxl call openSocket() without going through it (same in PgxlConnection). This branch puts the PeripheralRemovalGuard::pending() check and the m_connectTransport seam only inside connectToTgxl (src/core/TgxlConnection.cpp:85-87, src/core/PgxlConnection.cpp:84-86). Kept there through the merge, the lease no longer covers:

• the presence and startup handlers, which call autoConnectTo* on main;
• the reconnect timer when its target came from an automatic attempt;
• the queued unreachable → tryAlternate* fallback. An unreachable queued before the Remove click is delivered after it, and reconnects while the keychain delete is still running.

checkPendingRemoval drives only connectToTgxl, so it stays green while those three paths reconnect.

Fix: check the guard before begin*AttemptAt in all three entry points, or in one shared pre-check, and move m_connectTransport into openSocket(). Then add a case per entry point. Also, src/gui/RadioSetupDialog.cpp:11107 fills the recovery row from lastHost()/lastPort(). On main, an alternate attempt deliberately leaves the reconnect target (reconnectHost()) on the manual host while auth was refused at the alternate address, so the "authorization blocked" row would show the wrong endpoint. Use attemptHost(), and drop lastHost() in favour of main's reconnectHost().

2. Ruling item 1: the detail pages are the old grid, reparented.
buildPeripheralsTab still builds the nine-row grid and moves its cells into detail pages (src/gui/RadioSetupDialog.cpp:10353), with six itemAtPosition lookups left in place. The ruling asks for a per-device builder. The list also doesn't use the accepted wording. Driven on the demo backend after Add:

→ cell peripheralDeviceList 0 0
← "text": "Tuner Genius XL\nAddress not set\nOffline"

Use DIRECT / RADIO / OFFLINE, as the applets do.

3. Ruling item 2: Add writes connection intent.
src/gui/RadioSetupDialog.cpp:10733 calls setDiscoveryDismissed(device.id, false) on Add. Driven on the demo backend: an Add with nothing else done persisted

app_settings Peripherals = {"VisibleDevices":["tgxl"],"tgxl":{"DiscoveryDismissed":false}}

Add should create and select the row and write nothing about intent. Only Connect restores it.

4. Ruling item 3: Remove has no confirmation and no outcome table.
Driven on the demo backend: I selected the TGXL row and clicked peripheralRemoveButton. The device went immediately, with no confirmation window, and the list read rows: 0. The destructive credential delete (src/gui/RadioSetupDialog.cpp:10889) needs a confirmation first. The body and docs/peripherals-settings-ui.md also need a table giving the configuration, credential, reconnect and recovery result for success, failure, unconfirmed/timeout and no backend.

5. Ruling item 4: replace DiscoveryDismissed with a per-device "Stop auto-connecting" toggle.
src/core/PeripheralSettings.h keys dismissal by device type only ("tgxl"/"pgxl"), so removing a TGXL on one radio suppresses TGXLs on every radio, and an AG is never dismissed. Remove should no longer imply dismissal. Drop the key in favour of a visible per-device setting.

6. Ruling item 7: drive the presentation from model signals, not the 250 ms timer.
presentationTimer->start(250) (src/gui/RadioSetupDialog.cpp:11268) re-derives the list, re-sets accessible descriptions and writes the Peripherals document from the timer. Driving it from model signals also removes the visibility check that searches the label text for tr("Error:") or an em dash (:11181-11182).

7. A completed AG Remove deletes an address saved meanwhile from the AG applet.
src/gui/RadioSetupDialog.cpp:10795-10800 removes AG_ManualIp/AG_ManualPort without checking them. Sequence:

1. Remove AG while the keychain delete is pending (for example, a macOS prompt is open).
2. The AG applet stays usable, so AntennaGeniusApplet::tryManualConnect writes AG_ManualIp=B (src/gui/AntennaGeniusApplet.cpp:272). The guard silently suppresses its connect.
3. The delete finishes inside the 15 s window, and finishRemoval erases B.

That breaks the body's claim that stale Setup values never overwrite newer settings. Compare-and-clear against the values captured at the Remove click. Reasoned from code; the demo backend has no AG.

Should fix

• S1. An unconfigured AG or ShackSwitch row can't be removed while the other device's code is saved. In the Remove handler (src/gui/RadioSetupDialog.cpp:10933-10947) and src/gui/PeripheralAuthStore.cpp:415-421, the empty-endpoint check returns UnknownOwner before the different-owner check returns Cleared. Add an AG with no address while the ShackSwitch's code is in the shared slot, then Remove the AG: the notice tells the operator to connect a device that has no address, and connecting would tear down the live ShackSwitch. checkSharedCredentialRemoval always sets the IP key, so nothing covers it. Treat "no resolvable endpoint, slot bound to the other device" as Cleared. Reasoned from code.
• S2. The src/core/PeripheralSettings.h:20-21 header note still says TGXL/PGXL/AG/ShackSwitch "still use their own flat keys, not this class". That's no longer true now that VisibleDevices lives in the document (ruling item 5).

Nits (non-blocking)

• N1. Disabled Remove looks enabled. With an empty list, peripheralRemoveButton reports enabled: false, but it draws exactly like the enabled Add button (grabbed on the demo backend). Give its disabled state the disabled-text token.
• N2. a11y.
  • The detail page hides its status label except for errors, so a screen-reader user on the detail form has to go back to the list to hear state.
  • Disabled Add-menu entries give their reason only through setStatusTip (:10750, :10774), and a dialog has no status bar.
  • (docs/a11y.md, #4896.)
• N3. Docs. docs/peripherals-removal-evidence.md narrates PR history ("PR #6008 reflowed…"). Move that to the PR body and keep the docs to the current behaviour.
• N4. Body. It still says "UI approval is pending", has the manifest at 231→232 and calls the PR draft-dependent. Update it after the replay.
• N5. After a timeout, content stays disabled and the late completion returns early, so the page stays dead until Setup is reopened. That's acceptable given the notice, but the user doc should say so.

What I verified vs. read

• Built 74bf2e89 on Linux. peripheral_auth_dialog_test, peripheral_auth_keychain_test and peripheral_auth_handshake_test pass.

• Mutations: each of these fails peripheral_auth_dialog_test, then restored green:

  • removing the TGXL removal guard (TgxlConnection.cpp:85-87);
  • removing Add's setDiscoveryDismissed(false);
  • removing Remove's setDiscoveryDismissed(true).

  The tests pin the behaviour as built. Ruling items 2 and 4 change that behaviour, so those assertions change with it.

• Gates: engine-boundary, manifest, capability-records and command-plane are flat against #6008's head. The manifest change is the additions above, with no retag.

• Bridge: offscreen, isolated AETHER_SETTINGS_DIR, attached to DEMO-0001 only (get radio → "AetherSDR Demo"; the relaunch connected to nothing).

  • Radio Setup → Peripherals → Add "Tuner Genius XL" → the row read "Offline", and {"VisibleDevices":["tgxl"],"tgxl":{"DiscoveryDismissed":false}} was persisted.
  • The row survived a relaunch (rows: 1).
  • Remove went through with no confirmation (rows: 0).
  • The instance was closed.

• Attacks that held (read):

  • Removal lease lifecycle: the guard is RAII and owned by one shared_ptr. No path double-decrements it, and QtKeychain's finished fires on the main thread.
  • Bounded close: done() and closeEvent() both check the flag, and a late completion checks a QPointer.
  • Endpoint isolation: clearForEndpoint keeps the other endpoint's record; peripheral_auth_keychain_test covers the production path.
  • Credential exposure: <hidden> holds even after Show, and nothing new is written to AppSettings or logged.

• Not verified: a live TGXL/PGXL/AG, a real OS vault, and the AG/ShackSwitch paths at runtime (the demo backend has neither).

[w5jwp]

@ten9876 #6027 is replayed onto main after #6008 and ready for a fresh review. It answers your ruling on #6026 and both of your reviews (2026-10-02 and 2026-10-03); the checklist and outcome table are in the PR body, and the itemized reply is on #6026.

Not verified: the live app through the bridge on this head, a real OS vault, device hardware, native accessibility.

🤖 Generated with Claude Code

[w5jwp]

@ten9876 separate from this PR, found while replaying it: after the 4d5b9893 squash of #6008, nothing in src/gui connects TunerModel::relayedCommandRefused (src/models/TunerModel.h:152) any more, and git log -S relayedCommandRefused -- src/gui shows 4d5b9893 as the commit that touched it after #6044 (2d571294) added the hookup to showUnsupportedControlNotice(). I have not changed it here; it is outside #6027.

🤖 Generated with Claude Code

[ten9876]

Thanks for catching the dropped TunerModel::relayedCommandRefused hookup. The #6008 fallback rework (071ddd0b) replaced the wiring block that held #6044's connect. #6138 restores it verbatim and adds tgxl_refused_notice_wiring_test, which fails on 4d5b9893's source and passes with the fix.

[ten9876]

Approving at 6ed6f868.

Ruling on #6026, all conditions met:

• per-device-kind builders, with no grid and no reparenting;
• DIRECT / RADIO / OFFLINE wording;
• Add only creates the row;
• Remove confirms first and is covered by the outcome table;
• a per-device toggle replaces DiscoveryDismissed;
• signal-driven refresh, with no 250 ms timer;
• the header note and the a11y items.

Fixed in this pass (da7df5c7), each with a test that fails when the fix is reverted:

• An AG Remove finishes when a ShackSwitch beacon clears AG_ManualIp (checkRemovalIgnoresClearedAddress).
• An unaddressed shared row checks the other device's live session (checkRemovalBesideLiveSharedDevice).
• attemptStarted() from every socket-opening path, so automatic attempts show Connecting… (checkRemovalGuardStopsEveryConnectPath).
• The recovery row shows the endpoint that refused the code (checkRecoveryShowsRefusingEndpoint).
• Toggle wording per the maintainer's ruling; dead DiscoveryDismissed code, banners and a history note removed; close savers cleared with their devices.

Maintainer rulings:

• TGXL/PGXL Connect directly when available (off uses the radio's relay), AG/ShackSwitch Connect automatically;
• app-global per device type;
• a live session continues until it drops, and explicit Connect still works;
• the status wording, applet hiding on Remove, Remove resetting the toggle, and the extraction behaviour are accepted;
• the includer growth from the file split is accepted.

Verified empirically:

• Linux build of the head merged with main.
• The 9 focused targets pass.
• Mutations as listed, plus the earlier guard, Add and Remove mutations.
• The four static gates match main, and check_a11y reports 0 findings.
• On the demo backend: Add persists across a restart; the toggle wording is right; with the AG toggle off, a discovered Antenna Genius stayed OFFLINE through repeated beacons; Remove confirms, and Cancel keeps the row.

Not verified: a real OS vault, TGXL/PGXL/AG/ShackSwitch hardware sessions, and native accessibility on Windows/macOS. pgxl_panel_test (loopback) was not run on this PR.