Skip to content

feat(gui): MQTT Settings is a CanonWindow dialog, re-skinned to the canon palette. Principle VIII. - #6751

Open
w5jwp wants to merge 4 commits into
aethersdr:mainfrom
w5jwp:feat/mqtt-settings-canonwindow
Open

w5jwp wants to merge 4 commits into
aethersdr:mainfrom
w5jwp:feat/mqtt-settings-canonwindow

Conversation

@w5jwp

@w5jwp w5jwp commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #6750. Part of #6239. Waiting on the maintainer's pick: this full CanonWindow move is not on #6239's CanonWindow lists; #6239's "Persistent tool dialogs (one PR per dialog)" candidate covers only moving a dialog's colours to color.canon.*. #6750 has the scope question. Please don't merge before the pick. Merging is that pick: aethersdr-style-guide.md and dialog-patterns.md list MQTT Settings among the windows the maintainer has moved to CanonWindow, which is true only once this lands.

The MQTT Settings window (MqttSettingsDialog) moves from PersistentDialog to a CanonWindow dialog (Kind::Dialog), as the AetherRX and AetherTX Settings dialogs did:

  • Chrome: no title bar, the ambient ground, the canon header and the corner close button. It closes with Esc or Ctrl+W (⌘W) and opens centered on the main window. It still resizes from its edges (FramelessResizer). It no longer follows View → Frameless Window, so showMqttSettingsDialog() creates it directly instead of through showOrRaisePersistent(). That helper's trackPersistentDialog() only fed the Frameless Window toggle, so nothing else is lost.
  • Title and entry points: unchanged: "MQTT Settings", from Settings → MQTT... or the MQTT applet's Settings....
  • Size: minimum 620×560, 40 px taller for the header.
  • Look: under the canon/mqttSettings theme scope:
    • the ModemChrome canon sheet the AetherRX/TX Settings dialogs use;
    • SpotHub's canon rules for the tabs (cyan underline on the selected tab) and the Internal AetherSDR Topics group boxes;
    • fields at 29 px and buttons at 31 px instead of that sheet's 39–42 px and 38 px, since this is a six-row form rather than a list and a button row.
  • Port field: canon stepper arrows, and a text inset that matches the text fields. Qt drops a styled spin box's horizontal padding before it reaches the editor, so the inset is set with setTextMargins(). The padding and the inset come from the same named constants (padding plus the 1 px border), so they cannot drift apart.
  • Table checks: the Display column's check uses the canon check images. With the canon sheet alone it was an almost invisible dark square.
  • Field menus: the canon sheet clears every descendant to transparent, and a field's standard right-click menu (Undo/Cut/Copy/Paste) is a child of the field, so the sheet gives QMenu its canon panel back, as on TX Band Settings (0190287f).
  • Removed: the hard-coded #556 colour on the always-on built-in topics; the canon sheet's disabled rule colours them.
  • Retired key: MqttSettingsDialogGeometry joins the keys main.cpp removes at startup, as AboutDialogGeometry did when About moved to CanonWindow. Nothing reads it any more.

Shared code (src/gui/CanonIndicators.h):

  • spinArrowRules() moves from RadioSetupDialog.cpp as canonSpinArrowRules(). The function body is byte-identical (compared programmatically), and Radio Setup now calls the shared copy.
  • New canonItemCheckRules(view): the canon check images for an item view's check column. It and canonIndicatorRules() now share detail::canonIndicatorImages(). canonIndicatorRules()' output is unchanged: same template, same eight images.

There are no message boxes on the MQTT path, so CanonMessageBox is not involved.

Docs:

  • The MQTT user page says what OK, Apply, Cancel, the corner ✕, Esc and Ctrl+W do.
  • docs/style/* list MQTT Settings among the canon windows and canon/mqttSettings among the scopes.
  • screens.json: the mqtt-settings-dialog alt text names the canon header and the corner close button.
  • Stale screenshot, to recapture after merge: mqtt-settings-dialog.png shows the old window. It should be re-shot with tools/docs/capture_screenshots.py --only mqtt-settings-dialog (its spec still finds the window by class name). Native macOS captures were used during review; they are not replacements for the tool's shot.

Intent

MQTT Settings looks and closes like the other canon dialogs. Out of scope: what it saves, the keychain handling, the MQTT applet, and Radio Setup's own spin box text, which also sits on the field's left edge (the same Qt behaviour).

Design review (maintainer): the field and button sizes and the label alignment are visual calls for the maintainer.

Constitution principle honored

Principle VIII (Evidence Over Assertion) covers the checks below. The visual direction is RFC #6226's.

Test plan

  • Local build passes (macOS arm64, RelWithDebInfo, MQTT enabled).
  • canon_window_test passes.
  • New mqtt_settings_dialog_test (socket-free, the dialog is never accepted): 8 checks, all passing.
    • It is a CanonWindow. A field's standard right-click menu is parented inside the canon body and draws an opaque background; renaming the QMenu selectors so they match nothing fails this check.
    • Esc, Ctrl+W / ⌘W and the corner button each destroy a WA_DeleteOnClose window (Qt 6.12's QDialog::done() closes; it does not only hide), so a half-edited field never comes back on the next open.
    • CI does not run it before merge: it is not in ci.yml's frozen per-PR ctest -R steps, so green means it compiled; it runs in full-suite.yml.
  • gen_touchpoint_manifest.py --check, gen_reference.py --check, git diff --check pass.
  • Colour-audit ratchet (tools/audit_colours.py --compare-src <base> --strict, at 0190287f): unique colours 486 (−1), references 1877 (−2), setStyleSheet 597 (−2). tools/check_a11y.py src/gui/MqttSettingsDialog.cpp: no findings.
  • Docs site: npm ci && npm run build succeeds (Node 24, as CI), including its link and anchor check.
  • Automation bridge, offscreen, isolated settings (AETHER_SETTINGS_DIR), AETHER_AUTOMATION_NO_TX=1, connected to the demo radio (DEMO-0001, "AetherSDR Demo"), opened via MQTT...:
    • opens at 620×560; Host, User and Password 29 px, Port 32 px, buttons 31 px; the Port text lines up with Host's;
    • Use TLS moves the port from 1883 to 8883;
    • Apply saves: after Cancel the dialog is deleted, and reopening shows the applied host and port;
    • closing discards edits: setText mqttHost discard.me, then the bridge's close; the dialog was deleted, and reopening showed localhost;
    • with seeded settings (two subscriptions, one with Display ticked; one publish button): ticked, unticked and disabled checks all draw the canon images; cell text renders unclipped; a double-clicked cell's editor fits its row;
    • light theme (ActiveTheme = "Default Light"): every control readable, checked and disabled states distinct;
    • retired key: MqttSettingsDialogGeometry present in the scratch store before launch (count 1), gone after (count 0).
  • Radio Setup, Appearance & Behavior: the Single-Click Delay spin box still draws its arrows, and the radio buttons and check boxes are unchanged after the CanonIndicators.h refactor.
  • Not exercised: Esc and Ctrl+W through the bridge (its keyevent reaches only shortcuts; mqtt_settings_dialog_test drives both on the real dialog); typing into a table cell; a live broker; Windows and Linux builds. The app sets the Fusion style on every platform (main.cpp), so the spin-box inset behaviour measured on macOS is the same style.

Checklist

  • Commits are signed
  • No new flat-key AppSettings calls. As a centered dialog it no longer saves its geometry, and the old key is removed at startup.
  • Code is clean-room
  • All meter UI uses MeterSmoother (n/a)
  • Documentation updated: docs/user/docs/mqtt-station-automation.md, docs/style/*
  • Security-sensitive changes: n/a (keychain path unchanged)

🤖 Generated with Claude Code

@w5jwp
w5jwp requested a review from a team as a code owner October 24, 2026 20:41
@w5jwp

w5jwp commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author
Image Image Image

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at the PR head (5a2f76f) against the head checkout. The implementation is careful and the mechanical claims in the body hold up to checking — I verified the byte-identity claim, the ratchet, the retired key and the show-path precedent rather than taking them on trust. The one thing that is not settled is whether this window was picked for CanonWindow at all, and merging is that pick.

Everything below is reasoned from code: I am headless, with no build, no test run and no way to drive the GUI, so nothing here was reproduced at runtime.

1. Issue fit

Yes — #6750 is implemented point for point. CanonWindow with header and corner close (MqttSettingsDialog.cpp:135-137), canon/mqttSettings scope, the ModemChrome canon sheet plus the tab/group-box rules that sheet lacks, FramelessResizer::install(this) for the "still resizable" requirement, OK/Apply/Cancel save behaviour untouched (:310-320), the #556 literal gone from both sites, the retired key at src/main.cpp:613, and both shared helpers in CanonIndicators.h. Nothing in #6750 is left unaddressed.

The issue itself is one hour old, opened by the PR author, still labelled maintainer-review with no maintainer ruling on it — which is the subject of §3.

2. Scope

File / group What it changes Claimed? Verdict
src/gui/MqttSettingsDialog.{h,cpp} PersistentDialog → CanonWindow, canon sheet, port-field inset, #556 removed Yes In scope (#6750)
src/gui/MainWindow.cpp showMqttSettingsDialog() becomes the canon create-or-raise Yes In scope — required; showOrRaisePersistent would not compile against a CanonWindow
src/gui/CanonIndicators.h canonSpinArrowRules() moved in, new canonItemCheckRules(), shared detail::canonIndicatorImages() Yes In scope — #6750 names the extraction explicitly
src/gui/RadioSetupDialog.cpp drops the private spinArrowRules() + now-unused ThemedImageCache.h include, calls the shared copy Yes In scope — consumer side of the move. themedImagePath has no other caller in the file, and CanonIndicators.h carries the include
src/main.cpp MqttSettingsDialogGeometry joins the retired-key list Yes In scope
docs/user/docs/mqtt-station-automation.md documents OK/Apply/Cancel/✕/Esc/Ctrl+W Yes In scope
docs/style/* (3 roster sentences) lists MQTT Settings among windows "the maintainer has moved to" CanonWindow Yes Needs maintainer decision — see §3

Everything in the diff is explained by #6750. No unrelated files, no formatting churn, no new public/protocol surface, no settings key added, no guard or confirmation removed. The only - lines of substance are the two #556 stylesheets and the moved function, both accounted for.

FYI, not a finding: the three docs/style roster sentences are the same lines #6745, #6749 and #6735 each edit, so whichever lands first will conflict the others.

3. Needs maintainer decision (not a blocker, but it gates merge)

docs/style/aethersdr-style-guide.md:118, dialog-patterns.md:238, theme-style-guide.md:46 — these edits add MQTT Settings to the list of "the tool windows the maintainer has moved to it" and to canon/mqttSettings among the scopes. That sentence is only true once this merges, so the docs assert a pick that hasn't been made.

MQTT Settings appears nowhere on #6239 — not under Picked, and not in the CanonWindow candidate list (What's New, System Info, Help pages, SWR sweep licence, frameless message boxes). The line #6750 and this PR cite, "Persistent tool dialogs (one PR per dialog)", sits under the bullet for surfaces whose widgets move from base tokens to color.canon.* — a re-skin that keeps PersistentDialog chrome, which is a different change from moving the window's chrome to CanonWindow. #6239 says "Items under 'Candidates' are not approved scope until the maintainer picks them" (quoted in the #6750 triage comment, which reached the same conclusion independently).

So this is a self-nomination of the Network-Diagnostics / Radio-Setup class, not a reading of an existing bullet. That is a legitimate thing to ask for — #6735 asks for the same thing and says so in its body — but the body here presents it as settled ("Part of #6239", "as the AetherRX and AetherTX Settings dialogs did") without disclosing that the window is unlisted. Worth a sentence in the body either way.

Fold the body's own flagged visual calls into the same ruling: the 29 px fields / 31 px buttons, and the AlignRight | AlignVCenter label alignment.

4. Blockers

None.

5. Nits (all non-blocking)

  • src/gui/MqttSettingsDialog.cpp:41-100 — the composed sheet has no QMenu rule, so a field's right-click menu likely draws with no background. ModemChrome::styleSheet(…, Look::Canon) opens with a blanket rule whose canon background is transparent (ModemChrome.cpp:104-108, %2 = c.ground = "transparent" for kCanon at :52):

    QWidget {
        color: %1;
        background: %2;
        font-size: %3px;
    }
    

    A Qt type selector matches subclasses, and QLineEdit::createStandardContextMenu() parents its QMenu to the field — which is inside bodyWidget(), so the sheet reaches it. Host, User, Password and the CA-file edit, the Port spin's internal editor, and the tables' cell editors all have one. Neither ModemChrome's canon sheet, kMqttCanonRules, canonSpinArrowRules() nor canonItemCheckRules() contains a QMenu selector.

    Two independent in-repo data points: RadioSetupDialog.cpp:1069-1082 carries explicit QMenu rules in its canon sheet, whose QWidget rule is also background: transparent (:852-854); and #6735's body reports observing exactly this on TxBandDialog ("the menu on the RF and Tune power fields drew no background at all") and fixing it in a follow-up commit. On main this dialog had no body-wide sheet at all, so its context menus drew normally — this is a regression for this dialog, though the gap is pre-existing in every ModemChrome-canon consumer (AetherRxSettingsDialog, AetherTxSettingsDialog, FirmwareInstallDialog), which have far less text entry. Reasoned from code plus #6735's observation; I could not reproduce it.

    Drop-in — append to kMqttCanonRules (the block is RadioSetupDialog.cpp:1069-1082 verbatim):

    QMenu {
        color: {{color.canon.inkSoft}};
        background: {{color.canon.nested}};
        border: 1px solid {{color.canon.lineHi}};
        selection-background-color: {{color.canon.cyan}};
        selection-color: {{color.canon.onAccent}};
    }
    QMenu::item:selected {
        color: {{color.canon.onAccent}};
        background: {{color.canon.cyan}};
    }
    QMenu::item:disabled {
        color: {{color.control.unavailable}};
    }
    

    The better home is ModemChrome's if (look == Look::Canon) branch (ModemChrome.cpp:405-419), beside the QListWidget append, so the sibling dialogs get it too — but that widens this PR's blast radius, so a follow-up is fine.

  • No test references MqttSettingsDialog. canon_window_test covers the base class, not this dialog's wiring. The deterministic socket-free seam exists and the sibling PRs in this batch use it: #6745 adds rc28_mapping_dialog_test (is-a-CanonWindow, FramelessResizer installed, geometry key behaviour), #6735 extends tune_output_dialog_test. A few cheap checks here would be: the composed sheet is non-empty and contains a QTableWidget::indicator rule; canonSpinArrowRules() output is stable; the retired key is gone after startup. Not merge-gating — neither would run in the frozen per-PR ctest -R gate — so this is a consistency nit, not a withhold.

  • src/gui/MqttSettingsDialog.cpp:143-148 — the composed sheet calls detail::canonIndicatorImages() twice per rebuild (once inside ModemChrome's canon branch via canonIndicatorRules(), once via canonItemCheckRules()). Harmless — the images are cache-path lookups — and only on theme change. Mentioning it only so it isn't mistaken for a bug later.

  • The body's test plan omits tools/audit_colours.py, which docs/style/aethersdr-style-guide.md:202-204 makes a requirement of every migration ("Every migration keeps the colour-audit ratchet flat or lower"). I ran it: it passes comfortably (figures below), so this is a body-completeness nit, not a gate failure.

6. What I tried to break (and could not)

  • "The function body is byte-identical (compared programmatically)." Held. I extracted spinArrowRules from 1afefafdb:src/gui/RadioSetupDialog.cpp and canonSpinArrowRules from the head CanonIndicators.h, stripped the signature line: 42 lines each, diff clean. The canonIndicatorRules() template and its .arg() order are also unchanged, the aggregate-init order matches the struct field order, and the fail-closed path is preserved (return {} became nullopt → return {} in both callers), so a failed image write still yields Qt's own indicators rather than blanks.
  • Dangling pointer on reopen. WA_DeleteOnClose plus a cached pointer is the classic use-after-free here; MainWindow.h:1760 is a QPointer<MqttSettingsDialog>, so if (!m_mqttSettingsDialog) sees null after deletion and rebuilds, reconnecting settingsSaved. The hoisting of that connect into the fresh-construction branch is equivalent to the old wasFresh dance, and the new show path is character-for-character the AGC Calibration precedent at MainWindow.cpp:4663-4673.
  • "trackPersistentDialog() only fed the Frameless Window toggle, so nothing else is lost." Held. m_persistentDialogs has exactly one consumer: the propagation loop at MainWindow.cpp:10264-10272, inside setFramelessMode. Nothing else reads the list.
  • The retired key. No reader survives: MqttSettingsDialogGeometry appears only at src/main.cpp:613, and dialog/mqttSettings appears nowhere. docs/user/screens.json keys wait_visible/target on the class name MqttSettingsDialog, which is unchanged, so the capture spec still finds the window as the body claims.
  • "The canon sheet's disabled rule colours them" (the #556 removal). Held, and it is an improvement, not a regression. ModemChrome.cpp:161-164 has QRadioButton:disabled, QCheckBox:disabled { color: %11 }, where %11 is c.textDim = color.canon.muted (#8598b4). Against the group box's color.canon.nested (#0e1a2a) that is 5.96:1, versus the old #556 on #0f0f1a at 2.61:1 — the first clears docs/a11y.md's 4.5:1 aim, the second never did.
  • Mixed check-box appearance. I expected the table's new PNG checks to clash with ModemChrome's QSS-drawn ones (solid cyan fill, no tick). They don't: ModemChrome.cpp:405-410 strips its own ::indicator rules under Look::Canon and appends canonIndicatorRules(), so the dialog's QCheckBoxes and the Display column now draw from the same 18 px images.
  • No new theme tokens needed? Confirmed — resources/themes/default-dark.json carries a canon scope with tokens and no child scopes, so canon/mqttSettings re-palettes by prefix inheritance, exactly as canon/agcCalibration does. No theme file needed touching.
  • The colour ratchet. Ran tools/audit_colours.py --compare-src <main>/src --strict from the head checkout: unique colours 486 (base 487), total references 1877 (base 1879), setStyleSheet sites 597 (base 599) — OK on all three. Also ran tools/check_a11y.py src/gui/MqttSettingsDialog.cpp (0 findings) and tools/gen_touchpoint_manifest.py --check (up to date).
  • QSS escape / injection in the new helpers. canonItemCheckRules' only call site passes the literal QStringLiteral("QTableWidget"), and the multi-arg QString::arg(view, …) substitutes in one pass, so a view containing %2 could not re-enter later placeholders. kMqttCanonRules' %1/%2 are the two padding ints and the string holds no other %.
  • The CA-certificate file dialog. QFileDialog::getOpenFileName(this, …) at :220 parents to the window, not bodyWidget(), so the blanket-transparent sheet does not reach it — this is why the §5 finding is limited to the body's own context menus.
  • CodeGuard's 50 findings: all 50 refuted. The security-audit (CodeGuard) subagent confirmed every one as lexical — backticked identifiers in C++ doc comments (CanonIndicators.h:165, RadioSetupDialog.cpp:416/1428/1582), const QString& declarators and Qt &Class::signal syntax read as shell &/|, the literal /proc/self/status read, 224.0.0.1 as "PII", the Flex status code 50000053 as a bank account, "override" in a DSP comment as a prompt injection. Every MainWindow hit is also verbatim on main, so untouched by this PR. It separately checked the keychain path: the password reaches only QKeychain::WritePasswordJob::setTextData(), logs carry errorString() only (no secret, unlike the QrzClient case), the persisted MqttConnectionConfig holds no password, and setEchoMode(Password) is intact. No genuine security defect.
  • CI, honestly: build, check-macos, check-windows, Static checks, Build docs site and Sanitizer option configures are all green on 5a2f76f, and ENABLE_MQTT defaults ON (CMakeLists.txt:69) so this dialog really is compiled in those lanes. That proves compilation, not behaviour — no MQTT or canon dialog test is in the frozen per-PR ctest -R gate.
  • Could not check: anything at runtime. In particular the measured geometry (620×560, 29/31/32 px rows, the Port text lining up with Host's), the light-theme pass, and whether Esc / ⌘W actually delete the instance. On the last one, QDialog::done() honours WA_DeleteOnClose in Qt 6 and the body reports the bridge's close leaving the dialog deleted with edits discarded, so I have no reason to doubt it — but a surviving instance re-showing half-edited fields is the failure mode if that ever regresses, which is one more argument for the test nit above.
  • Also not checkable from here: the #556 → canon change on the light theme, and the mono-font claim for the Port field — though that one is a consistency fix rather than a restyle: ModemChrome.cpp:266-274 already sets the same "SF Mono", "Menlo", "Consolas", monospace stack on QLineEdit, so the spin box was the odd one out.

7. Recommendation

Needs maintainer decision. The engineering is clean: the refactor is provably behaviour-preserving, the show path copies an existing canon precedent exactly, the retired key has no reader left, the ratchet moved down, and I could not find a correctness, lifetime or threading defect. But MQTT Settings is on neither of #6239's approved lists, and the three docs/style roster edits state the pick as already made — so merging is the pick, and that is the maintainer's call, together with the field/button sizing and label alignment the body already flags.

Concrete next step: get the #6750 scope ruling (and the sizing call) from the maintainer; separately, add the QMenu block above so right-clicking a field doesn't show a transparent menu. The coverage nit is worth doing for parity with #6745 but shouldn't hold the merge.

Thanks for the unusually checkable body — the byte-identity claim, the retired-key count and the capture-spec note each pointed me straight at the thing to verify, which is the opposite of the usual experience.


🤖 aethersdr-agent · cost: $20.8488 · model: claude-opus-5

@aethersdr-agent

Copy link
Copy Markdown
Contributor

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

  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/CanonIndicators.h /tmp/aetherclaude/pr-6751/src/gui/CanonIndicators.h:165
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:9727
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:9839
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:9961
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:9962
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:9963
  • [HIGH] CS-VLN-PATH-PROC — Vulnerability: CS-VLN-PATH-PROC in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:6227
  • [HIGH] CS-VLN-PATH-PROC — Vulnerability: CS-VLN-PATH-PROC in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:6306
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:521
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:3583
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:519
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:1219
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:1702
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:1790
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:1867
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:1909
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:1944
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:2240
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:3516
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:3616
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:6169
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:8046
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:8120
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:8205
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:8388
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:8591
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:9828
  • [LOW] CS-PII-IP — PII detected: ip_address in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:6934
  • [HIGH] CS-PII-BANK — PII detected: bank_account in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:5541
  • [CRITICAL] CS-INJ-role_override — Prompt injection: role override attempt in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:11683
  • [CRITICAL] CS-INJ-role_override — Prompt injection: role override attempt (detected after NFKC normalization) in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:11683
  • [HIGH] CS-INJ-homoglyph — Mixed Unicode scripts detected (possible homoglyph attack) in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6751/src/gui/MainWindow.cpp:0
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:175
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:173
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:5708
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:6343
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:6493
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:416
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:1428
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/RadioSetupDialog.cpp /tmp/aetherclaude/pr-6751/src/gui/RadioSetupDialog.cpp:1582

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: $21.9970 · model: claude-opus-5

@w5jwp

w5jwp commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. It says it read 5a2f76f, the commit before the current head 415bbf40, which already covers the two substantive nits:

  • QMenu rule: fixed in 415bbf40. kMqttCanonRules carries the canon QMenu block (raised panel, lineHi border, cyan selection, control.unavailable disabled, line separators), matching TX Band Settings.
  • No dialog test: 415bbf40 adds mqtt_settings_dialog_test (8 checks, socket-free, never accepted).
    • It covers CanonWindow, and that a field's standard menu is parented inside the canon body and draws opaque. Renaming the QMenu selectors so they match nothing fails that check.
    • It also covers Esc, Ctrl+W / ⌘W and the corner button each destroying the WA_DeleteOnClose window. That settles the runtime doubt in your §6: Qt 6.12's QDialog::done() closes rather than only hiding, so a half-edited field can't survive into the next open.
    • The test isn't in the frozen per-PR gate; the body says so.

Colour audit in the test plan: added. At 415bbf40 the figures match yours: unique 486 (−1), references 1877 (−2), setStyleSheet 597 (−2).

canonIndicatorImages() built twice per rebuild: agreed it's harmless (cache-path lookups, on theme change only), so I've left it.

The same commit also names the canon header and the corner close button in the mqtt-settings-dialog alt text in screens.json, from the #6750 triage.

w5jwp and others added 3 commits October 24, 2026 22:07
…anon palette. Principle VIII.

MqttSettingsDialog moves from PersistentDialog to a CanonWindow dialog,
as the AetherRX / AetherTX Settings dialogs did: the canon header and
corner close button, the canon/mqttSettings theme scope, and the
ModemChrome canon sheet plus SpotHub's tab and group-box rules. Fields
and buttons are sized for a multi-row form. It opens centered, closes
with Esc or Ctrl+W, still resizes, and no longer follows View →
Frameless Window, so showMqttSettingsDialog() creates it directly.

The Port spin box gets canon steppers through canonSpinArrowRules(),
moved unchanged from RadioSetupDialog.cpp into CanonIndicators.h, and a
text inset the sheet's padding cannot give it. The table's Display check
uses the canon check images. The hard-coded aethersdr#556 disabled colour is gone.

Part of aethersdr#6239.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tings' retired geometry key goes at startup. Principle VIII.

Review follow-ups to the MQTT Settings move to CanonWindow:

- canonItemCheckRules(view) in CanonIndicators.h gives an item view's
  check column the canon check images. canonIndicatorRules() and it now
  share detail::canonIndicatorImages(); canonIndicatorRules()' output is
  unchanged. MQTT Settings uses it in place of regex-retargeting
  canonIndicatorRules()' text.
- MQTT Settings' field padding and the port field's text inset come from
  named constants (padding + the 1 px border), so they cannot drift apart.
- MqttSettingsDialogGeometry joins the retired keys removed at startup,
  as AboutDialogGeometry did: a CanonWindow dialog opens centred and
  nothing reads it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s it and the close paths. Principle VIII.

The canon controls sheet clears every descendant to transparent, and Qt
parents a field's standard right-click menu (Undo/Cut/Copy/Paste) to the field,
so the menus on the broker, topic and certificate fields drew with no panel.
QMenu gets its canon panel back, as on TX Band Settings.

mqtt_settings_dialog_test (8 checks, socket-free, never accepted) covers the
opaque field menu, failing without the QMenu rules, and records that Esc,
Ctrl+W / Cmd+W and the corner button each destroy a WA_DeleteOnClose window,
so a half-edited field never survives into the next open.

The screenshot's alt text names the canon header and the corner close button.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolves the conflicts in the three docs/style/* canon-window lists, where
main added FFTW Wisdom / canon/nr2Wisdom. Every list keeps main's entries
and this branch's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MQTT Settings moves to CanonWindow

1 participant