[on8st]

Refs #6387, #6388

Answers rfoust's three optional notes on the approving reviews of #6387 and
#6388.

Limits

• One documentation sentence, one comment and five include lines. No
  behaviour change: no statement, declaration or test registration is
  touched, and the five headers were already reached through <QtTest> and
  <QApplication>.
• Built and run on one Mac (macOS, Qt 6.12.0). Per-PR CI compiles these
  tests but does not run them. The Linux rule the sentence describes is not
  run on Linux here; it is exercised by audio_device_preference_policy_test
  on this host.
• The documentation now names the five ID patterns used by the Linux hot-plug
  filter. The source matches substrings or a prefix, including custom names;
  it does not establish any audio server's default device names.
• The include pass covers the Qt classes test(audio): pin which device-id list answers presence. Principle VIII. #6387 and fix(gui): dimmed Radio Audio Outputs rows look dimmed. Principle VIII. #6388 named first in the
  files they touched. Transitive includes that were there before them are
  listed below and left alone.

Change

1. test(audio): pin which device-id list answers presence. Principle VIII. #6387, docs/user/docs/audio-settings.md:28:

   this reads as applying to all audio-server virtual devices, but the
   exception depends on five ID patterns. A custom-named virtual device
   outside those patterns restores live.

   The reviewer's suggested sentence was the starting point. The follow-up
   commit names all five matching ID patterns, including monitor sources and
   custom names. isLinuxTransientAudioDeviceId leaves those ids out of the
   arrival list, so a saved device matching one is never seen arriving; at
   startup the saved id is looked up among all enumerable devices.

2. test(audio): pin which device-id list answers presence. Principle VIII. #6387, src/core/AudioDevicePreferencePolicy.h:54:

   this comment should describe test portability without claiming every
   lane runs it.

   The suggested wording is taken as written.

3. fix(gui): dimmed Radio Audio Outputs rows look dimmed. Principle VIII. #6388, tests/radio_setup_headphone_output_gate_test.cpp:38:

   Include it directly rather than relying on QtTest to supply it
   transitively, per the repository explicit-include convention.

   <QSignalBlocker> is included. The same pass over both PRs found four
   more Qt classes fix(gui): dimmed Radio Audio Outputs rows look dimmed. Principle VIII. #6388 named first in that file without their header:
   QColor, QCoreApplication, QList, QStringList. They are included
   too.

Found by the same pass and not changed, because the use predates both PRs:
QString and QWidget in that test; QString in
mixer_control_availability_test.cpp, RadioSetupDialog.cpp and
TitleBar.cpp; QByteArray and QList in MainWindow.cpp. The other
files of #6387 include what they name.

Evidence

Full ctest before the documentation-only follow-up, at ac0dd8f6, -j8:
770 of 772 passed (4 skipped: the
nr_stereo_independence_* set), 2 failed:

• peripheral_auth_dialog_interaction_test: "dialog 960x680, viewport 658,
  page minimum 784" (feat(peripherals): authenticate direct TGXL, PGXL, and Antenna Genius connections #6008); fails on clean main on this Mac, also on the
  serial rerun. This diff touches no dialog file.
• vkamp_connection_test: one of its two known checks, "commands after a
  disconnect are dropped, not queued" (Replace retained socket fixtures with injected and opt-in coverage #5254), at -j8; it passed on the
  serial rerun.

radio_setup_headphone_output_gate_test,
audio_device_preference_policy_test and mixer_control_availability_test
passed. tools/docs/gen_reference.py --check, check_test_registration.py --strict and gen_touchpoint_manifest.py --check pass. At the follow-up
commit f7dd95c9, npm ci && npm run build in docs/user/ passed, and the
source checks remained green.

73, ON8ST

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

No linked issue, and none is wanted: this is a Refs #6387, #6388 follow-up answering three optional notes on those PRs' approving reviews. GOVERNANCE.md asks for an issue or RFC for architectural changes, not for a docs/comment/include cleanup with a clear root cause, so the Refs form is right here. All three notes are addressed, each with the suggestion quoted in the body, and the "no behaviour change" claim holds — I could not find a statement, declaration, or registration change anywhere in the diff.

One claim I want to flag as unverified rather than accepted: that each rewording is "the reviewer's wording … taken as written." The body quotes rfoust's three suggestions as blockquotes, but those live in PR review bodies on #6387/#6388, and the tools I have return issue and bot comments only — list_issue_comments on #6387 gave me CodeGuard plus on8st's reply, not the review. So I cannot confirm the quoted text against the source. That is a one-glance check for the maintainer who wrote them, and it is the main thing left.

Scope

File	What it changes	Claimed?	Verdict
docs/user/docs/audio-settings.md:28	One sentence narrowed from "a saved virtual device of the audio server" to "some saved virtual devices, including default-named echo-cancel and combined devices"	Yes — note 1	In scope
src/core/AudioDevicePreferencePolicy.h:54-55	Comment text only; no executable line touched	Yes — note 2	In scope
tests/radio_setup_headphone_output_gate_test.cpp:35-48	Five #include lines added	Yes — note 3, and the title's "; includes"	In scope

Everything in the diff is explained. No CHANGELOG.md entry (correct). No new public surface, no settings key, no capability field, no protocol verb. No socket test added or removed — the touched test's only network-shaped symbol is QHostAddress(QHostAddress::LocalHost) at line 170, an inert value carrying the comment // never dialled, with no bind/listen/connectToHost anywhere in the file.

Reading the - lines: no guard, early return, or issue-citing comment was deleted. The one removal is the clause so each CI lane tests it, and that is the point of the change rather than a loss — see below.

Blockers

None.

Nits

• src/core/AudioDevicePreferencePolicy.h:52-55 — the comment correction is stronger than the body claims, and worth recording as such. The old text read // to offer (#2864). Compiled on every platform so each CI lane tests it. That was not merely loose, it was false: .github/workflows/ci.yml runs exactly seven named tests across its ctest -R steps (aether_dv_waveform_no_args, thumbdv_queue_test, dstar_model_test, digital_voice_waveform_process_test, cross_needle_meter_test, mac_nr_filter_test, asr_gpu_probe_test), and audio_device_preference_policy_test is in none of them. No per-PR lane runs it at all. The replacement — "so tests can exercise the Linux rule on any host" — is accurate. Non-blocking; just worth knowing this fixed a wrong comment rather than reworded a vague one.

• docs/user/docs/audio-settings.md:28 — "default-named" undersells the filter in one direction the body's Limits section does not cover. The rule is substring matching:

      return id.contains(".monitor")
          || id.startsWith("pulse_input_loopback")
          || id.contains("auto_null")
          || id.contains("echo-cancel")
          || id.contains("combined");

  Because echo-cancel and combined go through contains(), a custom-named device that keeps the substring — my-echo-cancel-sink, combined_out — is also left out of the arrival list, so it is not restored either. The body's Limits note says only that "these are the audio servers' default names is not checked"; the substring breadth is a separate point. "including" hedges the sentence enough that it is not wrong, and this is the maintainer's own wording, so I would not change it without their say. Context: the #6387 thread already parks contains("combined")'s over-breadth for its own issue off #6378.

• docs/user/docs/audio-settings.md:28 — the sentence names two of the five patterns. The unnamed three are .monitor, pulse_input_loopback, and auto_null. Of those, a .monitor source is the one an operator plausibly saves on purpose — it is the normal way to capture system audio as an Input — and it gets the same no-auto-restore treatment. Again "some … including" keeps it true, so this is a completeness nit on an Input-side case, not a correction.

Other tools' findings

All 10 CodeGuard CRITICALs are lexical false positives, and none sits on a line this PR touched — I verified each against the file and had the security-audit subagent confirm independently:

• CS-VLN-CMDI-PIPE × 4 at src/core/AudioDevicePreferencePolicy.h:59-62 — the || continuation lines of the single return quoted above. C++ logical-or, not a shell pipe.
• CS-VLN-CMDI-AMP × 5 at :56, :65, :79, :105, :110 — C++ reference declarations (const QByteArray& id). Two of them (:65, :79) carry no & at all; the tool is reporting the line after the one it matched.
• CS-VLN-CMDI-BACKTICK at tests/radio_setup_headphone_output_gate_test.cpp:74 — // The capabilities the production backend for \family` declares…, a Markdown backtick quoting a parameter name inside a //` comment. Pre-existing text, only renumbered by the added includes.

Neither file contains QProcess, system(, popen, exec*, or startDetached, so command injection has no vehicle here. No credential, crypto, or certificate findings. These same false positives fired on #6387 one line higher, which confirms the scan reads the whole file at the merge tip rather than the diff.

What I tried to break

• "The five headers were already reached through <QtTest> and <QApplication>" — tested the inverse risk, that they are unused churn. All five are genuinely used: QColor at lines 197 and 321, QCoreApplication at 341 and 437, QList at 344 and 345, QSignalBlocker at 215, QStringList at 322. So these are real include-what-you-use fixes, not dead additions, and they are in alphabetical order in the existing block. build, check-macos, and check-windows are all green on ac0dd8f, which is exactly what green proves here — the includes compile on all three toolchains.
• Tried to falsify the doc's load-bearing factual claim, that these devices are "used again at the next launch." It holds. The churn filter has only one production caller — arrivalAudioDeviceIds at src/gui/MainWindow.cpp:5226-5228, the hot-plug path. The startup restore at src/core/AudioEngine.cpp:1989-1998 matches the saved id directly against QMediaDevices::audioOutputs() / audioInputs() with no filtering at all (grep -c for the policy header in AudioEngine.cpp returns 0). So a filtered id is restored at launch and never on reappearance, as written.
• Checked the mechanism behind "rather than automatically restored when they reappear." decideAudioDeviceChange reaches RestorePreferred only via change.addedIds.contains(change.preferredId), and addedIds is built from arrivalIds, which the filter has already stripped — so a filtered id can never take that branch. Separately, liveMissing reads presentIds, which is unfiltered, so presence detection and the FollowDefault path are unaffected. The doc and the code agree, and the two sentences do not contradict each other.
• Checked whether the narrowing went too far and broke the surrounding paragraph. It reads as a general rule followed by a Linux exception, consistent with the preceding "It switches back to the saved device when that device connects, without asking." The "Build docs site" check is green, which is the broken-link and anchor gate.
• Confirmed both relevant tests are registered, at tests/tests.cmake:3158 and :3765, so nothing here counts as coverage on an unregistered or retired target. No new test is needed: there is no behaviour change to pin, and audio_device_preference_policy_test.cpp:191-203 already pins all five id patterns including echo-cancel-source and combined.
• Could not check: the quoted reviewer wording, as described under Issue fit. Everything above is reasoned from the source in the head checkout and from CI metadata, not reproduced at runtime — I have no build and ran no test. The two unrelated local failures the body reports (#6008, #5254) are consistent with my notes on both.

Recommendation

Approve. The diff is three narrowly-scoped edits that each do what the body says, the one factual claim in the new doc sentence checks out against both the policy header and the startup path, and the comment change corrects a statement that was actually wrong about CI. The nits are wording observations on a sentence the maintainer supplied; none needs to hold the merge.

Next step: whoever wrote the three notes on #6387/#6388 confirms the quoted wording is theirs — the only thing I could not verify from here — and this can go in. Thanks for the unusually legible body, and for listing the transitive includes you found and deliberately left alone; that made the scope audit a two-minute job instead of a twenty-minute one.

---

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

[aethersdr-agent]

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

• [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:59
• [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:60
• [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:61
• [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:62
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:56
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:65
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:79
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:105
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/core/AudioDevicePreferencePolicy.h /tmp/aetherclaude/pr-6403/src/core/AudioDevicePreferencePolicy.h:110
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in tests/radio_setup_headphone_output_gate_test.cpp /tmp/aetherclaude/pr-6403/tests/radio_setup_headphone_output_gate_test.cpp:74

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

[on8st]

Thanks for the review. Nothing to change, so no new commit.

The wording you could not verify is @rfoust's, from his inline notes on the approving reviews. On #6387, docs/user/docs/audio-settings.md:28, his suggestion block reads: "On Linux, some saved virtual devices, including default-named echo-cancel and combined devices, are used again at the next launch rather than automatically restored when they reappear." On src/core/AudioDevicePreferencePolicy.h:54: "Compiled on every platform so tests can exercise the Linux rule on any host." Both are taken verbatim. The include is his note on #6388, tests/radio_setup_headphone_output_gate_test.cpp:38.

On your two points about the sentence: agreed that the rule is substring matching, so a custom name that keeps echo-cancel or combined is left out too, and that a .monitor source saved as an Input gets the same treatment. "Some … including" keeps the sentence true; I have left his wording as it is. The over-broad contains("combined") stays parked for its own issue, as on #6378.

73, ON8ST

[rfoust]

Issue fit

PR #6403 follows the optional review notes on #6387 and #6388. The documentation now describes the five Linux ID matches exactly, including monitor inputs and custom device names containing echo-cancel or combined. The policy comment and direct Qt includes answer the other notes. No behavior-changing statement or test registration changed.

Scope

File	Change	Verdict
docs/user/docs/audio-settings.md	Clarifies the Linux hot-plug exception using the actual ID matches	In scope
src/core/AudioDevicePreferencePolicy.h	Corrects a test-portability comment	In scope
tests/radio_setup_headphone_output_gate_test.cpp	Includes five Qt classes used by the test	In scope

The PR body now describes the follow-up accurately. No unrelated files or new public surface were added.

Blockers

None.

Nits

None remaining. The follow-up removes the unverified “default-named” claim and covers the previously omitted .monitor, pulse_input_loopback, and auto_null matches.

Verification

• I compared the sentence against isLinuxTransientAudioDeviceId, the hot-plug arrival list, and the unfiltered startup restore path. The original inline notes on #6387/#6388 were checked directly.
• The touched test adds includes only; its QHostAddress is an inert value, with no socket bind, listen, or connect added.
• On the signed follow-up commit f7dd95c9, npm ci && npm run build passed in docs/user/; generated-reference, test-registration, touchpoint-manifest, and diff checks passed. The socket-free audio policy test from the prior head passed 63/63 checks. GitHub verified the new signature.
• Required CI checks passed on the updated head. No app runtime test was needed for this documentation/comment/include-only diff.

What I tried to break

I checked custom names containing the filtered substrings, saved monitor inputs, all five ID patterns, startup versus hot-plug behavior, direct use of each added Qt header, and current-main mergeability. None produced a remaining finding.

Approve. The corrected wording is now exact, the other two follow-ups are narrow, and the required checks passed on the signed head.