[ten9876]

Summary

Part of #6239 (style guide follow-ups to RFC #6226). Three canon migrations, one commit each.

1. AetherModem moves to CanonWindow, following the About window's recipe for tool windows, as AetherRX and AetherTX did in #6334:

• No title bar, the ambient ground, a canon header and the corner close button. Drag any empty area to move it, resize it from every edge, and close it with Esc or Ctrl+W.
• It's a CanonWindow::Kind::Workspace: an independent top-level window with its own taskbar entry, where Return in a field never clicks a default button.
• It stays a service host. MainWindow keeps one instance for the session because it owns the KISS TCP server and the APRS modem, so closing it only hides it.
• Geometry persists under its old key (Ax25HfPacketDecodeDialogGeometry), so saved positions carry over.
• The minimum height grows from 680 to 720 px, the extra height of the header that replaces the title bar, so the tabs' fields keep their height.
• It sits under the canon/aetherModem theme scope. Its stylesheet and the D-STAR page's are rewritten in color.canon.* tokens:
  • selection and focus are canon cyan (maintainer's choice);
  • status dots keep green, amber and red;
  • the log's rich-text colours resolve through the window's scope.

2. Canon check boxes and radio buttons (gui/CanonIndicators.h), now the style guide's building block:

• Look: 18 px; ModemChrome's own indicator rules are removed from its canon look, so only the painted images draw. A checked box is cyan with an onAccent tick, a selected radio is a cyan ring with a centre dot, and each has a muted disabled variant. Before, a checked box only changed fill and a selected radio was only an outline, so it was hard to see what was selected.
• Painted: they're drawn at 1x, 2x and 3x, because a stylesheet border can draw neither a tick nor a clean translucent circle.
• Cached safely: they're cached through the new gui/ThemedImageCache.h, which ComboStyle.h's combo arrow now uses too (maintainer's call to fold it in). It writes to the user's own cache directory, never the shared temp directory, keyed by every colour painted. Writes are atomic, and it fails closed: if the images can't be written, the rules are left out and Qt's own indicators and arrow show rather than blank ones. Paths are quoted in url("...").
• Theme: applyCanonSheet() rebuilds the sheet on a theme change, so the images follow the theme.
• Everywhere: every canon window uses them: AetherModem, AetherRX, AetherTX, Network Diagnostics, Waveforms and Remote Access.

ModemChrome gains a canon look (ModemChrome::Look::Canon):

• Slots: every colour in its sheet is now a named slot filled per look.
• Modem look: what the docked applets wear, and byte-identical to main (13,481 bytes at each scale, compared against the merge base).
• Canon look: canon surfaces and ink, cyan selection, stage tabs in ink, the canon indicators and a list card.
• Who uses it: AetherRX/AetherTX's stage column and AetherRX's AetherNR page (AetherDspWidget in dialog mode). Those had kept the old navy-and-green chrome.
• AetherNR page: its hard-coded label colours become text tokens, and its painted parts resolve through the window's scope. In the docked applet, primary labels are unchanged, while the secondary and disabled labels move a shade to the theme's own values.

3. The AetherRX and AetherTX Settings dialogs (the profile library behind the gear) are short-lived CanonWindow dialogs. They're still modal over their window, use the canon look, and are 40 px taller for the header.

Docs:

• docs/style/{aethersdr-style-guide,dialog-patterns,theme-style-guide}.md add the canon check box and radio button, ModemChrome's canon look, and the new canon windows.
• The AetherModem user-guide page describes the window behaviour.

Touchpoints: core/ThemeManager.h includers go 171 → 173, from DStarModemPage.cpp and the new CanonIndicators.h. Both are UI theming (ui-support), and the manifest is regenerated.

Stale screenshots, to recapture after merge: aethermodem-aprs.png (aethermodem-packet-radio.md), aetherrx-profiles.png (aetherial-audio.md), and the AetherRX/AetherTX stage-column shots aetherrx.png, aethertx.png, aethertx-eq.png, aethertx-compressor.png and first-session-aetherrx-nnr.png. They show the old chrome and green indicators.

Known and unchanged from main: the Mailbox tab's Listen callsign and Vanity alias fields clip their text the same way on main.

Constitution principle honored

Principle VIII (Evidence Over Assertion): each behavioural claim below has a test that fails without its fix, a byte comparison, or a render. The visual direction is RFC #6226's.

Test plan

• Local build passes

• canon_window_test samples the 3x indicator images: the checked box shows its tick, the selected radio its dot, and the unchecked ones neither. Removing the tick and the dot fails it.

• modem_chrome_test: both looks fill every %N argument and @slot@, only the canon look carries the canon indicators, and the canon look's only indicator rules are the painted ones.

• canon_window_test also covers the cache:

  • the key changes when aqua or line change;
  • an unwritable cache gives empty indicator rules, and they recover once it's writable;
  • the combo arrow's rule likewise disappears when blocked and names a quoted path otherwise.

  Each of these fails when its fix is removed.

• The modem look is byte-identical to the merge base at both scales: old and new ModemChrome.cpp built side by side, and their output compared.

• canon_window_test, modem_chrome_test, stage_tab_bar_drag_test, theme_manager_test, theme_seed_test, compact_metrics_test, rx_chain_runner_test, rx_stage_reorder_test, window_show_state_test and comp_makeup_fader_test pass locally.

• The static-check scripts pass locally. The touchpoint manifest is regenerated.

• The user docs site builds (npm ci && npm run build), and gen_reference --check passes.

• Rendered offscreen with the demo app on scratch settings:

  • all six AetherModem tabs;
  • AetherRX (AetherNR and Gate) and AetherTX;
  • Network Diagnostics (Overview and Application Logs);
  • Waveforms;
  • both Settings dialogs;
  • a 2x render of the indicators.

• The maintainer ran the AetherModem build against the FLEX-8600. The check box and radio fix came from that run.

• Remote Access was not rendered. It only opens with a radio running the tailnet shim connected; its change is the same shared indicator rules.

Checklist

• Commits are signed
• No new flat-key AppSettings calls. Existing geometry keys are reused.
• Code is clean-room
• All meter UI uses MeterSmoother (unchanged)
• Documentation updated: docs/user/docs/aethermodem-packet-radio.md, docs/style/*
• Security-sensitive changes: n/a

🤖 Generated with Claude Code

[github-actions]

Accessibility check findings

This PR touches src/gui/ files with 1 accessibility issue(s).
Full details appear as inline annotations on the diff above.

Files scanned : 16
Files with findings: 1
Total findings : 1

See docs/a11y.md for the canonical
Qt patterns (setAccessibleName, QAccessibleValueChangeEvent,
QAccessibleInterface subclasses, etc.). Structural remediation
(keyboard navigation, custom QAccessibleInterface subclasses)
is tracked in #3288.

Findings are warnings only — the build is not blocked.

[ten9876]

Implements the surfaces explicitly picked in tracking issue #6239 under approved RFC #6226: AetherModem becomes a persistent service-host workspace on CanonWindow; the RX/TX profile libraries become short-lived canon dialogs; the shared indicators and canon ModemChrome look reach the named windows. No blocking correctness or governance defect found. The custom-theme cache and screenshot follow-ups below are nonblocking.

Scope

File/group	Change; claimed in title/body?	Verdict
Ax25HfPacketDecodeDialog.{h,cpp}, MainWindow_DigitalModes.cpp	CanonWindow workspace, old geometry key, canon styles; yes	In approved scope; service ownership retained
AetherRxSettingsDialog.{h,cpp}, AetherTxSettingsDialog.{h,cpp}	Modal profile libraries on CanonWindow; yes	In approved scope
CanonIndicators.h, ModemChrome.{h,cpp}	Painted states, per-look colours, theme rebuild helper; yes	In approved scope; cache nit below
AetherDspWidget.cpp, StageTabBar.cpp	Canon dialog look and scoped painting; disclosed applet label token cleanup	In scope; shared-widget effect disclosed
DStarModemPage.cpp, NetworkDiagnosticsDialog.cpp, TailnetShimDialog.cpp, WaveformsDialog.cpp	Canon tokens/shared indicators; yes	In approved scope
Two modified test sources	Indicator pixel sampling and both-look sheet checks; yes	In scope; mutation checks detect removed drawings/rules
Style/user docs and touchpoint manifest	Behaviour/style documentation and ThemeManager includer count; yes	In scope; stale screenshots need disclosure

All three commits were authored October 10 UTC (October 9 Pacific). No unrelated commit or file, new dependency/thread, base-token change, new settings key, CHANGELOG entry, or weakened baseline found. The two new ThemeManager includers explain the disclosed 171→173 manifest count; the UI-support classification is unchanged. The tracker explicitly lists all three migrations, so the bundled visual work has maintainer direction.

Blockers

None.

Nits — nonblocking

1. Include every painted colour in the cache key — src/gui/CanonIndicators.h:78. The key omits line and aqua, although disabled outlines and the checked-box edge use them. A Qt 6.12 probe edited only these two root theme tokens, then regenerated the rules:

   changed line=#ff00ff aqua=#ff00ff rules_identical=1
   checked_box_top_edge=#8ef7e6 expected=#ff00ff
   

   The existing PNG is reused, so those theme edits do not reach the indicators. Normal Dark/Light switches may mask it by changing another keyed colour. Add the missing colours:

       const QString key = QString(control.name(QColor::HexArgb) + lineHi.name(QColor::HexArgb)
                                   + line.name(QColor::HexArgb) + cyan.name(QColor::HexArgb)
                                   + aqua.name(QColor::HexArgb) + onAccent.name(QColor::HexArgb)
                                   + muted.name(QColor::HexArgb)).remove(QLatin1Char('#'));

   This was reproduced through the image-generation helper in a standalone Qt process, not the desktop Theme Editor. A socket-free test changing only one omitted token would cover it.

2. Refresh or disclose stale screenshots — docs/user/docs/aethermodem-packet-radio.md:31, docs/user/docs/aetherial-audio.md:206. The pages still embed aethermodem-aprs and aetherrx-profiles from before these window migrations; the changed screenshot files are absent and the PR body does not name stale IDs. The AGENTS user-doc workflow permits either refreshing affected shots or saying which are stale. Name these and the affected RX/TX stage-column shots in the body if captures are deferred.

What I tried to break

• Built only AetherSDR and the ten named focused test targets, with pinned Qt 6.12, shared core, -O2 -g1 and isolated test settings. All ten pass. The first invocation lacked the offscreen platform for rx_chain_runner_test and aborted during Qt display initialization; setting QT_QPA_PLATFORM=offscreen resolves it. This is an environment issue, not a PR failure. Both modified tests and the additional selected targets were inspected for socket ownership; no socket-owning test was run.
• Removed tick/dot drawing in separate scratch mutants: canon_window_test fails both positive pixel assertions. Removed the Canon indicator append: modem_chrome_test fails its new look check. Original exact-head tests pass. Mutation binaries use exact PR sources and unchanged pinned-Qt settings/theme support objects from the earlier review; no production sources were edited.
• Compiled old/new ModemChrome side by side: the default Modem look is byte-identical at Dialog and Compact scales. Engine-boundary findings remain 102 with only shifted source lines; capability and command-plane findings are identical. Both manifests check clean; registration, frozen CI gate and generated user-reference checks pass. The accessibility bot warning at Ax25HfPacketDecodeDialog.cpp:3333 is pre-existing unchanged code, verified against the merge base.
• Drove only owned review instances (PIDs 2346720, 2349613 and 2351782), offscreen with isolated settings, the explicit aethersdr-pr6339-review socket and TX disabled. The radio response was {"model":"AetherSDR Demo","serial":"DEMO-0001","connected":true,"transmitting":false}. Captured and inspected all five demo-visible modem tabs, RX/TX, both modal Settings dialogs, Network Diagnostics and an RX Dark→Light→Dark switch. Checked tick/dot state in actual window captures. Hiding/reopening the modem retained {"profileId":"Vhf1200","baud":1200,"enabled":false}. After resizing to 1180×860 and restarting, the bridge returned {"geometry":{"x":200,"y":12,"w":1180,"h":860}}. A separate legacy-base64 seed restored to {"geometry":{"x":1,"y":39,"w":1080,"h":760}}, matching Qt's existing restoration/clamping behavior on the same virtual screen. The first legacy attempt used a different virtual-screen size and did not preserve the requested rectangle, so it was repeated with matching configuration rather than classified as a PR regression. Slice removal and simulated disconnect completed without a crash. All owned instances were closed and their exit confirmed; no live radio was connected.

Verification limits and CI

The demo hides D-STAR and disables Waveforms, and Remote Access requires a connected tailnet-shim radio; those three changed surfaces were inspected in source but not rendered locally. No live KISS/APRS service was enabled, so continuity of an actively running service was checked through retained ownership/teardown code rather than live traffic. OS taskbar behavior, compositor move/resize gestures and native Windows/macOS runtime were not demonstrated. The shared-core build differs from shipping linkage; none of the findings depends on that choice. The desktop theme switch worked, but the two-colour cache nit was reproduced in the standalone helper rather than through Theme Editor. A full local default-target build or broad suite was not run.

Required build, check-windows, check-macos and Static checks are successful. Head is 8f53d2b76438e34ec3deaec182b9f0aea263f5bc; CI/Static/Docs runs were created 2026-10-10T04:48:41Z, after current main ca5e0634d3 (2026-10-10T02:35:53Z). No stale-main timing gap found. The docs-site CI build passed; I did not repeat npm installation/build locally. The optional automated code-review skill was unavailable; manual and parallel audits were performed.

Recommendation: Approve with nits. The picked migrations, retained service ownership and focused regressions hold up to the checks above. Fix the cache key and refresh or disclose affected screenshots. This is a COMMENT review, not an approval; approval and merge remain the operator's call.

[aethersdr-agent]

Issue fit

Yes, for all three picked items. #6239's "Picked: tool windows on CanonWindow" names this PR against exactly three lines — AetherModem (Ax25HfPacketDecodeDialog), the AetherRX/AetherTX Settings dialogs, and the canon check boxes/radio buttons plus ModemChrome's canon look — so three items in one PR is the maintainer's own tracking ruling, not a bundling violation. Each lands as its own signed commit. The docs half of the ask is there too: dialog-patterns.md, the style guide, theme-style-guide.md and the user page all gained the AetherModem rows, and the user page documents the chrome change the operator will actually notice (no title bar, Esc/Ctrl+W, closing only hides).

One claim I want to flag as not holding up: src/gui/ModemChrome.cpp (end of styleSheet) says of the canon indicators "their rules are more specific than the ones above". They are not — see nit 1.

Scope

File / group	What it changes	Claimed?	Verdict
docs/style/{aethersdr-style-guide,dialog-patterns,theme-style-guide}.md, docs/user/docs/aethermodem-packet-radio.md	AetherModem + Settings added to the canon lists; new style-guide rows for the indicators and the canon ModemChrome look; user-facing chrome paragraph	Yes	In scope
docs/architecture/aetherd-touchpoints.md	ThemeManager.h 171→173	Implied	In scope — two new includers (CanonIndicators.h, DStarModemPage.cpp); the manifest check is green
src/gui/CanonIndicators.h (new)	Painted canon indicators + applyCanonSheet()	Yes	In scope; referenced by 6 TUs — not a dead addition
src/gui/Ax25HfPacketDecodeDialog.{h,cpp}, src/gui/DStarModemPage.cpp	PersistentDialog → CanonWindow::Kind::Workspace; sheet and log HTML to tokens	Yes	In scope (the D-STAR page is only ever a tab inside AetherModem)
src/gui/MainWindow_DigitalModes.cpp	drops setFramelessMode() ×3 and trackPersistentDialog() ×3	Yes	Correct and necessary — the class is no longer a PersistentDialog; the only thing lost is View → Frameless propagation, which dialog-patterns.md documents as deliberate
src/gui/Aether{Rx,Tx}SettingsDialog.{h,cpp}	QDialog → CanonWindow, canon body sheet, 460×380 → 460×420	Yes	In scope
src/gui/ModemChrome.{h,cpp}, src/gui/StageTabBar.cpp	Look::{Modem,Canon}; stage column takes Canon	Yes	In scope — StageTabBar is only built by AetherRxDialog.cpp:129 and AetherialAudioStrip.cpp:138, both canon windows
src/gui/{NetworkDiagnostics,TailnetShim,Waveforms}Dialog.cpp	delete local fill-only indicator rules, append canonIndicatorRules()	Yes ("in every canon window")	In scope
src/gui/AetherDspWidget.cpp	setDialogMode applies the canon sheet; ModemChrome::colour(…, this); label hex → {{color.text.*}}	Mostly	In scope, with one undisclosed side effect — nit 4
tests/{canon_window,modem_chrome}_test.cpp	indicator pixel assertions; canon-look sheet assertions	Yes	In scope

Nothing in the diff is unexplained by #6239. No new protocol verb, settings key, capability or CLI surface. No CHANGELOG.md entry (correct). No removed guard; the one deleted comment block (AetherDspWidget.cpp:703, "Nothing left to do per-widget…") is replaced by a real implementation, not by a regression.

Socket tests: none added, modified or removed. No QTcpServer/QUdpSocket/bind/listen/connectToHost and no Fake* peer in either changed test; both are offscreen widget tests.

Blockers

1. src/gui/CanonIndicators.h:45-58 — the indicator cache fails open, and the failure mode is invisible check boxes.

    if (QFile::exists(path) && QFile::exists(base + QStringLiteral("@3x.png"))) {
        return path;
    }
    for (int scale = 1; scale <= 3; ++scale) {
        ...
        pm.save(scale == 1 ? path : base + QStringLiteral("@%1x.png").arg(scale), "PNG");
    }
    return path;

pm.save()'s return value is discarded and return path; runs unconditionally, so a path that was never successfully written still gets interpolated into the sheet:

        "QCheckBox::indicator, QRadioButton::indicator {"
        " width: 18px; height: 18px; border: none; background: transparent; }"
        "QCheckBox::indicator:unchecked { image: url(%1); }"

Because the same rule set clears the border and the background, a missing image leaves nothing drawn at all — not a degraded box, an empty one. Every check box and radio button in every canon window (AetherModem's Enable Modem and Autostart, Network Diagnostics, Waveforms, Tailnet, the AetherNR page, the stage column) would be blank, both states identical. That is precisely the outcome the header and the new style-guide row say must never happen: "Never a fill or ring alone: the operator must be able to see what is selected."

Reachable without an adversary: a read-only or full TMPDIR, a restrictive TMPDIR on a hardened host, or an existing unwritable file at that name. And with one — this is the confirmed half of the security audit (security-audit (CodeGuard), codeguard-0-file-handling-and-uploads, medium, Linux shared /tmp only; macOS /var/folders and Windows %TEMP% are per-user) — the name is fully predictable (aethersdr_canon_<one of 8 literals>_v1_<hex of five shipped theme tokens>), so a local user can pre-create both files and have Qt decode their PNG for every indicator in the app: UI-state spoofing on toggles that gate TX-adjacent behaviour, plus untrusted bytes into libpng. If only <base>.png is pre-created, the two-file gate falls through, save() fails silently, and the attacker's file is used anyway.

Worth saying plainly: src/gui/ComboStyle.h:32 already does this (temp dir, colour-keyed name, single-file gate, unchecked save()), so the pattern is pre-existing and hardening it everywhere belongs in its own issue — this PR is only on the hook for the new copy. Fail-closed plus a per-user directory covers both the deterministic break and the local-attacker case:

inline QString canonIndicatorPath(const QString& name, const QString& colourKey,
                                  const IndicatorPainter& paint)
{
    constexpr int kSize = 18;
    // Bump when the drawing changes: the cache is keyed by colours, so an old
    // file would otherwise outlive a new design.
    constexpr int kDrawingVersion = 1;
    const QString dir = QStandardPaths::writableLocation(QStandardPaths::CacheLocation)
                        + QStringLiteral("/canon-indicators");
    if (!QDir().mkpath(dir)) {
        return {};
    }
    const QString base = dir + QStringLiteral("/%1_v%2_%3")
        .arg(name).arg(kDrawingVersion).arg(colourKey);
    const QString path = base + QStringLiteral(".png");
    if (QFile::exists(path) && QFile::exists(base + QStringLiteral("@2x.png"))
        && QFile::exists(base + QStringLiteral("@3x.png"))) {
        return path;
    }
    for (int scale = 1; scale <= 3; ++scale) {
        QPixmap pm(kSize * scale, kSize * scale);
        pm.fill(Qt::transparent);
        QPainter p(&pm);
        p.setRenderHint(QPainter::Antialiasing);
        p.scale(scale, scale);
        paint(p);
        p.end();
        // Fail closed: a path we did not write would render as no indicator at
        // all, since the rules below clear the border and the background.
        if (!pm.save(scale == 1 ? path : base + QStringLiteral("@%1x.png").arg(scale), "PNG")) {
            qCWarning(lcGui) << "canon indicators: cannot write" << path;
            return {};
        }
    }
    return path;
}

…and in canonIndicatorRules(), return QString() if any of the eight paths came back empty, so the window falls back to Qt's native indicators instead of blank squares. (Needs <QStandardPaths>; lcGui or a local category for the warning.)

Two smaller things to fold into the same edit:

• The cache key omits two tokens it paints with. key is built from control, lineHi, cyan, onAccent, muted (CanonIndicators.h:78-80), but aqua is the checked box's edge (:117) and line is the disabled edge (:119, :127). Two themes differing only in color.canon.aqua or color.canon.line share a filename and serve each other's images — the exact collision the header comment says the key prevents. Add both.
• image: url(%1) embeds the path unquoted. A temp path containing a space is routine on Windows (C:/Users/John Smith/AppData/Local/Temp/…), and Qt's CSS url() token is not reliable for unquoted paths with spaces or parens. Quoting it (image: url("%1")) is free insurance, and with CacheLocation the path still comes from the user profile. I could not render this to confirm Qt's lexer behaviour — flagging it as cheap to make moot rather than as a proven break. (tests/canon_window_test.cpp:248's url\\(([^)]+)\\) regex would need the quotes stripped.)

Nits

• src/gui/ModemChrome.cpp (end of styleSheet) — "their rules are more specific than the ones above" is not right for the state rules. canonIndicatorRules()'s generic QCheckBox::indicator, QRadioButton::indicator { border: none; background: transparent; } wins over the base unchecked block only on source order (equal specificity), and loses to the base QCheckBox::indicator:checked { background: @checkFillChecked@; border-color: %18; } and QRadioButton::indicator:checked:hover { border-color: %19; }, which are more specific. So in the canon look a checked box/radio should still get a 2 px cyan/aqua QSS ring drawn around the painted 18 px image. The three dialogs in this very diff took the clean route — NetworkDiagnosticsDialog.cpp, WaveformsDialog.cpp and TailnetShimDialog.cpp all delete their indicator blocks. Doing the same here (elide the QRadioButton::indicator* / QCheckBox::indicator* blocks when look == Look::Canon) removes the question. Reasoned from QSS specificity; I can't render it to show the ring.
• src/gui/CanonIndicators.h:150-158 — applyCanonSheet() double-registers. ThemeManager::applyStyleSheet already tracks the widget and reapplies on themeChanged (ThemeManager.cpp:354), so each theme switch applies twice: first the tracked reapply, which re-expands the stored template still carrying the old theme's PNG paths, then this connection's rebuild with the new key. Correct only because ThemeManager's connection is made in its constructor and therefore runs first. Worth one line of comment, or skipping the tracked path for these sheets.
• tests/canon_window_test.cpp:248 and tests/modem_chrome_test.cpp:122 write the same eight temp PNGs; whichever runs second can hit the two-file exists-gate against a partially written @3x.png under ctest -j and sample torn bytes. Today they differ (modem_chrome_test links no ${THEME_TEST_RESOURCES}, so its canon tokens come from the seed and the colour key differs), so this is latent, not live. Writing to a per-test CacheLocation under TestSettingsProfile would close it for good.
• src/gui/AetherDspWidget.cpp:755-760, :1230-1231, :1462-1463 — the hex→token migration changes the docked AetherNR applet too, not just the canon page, and the body doesn't mention it. In dark it is near-identical (#c8d8e8 → color.text.primary = #c8d8e8 exactly; #8090a0 → color.text.secondary = #8ea8c0; #48515a → color.text.disabled = #3a4a5a) and in light it is a strict improvement, so this reads as a welcome ratchet payment rather than a smuggled restyle — but #6239 says "nothing here authorises restyling a screen as a side effect of other work", so it is worth a line in the PR body.
• tests/modem_chrome_test.cpp:122 asserts only that the canon sheet contains aethersdr_canon_check_on — it would still pass with an empty tick. The real guard is canon_window_test's pixel sampling, which would fail if the tick path or the dot were dropped. Good pairing; no change wanted, just noting which half carries the weight.

What I tried to break

• "Saved geometry carries over" (Ax25HfPacketDecodeDialog.cpp:890). PersistentDialog stored saveGeometry().toBase64(); CanonWindow stores "x,y,w,h". The claim holds: a base64 blob has no ,, so split(',') yields 1 part and CanonWindow.cpp:188 falls through to the restoreGeometry(fromBase64(...)) branch, then the next save rewrites it in the new form. The minimumSize bump 680→720 is absorbed by expandedTo(minimumSize()) at :173.
• The shared-instance trap on setDialogMode. It used to be a no-op and is now a one-way canon restyle, and the style guide calls the AetherNR page "the docked applet's widget" — so I checked whether the docked applet and AetherRX share one AetherDspWidget. They don't: AetherRxDialog.cpp:172 and ClientRxDspApplet.cpp:28 each construct their own, and setDialogMode(true) has exactly one caller (AetherRxDialog.cpp:173). The docked applet keeps Look::Modem. This was my leading blocker candidate and it did not survive.
• Return/Enter after Kind::Workspace. AetherModem has three QLineEdit::returnPressed paths (TX text :1214, terminal :3937, APRS message :4602). None relied on auto-default: :1479-1482 already clears setAutoDefault/setDefault on every body button (pre-existing, commit 6af248b), and Workspace additionally suppresses default buttons. closeSettingsIfOpen()'s reject() and both dlg.exec() call sites still compile and behave, since CanonWindow derives from QDialog.
• What dropping trackPersistentDialog() costs. Read MainWindow::setFramelessWindow (MainWindow.cpp:10202): the tracked list is used for setFramelessMode() propagation only — no shutdown geometry flush, no disconnect teardown. Nothing else is lost, and CanonWindow::hideEvent (:138) does the geometry flush Escape/⌘W needs.
• Unresolved theme tokens. Extracted every {{token}} added across Ax25HfPacketDecodeDialog.cpp, DStarModemPage.cpp, AetherDspWidget.cpp, ModemChrome.cpp and CanonIndicators.h and resolved each against both shipped themes — all present (color.accent.* live as flat dotted keys, so a naive nested walk reports false misses). All seven color.canon.* the indicators paint with are also in ThemeSeedGenerated.cpp, so the compiled-seed fallback doesn't render them transparent. canon_window_test does link ${THEME_TEST_RESOURCES}, so its pixel assertions run against the real palette rather than an all-zero one.
• CodeGuard's 13 handed-down leads: all refuted (security-audit (CodeGuard) re-read every cited line). The eight CS-VLN-CMDI-BACKTICK hits are Markdown backticks in C++ comments or in operator-help strings passed to automationError(); CS-PII-DOB-ISO is "Observed 2026-07-31 on the HL2" in a comment; the three SSRF hits are a comment about AppSettings keys, a comment showing "127.0.0.1:51234 (wsjtx)" as a display format, and a QLineEdit placeholder; CS-PII-EMAIL is you@example.com placeholder text (RFC 2606); CG-PATH-001 is compile-time ../Resources/ bundle probing feeding a QFileDialog start directory; CS-VLN-PATH-UNC is "^[\x20-\x7B\x7D-\x7E]{0,20}$", a D-STAR message validator, not a path. Only the CanonIndicators.h file-handling finding survived.
• Could not check: anything requiring a run. No build, no GUI, so the double ring, the blank-indicator failure and the Windows url() lexing are all reasoned from the code and from Qt's documented QSS behaviour, not observed. CI is green on 8f53d2b across all 10 checks — but note that ci.yml only ever runs ctest -R against a named handful (lines 820, 832, 1174, 1189); neither canon_window_test nor modem_chrome_test is among them, so green does not mean the new assertions passed anywhere.

Recommendation

Request changes — on blocker 1 alone. The three-window move itself is careful, well-documented work that survived everything I threw at it: the geometry migration, the Return-key seam, the shared-widget trap, the propagation that got deleted, and every token. What needs fixing is one new helper that hands back a file path it never confirmed writing, into rules that clear the border and the background, so the failure renders as no indicator at all; the fix is mechanical and the cache-key and quoting tweaks ride along in the same edit. Nit 1 (dropping the shadowed indicator blocks from Look::Canon, as the other three dialogs in this diff already do) is the one I'd most like to see taken even though it's non-blocking. The ComboStyle.h:32 twin should get its own hardening issue rather than being pulled in here. Thanks for the user-facing paragraph on the new chrome — that's the part operators will actually hit first.

---

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

[aethersdr-agent]

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

• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/AetherDspWidget.cpp /tmp/aetherclaude/pr-6339/src/gui/AetherDspWidget.cpp:123
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/Ax25HfPacketDecodeDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/Ax25HfPacketDecodeDialog.cpp:168
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/Ax25HfPacketDecodeDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/Ax25HfPacketDecodeDialog.cpp:1636
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/Ax25HfPacketDecodeDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/Ax25HfPacketDecodeDialog.cpp:1893
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/Ax25HfPacketDecodeDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/Ax25HfPacketDecodeDialog.cpp:1974
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/Ax25HfPacketDecodeDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/Ax25HfPacketDecodeDialog.cpp:2658
• [MEDIUM] CS-PII-DOB-ISO — PII detected: dob in src/gui/Ax25HfPacketDecodeDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/Ax25HfPacketDecodeDialog.cpp:2661
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/Ax25HfPacketDecodeDialog.h /tmp/aetherclaude/pr-6339/src/gui/Ax25HfPacketDecodeDialog.h:154
• [HIGH] CS-VLN-SSRF-PRIV-10 — Vulnerability: CS-VLN-SSRF-PRIV-10 in src/gui/NetworkDiagnosticsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/NetworkDiagnosticsDialog.cpp:1175
• [HIGH] CS-VLN-SSRF-LOCALHOST — Vulnerability: CS-VLN-SSRF-LOCALHOST in src/gui/NetworkDiagnosticsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/NetworkDiagnosticsDialog.cpp:1547
• [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/gui/NetworkDiagnosticsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/NetworkDiagnosticsDialog.cpp:431
• [LOW] CS-PII-IP — PII detected: ip_address in src/gui/NetworkDiagnosticsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/NetworkDiagnosticsDialog.cpp:1175
• [LOW] CS-PII-IP — PII detected: ip_address in src/gui/NetworkDiagnosticsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/NetworkDiagnosticsDialog.cpp:1547
• [HIGH] CS-VLN-SSRF-PRIV-192 — Vulnerability: CS-VLN-SSRF-PRIV-192 in src/gui/TailnetShimDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/TailnetShimDialog.cpp:281
• [HIGH] CS-VLN-SSRF-PRIV-192 — Vulnerability: CS-VLN-SSRF-PRIV-192 in src/gui/TailnetShimDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/TailnetShimDialog.cpp:281
• [MEDIUM] CS-PII-EMAIL — PII detected: email in src/gui/TailnetShimDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/TailnetShimDialog.cpp:238
• [LOW] CS-PII-IP — PII detected: ip_address in src/gui/TailnetShimDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/TailnetShimDialog.cpp:281
• [LOW] CS-PII-IP — PII detected: ip_address in src/gui/TailnetShimDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/TailnetShimDialog.cpp:281
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/WaveformsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/WaveformsDialog.cpp:218
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/WaveformsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/WaveformsDialog.cpp:219
• [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/WaveformsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/WaveformsDialog.cpp:220
• [HIGH] CS-VLN-PATH-UNC — Vulnerability: CS-VLN-PATH-UNC in src/gui/WaveformsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/WaveformsDialog.cpp:1255
• [HIGH] CS-VLN-PATH-UNC — Vulnerability: CS-VLN-PATH-UNC in src/gui/WaveformsDialog.cpp /tmp/aetherclaude/pr-6339/src/gui/WaveformsDialog.cpp:1255

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

[github-actions]

Accessibility check findings

This PR touches src/gui/ files with 1 accessibility issue(s).
Full details appear as inline annotations on the diff above.

Files scanned : 18
Files with findings: 1
Total findings : 1

See docs/a11y.md for the canonical
Qt patterns (setAccessibleName, QAccessibleValueChangeEvent,
QAccessibleInterface subclasses, etc.). Structural remediation
(keyboard navigation, custom QAccessibleInterface subclasses)
is tracked in #3288.

Findings are warnings only — the build is not blocked.

[ten9876]

Addressed in 20f28a9.

@aethersdr-agent: blocker, the indicator cache fails open. Fixed by the new src/gui/ThemedImageCache.h, which both the canon indicators and ComboStyle.h's combo arrow now use (the maintainer's call to fold the arrow in):

• Location: it writes to the user's own QStandardPaths::CacheLocation, never the shared temp directory.
• Atomic writes: each file goes through QSaveFile.
• Fails closed: a path comes back only once every scale was written. Otherwise canonIndicatorRules() and comboArrowRule() return nothing, and the window keeps Qt's own indicators and arrow.

canon_window_test blocks the cache directory with a regular file. The indicator rules and the arrow rule then come back empty, and both recover once the block is removed. Mutation: returning a path despite the failure fails it at canon_window_test.cpp:316 (indicators) and :329 (arrow).

Both reviews: cache key missing line and aqua. Fixed; the key now covers all seven colours painted. canon_window_test changes only color.canon.aqua, then only color.canon.line, and expects a new rule set each time. With the two colours removed from the key it fails at :301 and :302.

Unquoted url(). Fixed: url("...") for both the indicators and the arrow. The test's regex reads the quoted form, and the arrow test asserts the quote.

Nit: ModemChrome's :checked indicator rules outrank the canon reset. You were right, and the comment was wrong. The canon look now removes ModemChrome's own indicator rules before adding the painted ones. modem_chrome_test asserts that the canon sheet's only ::indicator rules are the painted ones; keeping the old rules fails it. The modem look is still byte-identical to the merge base at both scales (13,481 bytes, old and new ModemChrome.cpp built side by side).

Nit: applyCanonSheet() registers twice on a theme change. It's documented in place now. ThemeManager's own reapply runs first, because its connection is made in its constructor, so the rebuild with the new theme's images lands last.

Nit: tests sharing temp PNGs under ctest -j. Closed by the above. Each test now writes to its own TestSettingsProfile cache through atomic writes.

Nit: the docked applet's label change is undisclosed. It's in the description: "In the docked applet, primary labels are unchanged, while the secondary and disabled labels move a shade to the theme's own values."

@ten9876: stale screenshots. The description now names them: aethermodem-aprs.png, aetherrx-profiles.png, aetherrx.png, aethertx.png, aethertx-eq.png, aethertx-compressor.png and first-session-aetherrx-nnr.png.

Automated /code-review during landing. Its findings 1–3 and 5 are the fixes above. #9 is also fixed: AetherModem's loop clearing auto-default buttons is removed, because the workspace kind's Return handling made it dead. #7 is closed without change: AetherModem's log lines were fixed hex on main; new lines now follow the theme. Closed without change by maintainer decision:

• the 720 px minimum height stays;
• the indicators keep the root theme's colours (no bundled theme overrides color.canon.* per scope);
• cached image sets aren't pruned (per-user, about 1 KB each).

Accessibility bot (Ax25HfPacketDecodeDialog.cpp:3333). Pre-existing code that this PR doesn't change; @ten9876's review checked it against the merge base.

CodeGuard. All 13 leads were refuted in the agent review above. The one real file-handling finding is the cache fix in this commit.

Verification at 20f28a9:

• The build and 11 focused tests pass, including ctcss_tone_label_test for ComboStyle.
• The static checks pass, and the touchpoint manifest is unchanged.
• The offscreen demo app renders the indicators and the combo arrows from ~/.cache/AetherSDR/AetherSDR/{canon-indicators,combo-arrows}/ with no stylesheet parse warnings.