[w5jwp]

Summary

Fixes #6358

unified_title_bar_test failed on macOS because its Alt+F assertion needs a menu mnemonic and Qt has none on macOS. This gates that assertion on whether mnemonics exist, and opens the File menu directly where they do not, so the check that follows it still runs.

Only tests/unified_title_bar_test.cpp changes (one include, one gate):

+#include <QKeySequence>
...
-        QTest::keyClick(&menuHost, Qt::Key_F, Qt::AltModifier);
-        check(QApplication::activePopupWidget() == fileMenu,
-              "Alt+F still opens the File menu with the menu bar hidden");
+        // Qt has no menu mnemonics on macOS, so there the menu is opened directly.
+        if (!QKeySequence::mnemonic(QStringLiteral("&File")).isEmpty()) {
+            QTest::keyClick(&menuHost, Qt::Key_F, Qt::AltModifier);
+            check(QApplication::activePopupWidget() == fileMenu,
+                  "Alt+F still opens the File menu with the menu bar hidden");
+        } else {
+            std::fprintf(stderr, "  mnemonics are off on this platform: Alt+F check skipped\n");
+            fileMenu->popup(menuBtn ? menuBtn->mapToGlobal(QPoint(0, menuBtn->height())) : QPoint());
+        }
         check(rounded(lateSubmenu), "opening a menu rounds the submenus added to it since");

Why

The test forces setNativeMenuBar(false) so every host exercises the inline-bar path. TitleBar::addAppMenuMnemonic (src/gui/TitleBar.cpp:1149) creates a QShortcut from QKeySequence::mnemonic(...) and does nothing if it is empty, which it always is on macOS. With no shortcut the File menu never opens, and the second check fails with it because submenus are rounded in aboutToShow (src/gui/RoundedMenu.h:26). It is a test problem only: on macOS the real menu bar is native and setMenuBar returns before any of this is built (TitleBar.cpp:1096-1097).

Review notes

• Gate on the capability, not the platform. A Q_OS_MACOS #ifdef would also be wrong on any Qt configuration that turns mnemonics on or off. Checking QKeySequence::mnemonic tests exactly the thing the assertion depends on.
• Linux/Windows are unchanged. There mnemonic("&File") is non-empty, so the first branch is the original code, byte for byte. CI's Ubuntu full suite keeps running it.
• The rounding check is not dropped on macOS. The else branch opens the menu with popup() anchored at the hamburger button, the same anchor the real mnemonic handler uses (TitleBar.cpp:1164), so aboutToShow fires and rounded(lateSubmenu) is still meaningful.
• The skip is visible. The stderr line makes the macOS run say what it did not test rather than passing silently.
• Known limit: the Alt+F mnemonic path itself cannot be exercised on macOS, because the platform has no such mnemonic. That is inherent, not something this change could recover.
• Not touched: the minimal-mode "mnemonics inert" check at line 1007 still passes on macOS, vacuously. It stays a true statement, and removing it would weaken Linux/Windows.

Constitution principle honored

Principle VIII, Evidence Over Assertion. A test that is red for an environmental reason teaches reviewers to ignore red; this makes the macOS result mean something again, and states in the output what it skipped.

Test plan

Run locally on macOS (Apple clang 21.0.0, Qt 6.12.0, RelWithDebInfo, QT_QPA_PLATFORM=offscreen via the CTest property):

• Reproduced on unmodified main first: the two FAIL: lines above, on repeated runs, and with all other changes stashed
• Local build passes: cmake --build build --target unified_title_bar_test, exit 0
• ctest -R '^unified_title_bar_test$': passes in 0.77 s, output includes mnemonics are off on this platform: Alt+F check skipped and unified_title_bar_test: all checks passed
• No new warnings from the file; the two it already had are unchanged (Q_OS_MAC at line 289, tracked in chore(build): migrate deprecated Q_OS_MAC to Q_OS_MACOS (Qt 6.12 clang deprecation warnings) #6354; setActiveWindow at line 983)
• Linux/Windows behavior: not run locally; unchanged by construction (original branch), covered by CI
• Behavior verified on a real radio: not applicable, test-only change
• Existing tests pass (CI): pending

I only ran this one test; the rest of the suite is untouched by a one-file test edit.

Checklist

• Commits are signed (docs/COMMIT-SIGNING.md), SSH signing with the w5jwp signing key
• No new flat-key AppSettings calls (Principle V): none
• Code is clean-room (Principle IV): test-only
• All meter UI uses MeterSmoother: no UI touched
• Documentation updated if user-visible behavior changed: nothing user-visible. CHANGELOG.md is not touched
• Security-sensitive changes reference a GHSA if applicable: not applicable

🤖 Generated with Claude Code

[aethersdr-agent]

Issue fit

Yes. #6358 asks for exactly three things — gate the Alt+F assertion on the capability rather than on Q_OS_MACOS, keep the submenu-rounding check alive on the gated path, and add the explicit #include <QKeySequence> — and the diff does all three and nothing else. I checked the issue's load-bearing claims against the head checkout rather than taking them on trust, and each one holds:

• TitleBar::addAppMenuMnemonic (src/gui/TitleBar.cpp:1149-1155) really does bail on an empty mnemonic: const QKeySequence key = QKeySequence::mnemonic(menuAction->text()); if (key.isEmpty()) return; — so the new gate is the same predicate the production code uses, not a proxy for it.
• src/gui/TitleBar.cpp:1096-1097 — if (mb->isNativeMenuBar()) return; // macOS / a global-menu desktop shows the menus itself — returns before the hamburger or any mnemonic is built, and grep -rn setNativeMenuBar src/ has zero hits. The gated behaviour is genuinely unreachable in the shipping macOS app, so "test-only, no user impact" is correct.
• AetherSDR::roundMenuTree (src/gui/RoundedMenu.cpp:37-52) sets the two flags, then connects a prepare lambda that recurses through action->menu() to QMenu::aboutToShow. QMenu::popup() emits aboutToShow synchronously on the same thread, so the else branch does leave rounded(lateSubmenu) with real teeth instead of quietly dropping it. (Reasoned from code — I ran nothing.)

Scope

File	What it changes	Claimed by the body?	Verdict
tests/unified_title_bar_test.cpp	#include <QKeySequence>; wraps the Alt+F keyClick/check pair in a QKeySequence::mnemonic gate with a popup() + stderr-note fallback	Yes	In scope

git diff --stat b3e0302ea..HEAD is one file, 10 insertions / 3 deletions. Nothing else in the diff, no production code, no build config, no CHANGELOG.md entry (correct — it is a release-prep file and must not be touched here). The body's self-certifications check out. Everything in the diff is explained by #6358.

No socket test added or modified. The target links Qt6::Network, but the file opens nothing: no QTcpSocket/QUdpSocket/QLocalServer/connectToHost/bind/listen, and no Fake* peer. The changed hunk adds only a QKeySequence probe and a QMenu::popup().

Blockers

None.

Nits

All non-blocking.

• tests/unified_title_bar_test.cpp:1055 — the gate hardcodes a second copy of the menu title:

          if (!QKeySequence::mnemonic(QStringLiteral("&File")).isEmpty()) {

  Production derives it from the action (TitleBar.cpp:1153: QKeySequence::mnemonic(menuAction->text())). Today the literal matches :967's menuBar->addMenu(QStringLiteral("&File")), but if that title ever loses or moves its & the gate and the menu drift apart silently. Deriving it removes the coupling:

          if (!QKeySequence::mnemonic(fileMenu->menuAction()->text()).isEmpty()) {

• tests/unified_title_bar_test.cpp:1059-1060 — the fallback opens the menu but never asserts it opened, so on macOS "the File menu can be shown at all" is no longer checked anywhere; only rounding is. One line restores it without touching the Linux/Windows path:

              fileMenu->popup(menuBtn ? menuBtn->mapToGlobal(QPoint(0, menuBtn->height())) : QPoint());
              check(QApplication::activePopupWidget() == fileMenu,
                    "the File menu opens where mnemonics are unavailable");

• tests/unified_title_bar_test.cpp:1008-1010 — the minimal-mode "mnemonics inert" check passes vacuously on macOS for the same root cause, and unlike the new gate it says nothing about it. You disclose this in the body and I agree with keeping the assertion; the asymmetry (one skip announced, one silent) is the only reason to mention it.
• Context for the maintainer, not a defect in this PR: #6348 and #6353 both cite these two failures as "the #6224 checks". #6224 is the unified-title-bar native-platform tracker (Snap Layouts, #4557 drift, macOS M1 pointer input, xcb move) and never mentions unified_title_bar_test; it explicitly puts "anything already covered by unified_title_bar_test" out of scope. #6358 is the correct home for this symptom, so those PRs' "pre-existing failure" references resolve here once this lands.

What I tried to break

• Is the gate hiding a real macOS defect? This was my main attack — a capability probe that skips an assertion looks exactly like a test silenced to go green. It isn't: the probe is the production gate verbatim (TitleBar.cpp:1153-1155), setMenuBar returns at :1096-1097 when the bar is native, and nothing in src/ forces setNativeMenuBar(false). There is no user-facing Alt+F on macOS to be broken.
• Does the else branch leave the next assertion vacuous? No — roundMenuTree's prepare is wired to aboutToShow with a direct connection and recurses into submenus, and popup() emits that signal synchronously, so rounded(lateSubmenu) is evaluated after prepare() has run. Had the fallback been show() or a deferred popup, the check would have become a tautology.
• Does it perturb the code after the hunk? Both branches end with exactly one open popup, closed by the existing fileMenu->close(); app.processEvents();, so the "offscreen does not hand activation back after a popup closes" invariant documented at :1065-1068 is unchanged, and the hamburger mouseClick that follows still runs from the same state. menuBtn is already asserted non-null and visible at :1012-1013, so the menuBtn ? ternary cannot be the live path.
• Include hygiene by the PR's own standard. <QKeySequence> is correctly added. I also checked the new QPoint(0, menuBtn->height()) / QPoint() calls: <QPoint> is already present at :51 and QPoint has been used since :174, so the hunk introduces no second transitive include. Nothing to fix.
• Linux/Windows regression. The first branch is the pre-change code byte for byte, and mnemonic("&File") is non-empty there, so existing coverage is untouched. If some Qt build ever re-enabled mnemonics on macOS via qt_set_sequence_auto_mnemonic(true), the gate would take the assertion branch — which is precisely why the capability probe beats a Q_OS_MACOS #ifdef.
• Could not verify, and it matters: that QKeySequence::mnemonic("&File") is actually empty under QT_QPA_PLATFORM=offscreen on macOS. Qt's switch lives in qkeysequence.cpp, not a header, and I have no build or test run here — every finding above is reasoned from source. The only evidence the else branch is ever taken is your reported mnemonics are off on this platform: Alt+F check skipped line; a reviewer on a Mac confirming that line appears would close the loop.
• CI, for the record. All five checks on dbed8a0 are green (build, check-macos, check-windows, Static checks, Sanitizer option configures), but none of them runs this test: ci.yml's frozen gate limits macOS to mac_nr_filter_test and asr_gpu_probe_test, and full-suite.yml runs the unfiltered suite on ubuntu-latest on push to main, i.e. after merge. So green here is not evidence the macOS red is fixed, and the body's "CI's Ubuntu full suite keeps running it" is true only post-merge. Worth stating plainly, since the whole point of the PR is a result nobody's pre-merge lane sees.

Security audit (CodeGuard): all eight leads refuted — every one is pre-existing, outside the changed hunk, and a lexical false positive on test code. CS-VLN-CMDI-AMP at :97, :686, :704 matches the C++ reference declarator in parameter lists and lambda captures (const QString& id, [&requestedId, ...]); the file invokes no shell. CS-VLN-CMDI-BACKTICK at :13, :28, :113 matches Markdown-style backticks inside // comments. CS-VLN-SSRF-PRIV-192 and CS-PII-IP at :437 are the same line — connected.transport = QStringLiteral("192.168.1.21"), an RFC 1918 fixture string assigned to a field the radio tab renders as a display label (the sibling at :444 sets it to "SmartLink"); the file builds no request and opens no socket. No credential, no unsafe C API, no format string under test control.

Recommendation

Approve with nits. The diff is minimal, the gate is the production predicate rather than a platform guess, the follow-on rounding check keeps its teeth on the skipped path, and the skip announces itself instead of passing silently — which is the right call for a red test that was teaching Mac developers to ignore red. I could not break it, and the one thing I'd most like confirmed (that the else branch is actually taken) is a macOS-only observation no lane in this repo makes. Next step: take the two one-line nits at :1055 and :1059 if you agree, or land as-is and let the post-merge Ubuntu full-suite.yml run confirm Linux is unchanged. Thanks for the unusually well-evidenced issue — the root-cause write-up on #6358 made this reviewable from source alone.

---

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

[aethersdr-agent]

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

• [HIGH] CS-VLN-SSRF-PRIV-192 — Vulnerability: CS-VLN-SSRF-PRIV-192 in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:437
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:97
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:686
• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:704
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:13
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:28
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:113
• [LOW] CS-PII-IP — PII detected: ip_address in tests/unified_title_bar_test.cpp /tmp/aetherclaude/pr-6359/tests/unified_title_bar_test.cpp:437

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

[w5jwp]

Thanks for the review. Pushed 9d2188c.

Taken

• The mnemonic gate now derives its text from fileMenu->menuAction()->text(), as production does, instead of hardcoding &File.
• The fallback popup() now asserts activePopupWidget() == fileMenu. It holds on macOS offscreen; the test passes with the skip line printed.

Not taken

• The minimal-mode inert check still passes vacuously on macOS. Agreed to keep it, as discussed in the review.

🤖 Generated with Claude Code

[ten9876]

Issue fit

Yes. #6358 requires an explicit QKeySequence include, a capability-based Alt+F gate, and retained submenu-rounding coverage when mnemonics are unavailable. The current head implements all three, derives the capability from the actual menu action, and additionally asserts that the fallback popup opened. The earlier review's two actionable nits are addressed.

Scope

File/group	Change	Claimed?	Verdict
tests/unified_title_bar_test.cpp:44,1055–1068	Explicit include; mnemonic capability gate; fallback popup and assertion	Yes	In scope, test only

Everything in the diff is explained by #6358. Both commit author dates and messages fit the issue; no production code, default, UX preference, protocol surface, test registration or CHANGELOG change. The body's checklist holds. The illustrative body diff predates the follow-up commit; the current source contains the stronger action-derived gate and popup assertion described above.

Blockers

None.

Nits (non-blocking)

None requiring a source change. The minimal-mode assertion still passes vacuously when mnemonics are disabled, as already disclosed; its coverage remains useful on platforms with mnemonics.

What I tried to break

• Built only unified_title_bar_test in an isolated head worktree using GCC 16.2.1 and pinned Qt 6.12.0. Its registered offscreen CTest passes on Linux.
• Exercised the missing-mnemonic branch by declaring Qt's exported qt_set_sequence_auto_mnemonic(bool) switch in the scratch test and calling it with false immediately after QApplication construction. The merge-base test fails with exactly FAIL: Alt+F still opens the File menu with the menu bar hidden and FAIL: opening a menu rounds the submenus added to it since. The head test, under the identical switch, passes and prints mnemonics are off on this platform: Alt+F check skipped and unified_title_bar_test: all checks passed.
• Mutated the head test by removing its fallback fileMenu->popup(...) while leaving mnemonics disabled. CTest returns failure with FAIL: the File menu opens where mnemonics are unavailable and the submenu-rounding failure. Both retained assertions therefore detect the missing operation. Restored the exact head source and confirmed the registered test passes again.
• Read the production mnemonic predicate, native-menu early return, popup anchor, and roundMenuTree aboutToShow recursion. The gate matches production; closing the popup preserves the later hamburger click coverage. git merge-tree against current main is clean. The file has no socket ownership or synthetic peer; linking Qt Network is not a socket test.

The Linux capability-toggle experiment verifies the failing condition and fallback, not a native macOS runtime. The author's native macOS result was not independently rerun. No application bridge session was launched because only a socket-free GUI test changes; the actual widgets were driven by that test. No operator instance or radio was touched. No separate automated review skill was available; existing bot findings were checked against the latest source rather than adopted.

CI's five reported checks succeeded, but neither the macOS nor Windows PR gate runs this test. Earliest head run creation was 2026-10-11T15:05:59Z, preceding current main 0625d24 at 2026-10-11T15:12:10Z. The pull_request workflow tested an older merge snapshot; non-strict protection does not force refresh. The local test evidence above is separate from those green build checks.

Recommendation

Approve. The original condition fails on the base and succeeds on the head, and a broken fallback fails the retained assertions. Obtain the normal CODEOWNERS decision and refreshed integration checks; no remaining implementation blocker found.

[ten9876]

Landing approval at verified head 9d2188c.

9d2188c: tests/unified_title_bar_test.cpp derives the mnemonic gate from the menu action and asserts the fallback popup. The prior two actionable nits are fixed. The existing minimal-mode non-event assertion remains valid with mnemonics disabled; the capability-toggle run confirms no popup, while the same test exercises the mnemonic path normally on Linux.

The complete scope, governance, empirical evidence and unverified boundaries are recorded in my preceding review. I re-inventoried reviews, comments, every thread, current signatures and required checks: no unresolved thread, active CHANGES_REQUESTED review, or remaining actionable finding. No additional code push was needed. The manual adversarial pass substitutes for the unavailable automated skill; no operator app or radio was touched.

All four required checks pass on this head and it merges cleanly. Main at arming is 7e080e0, newer than the PR run creation; strict:false deliberately permits this and the post-merge CI is the independent verification. No update solely for CI freshness was made.

Independent test evidence: registered unified_title_bar_test passes; with Qt mnemonics disabled, base fails the exact two issue assertions and head passes; omitting the fallback makes both retained assertions fail. Native macOS was not independently driven.