Skip to content

feat(gui): FreeDV Reporter is a CanonWindow workspace, built like AetherRX and AetherTX. Principle VIII. - #6757

Open
w5jwp wants to merge 3 commits into
aethersdr:mainfrom
w5jwp:feat/freedv-reporter-canonwindow
Open

w5jwp wants to merge 3 commits into
aethersdr:mainfrom
w5jwp:feat/freedv-reporter-canonwindow

Conversation

@w5jwp

@w5jwp w5jwp commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #6755. 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.*. Please don't merge before the pick. Merging is that pick: aethersdr-style-guide.md and dialog-patterns.md list FreeDV Reporter among the tool windows the maintainer has moved to CanonWindow, which is true only once this lands.

FreeDV Reporter (FreeDvReporterDialog) moves from PersistentDialog to a CanonWindow workspace, using the same approach as AetherRX, AetherTX and SpotHub (#6401):

  • Chrome: no title bar; the canon background, the canon header and the corner close button. Drag any empty area to move it, resize it from any edge, close it with Esc or ⌘W / Ctrl+W.
  • Kind: CanonWindow::Kind::Workspace, an independent top-level window with its own taskbar entry. showFreeDvReporter() already created it directly rather than through showOrRaisePersistent(), so MainWindow is unchanged.
  • Return: a workspace window doesn't turn Return into a default-button click. Under PersistentDialog, the band buttons were auto-default, so Return in the message field could also click the first band button. Now Return only sends, through the field's returnPressed.
  • Geometry: persists under its old key (FreeDvReporterGeometry). CanonWindow saves x,y,w,h; it reads the base64 saveGeometry() value the PersistentDialog left there once, and replaces it on the next save, so saved positions carry over.
  • Size: minimum height 390 (was 350), to make room for the header. The minimum width is 780 or whatever the bottom row needs, whichever is larger: an explicit minimum overrides the layout's own, and the canon's semibold band buttons need more than 780 (825 with the offscreen fonts), so Track and Band would otherwise clip.
  • Closing hides it, as with AetherTX: MainWindow keeps sending it stations while it is closed, so reopening shows the same window with the list already filled.

Look. It sits under a new canon/freedvReporter theme scope with one canon sheet, based on SpotHub's:

  • the station list is nested, with a cyan header row whose outer corners follow the table's rounded border, and canon scrollbars;
  • the message field sits on control; disabled (reporting off) it reads muted;
  • disabled Send keeps canon.muted ink (6.71:1 dark, 5.94:1 light on the ground), so it reads as greyed-out rather than absent, which is how the row explains its gating (Allow custom status message configuration in FreeDV Reporter panel #4231). The rule is SpotHub's. Its canon.line border is 4.03:1 in dark but 1.55:1 in light, under the 3:1 non-text floor, so in light the disabled button reads by its text alone. That's a property of the canon palette that every canon window shares, not something this PR introduces;
  • the Track check box and the Band / Freq radio buttons use the canon indicators;
  • a selected band button is aqua on the nested fill, like a selected stage tab in AetherRX. Band buttons take focus from Tab only, so a band clicked off doesn't keep the focus border, which matches the checked one;
  • the message field's right-click menu gets a raised panel. It is a child of the field, so the sheet's transparent rule would otherwise reach it.

Cleanup. The nine per-widget sheets (label, field, Send, table, separator, check box, two radio buttons, band buttons) are gone. So are the bottom Close button and the separator line above it, which the corner close button replaces as in other canon workspaces.

No CanonMessageBox: the window never shows a QMessageBox.

Docs: docs/style/dialog-patterns.md lists FreeDV Reporter among the canon workspaces that save their geometry, and as the second window, after AetherTX, that hides on close. docs/style/aethersdr-style-guide.md and docs/style/theme-style-guide.md list the window and its canon/freedvReporter scope. The user docs don't describe this window's chrome or the Close button, so they are unchanged.

Constitution principle honored

Principle VIII (Evidence Over Assertion) covers the checks below. The visual direction is RFC #6226's; the selected-band fill is part of what the maintainer is asked to review.

Test plan

  • Local build passes (macOS, Ninja, RelWithDebInfo): AetherSDR and freedv_reporter_message_test.
  • freedv_reporter_message_test passes. Its target now compiles CanonWindow.cpp instead of PersistentDialog.cpp / FramelessWindowTitleBar.cpp, and gains window checks, all socket-free: the window is a CanonWindow with FramelessResizer; a base64 rectangle saved by the old dialog restores, and hiding saves x,y,w,h under the same key; the message field's menu grabs opaque; the minimum width fits the layout and, on the shown window at that minimum, no labelled control is narrower than its minimum size hint; and every band button takes Tab focus only.
  • Mutation check: dropping the QMenu rules fails the opaque-menu check, and dropping the focus policy fails the focus check. Measured before the fix, the minimum width was 780 against a layout minimum of 825. Dropping the line that raises the minimum to the layout's fails both minimum checks: Track, Band and Freq clip (48 px for 58, 56 and 53).
  • canon_window_test passes.
  • CI does not run these checks before merge. freedv_reporter_message_test is not in ci.yml's frozen per-PR ctest -R steps (docs/agents/tests-ci.md), so the green checks prove it compiles; it runs post-merge in full-suite.yml. The local runs above are the evidence.
  • Full GUI with no radio connected, opened via View → FreeDV Reporter… and captured with screencapture -l (the automation bridge was unavailable). It draws the canon header and corner ✕, the canon header row, muted "Message (reporting off)" with Send dimmed, canon indicators, and "All" selected in aqua. The table scrolls horizontally at this width, as it did before. Offscreen grabs of the fixed build show the rounded header corner and the opaque field menu.
  • The static checks pass: check_engine_boundary, gen_touchpoint_manifest --check, check_capability_records, check_command_plane, check_a11y.py and gen_reference.py --check. The colour-audit ratchet is unchanged against main (487 / 1879 / 599). embed_screenshots.py --check reports your-first-session.md out of step; this PR does not touch that file.
  • Not exercised: live reporter traffic (reporting off, empty table), so the TX/RX/message row highlights on the canon surface are unseen; double-click to tune; restoring geometry across a real relaunch (the test covers it within one process); and Windows/Linux.

Checklist

  • Commits are signed (9100df8b, ED25519)
  • No new flat-key AppSettings calls; the existing geometry key is reused
  • Code is clean-room
  • No meter UI touched
  • Documentation updated: docs/style/dialog-patterns.md, aethersdr-style-guide.md, theme-style-guide.md; no CHANGELOG.md entry. ThemeManager::checkBoxIndicatorStyle()'s comment now names the one dialog that still uses it (MidiMappingDialog).
  • Security-sensitive changes: n/a

🤖 Generated with Claude Code

@w5jwp
w5jwp requested a review from a team as a code owner October 24, 2026 21:14
@w5jwp

w5jwp commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

After:

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.

1. Issue fit

Yes — every bullet of #6755 maps to a hunk, and I verified each against the head checkout rather than taking the body's word:

#6755 asks Verified
CanonWindow + Kind::Workspace FreeDvReporterDialog.h:28, .cpp:263
Resize from any edge FramelessResizer::install(this) at .cpp:273 — required, since CanonWindow only moves
Keep FreeDvReporterGeometry .cpp:271; CanonWindow::restoreGeometryFromSettings() falls back to restoreGeometry(QByteArray::fromBase64(...)) when the value isn't four comma-separated ints, then saveGeometryToSettings() rewrites x,y,w,h. Carry-over claim holds.
Closing hides it Holds, and for a load-bearing reason: CanonWindow sets WA_DeleteOnClose nowhere, and Esc / ⌘W go through QDialog::reject() → hide (CanonWindow.cpp:129). MainWindow holds a raw, non-QPointer m_freedvReporterDialog (MainWindow.h:1343), so a WA_DeleteOnClose here would have been a use-after-free on the next showFreeDvReporter(). It isn't present.
One canon sheet for nine .cpp:117-249
Bottom Close + separator gone Confirmed in the diff
No CanonMessageBox No QMessageBox on the path

The body's "showFreeDvReporter() already created it directly … so MainWindow is unchanged" is true: MainWindow_DigitalModes.cpp:1239-1241 is a direct show()/raise()/activateWindow(), never showOrRaisePersistent(), so unlike the sibling PRs in this batch this one owes no call-site conversion and loses no View → Frameless Window behaviour it had.

2. Scope

Everything in the diff is explained by #6755. No out-of-scope files; git diff --name-only 1afefafdb..HEAD is exactly seven.

File Changes Claimed? Verdict
src/gui/FreeDvReporterDialog.{h,cpp} base class, canon sheet, nine per-widget sheets + Close + separator removed Yes In scope
docs/style/dialog-patterns.md four canon lists + the hide-on-close bullet Yes In scope — see the decision item below
docs/style/aethersdr-style-guide.md, theme-style-guide.md window + canon/freedvReporter scope Yes In scope
tests/freedv_reporter_message_test.cpp 4 new cases Yes In scope
tests/tests.cmake PersistentDialog.cpp/FramelessWindowTitleBar.cpp → CanonWindow.cpp + ${THEME_TEST_RESOURCES} Yes In scope; link closure matches flex_control_dialog_size_test, Qt6::Test was already linked

No new settings key, no protocol surface, no base-token value changed. CHANGELOG.md correctly untouched.

3. Blockers

None. The implementation is clean; the one thing holding it is a ruling, not a defect.

4. Needs maintainer decision — this is an unlisted CanonWindow pick, and the docs edit asserts the pick already happened

Not a code problem, and not a governance violation — but the body doesn't surface it, and its sibling does.

#6239's own text: "Items under 'Candidates' are not approved scope until the maintainer picks them." FreeDV Reporter is on no list there — not the picked "tool windows on CanonWindow" set, and not the "short-lived windows that could move to CanonWindow" candidates. The only bullet that covers it is "Persistent tool dialogs (one PR per dialog)", which sits under "surfaces whose widgets move from base tokens to color.canon.*" — a re-skin that keeps PersistentDialog chrome. So "Part of #6239" in the body shouldn't be read as authorisation for a full Kind::Workspace move. docs/style/dialog-patterns.md:275-277 is the sentence the move contradicts, and this PR edits around it while leaving it standing:

and workspace dialogs keep the persistent pattern above.

Two things make it worth naming rather than waving through:

  1. The docs edits make merging the ruling. aethersdr-style-guide.md:118 and dialog-patterns.md:238 now list FreeDV Reporter among "the tool windows the maintainer has moved to it" — true only once this lands. Sibling #6753 (same author, same day, same shape) says so out loud: "Waiting on the maintainer's pick … please don't merge before the pick. Merging is that pick." #6757 carries no such line despite being in the same position.
  2. The body's one appeal to a ruling isn't on the record. "The maintainer reviewed the running window and kept the selected-band fill as drawn" — I checked the comments on both #6755 and #6757; the only non-bot comments are the author's own screenshots, and #6755 is still open and labelled maintainer-review. The triage bot on #6755 reached the same conclusion independently ("this is an unlisted pick, not a reading of #6239"). If that review happened out of band, fine — but it isn't citable, and it's the only thing in the body resembling the pick.

Counterweight, in fairness: SpotHub (#6401), Radio Setup, Network Diagnostics, Waveforms and Updates all appear in these same doc lists without being on #6239's picked list, so unlisted moves of exactly this class have landed before. The precedent is real; the call is @ten9876's. Per the triage bot, this arrives as one of a wave (#6735, #6753, #6757 workspace; #6745, #6749, #6751 dialog) that is cheaper to rule once as a batch than per surface. Concrete next step: add #6753's "don't merge before the pick" caveat to the body, or get the batch ruled.

5. Nits (all non-blocking)

  • src/gui/FreeDvReporterDialog.cpp:172 — the deleted Send-button sheet carried a comment naming a symptom and a prior ruling, which §3 says to quote back:

    // Widget×state tokens rather than generic chrome (docs/a11y.md): dimming the text alone left the disabled button reading as absent rather than greyed-out, which undercuts the row's own gating (#4231 review).

    The replacement is the opposite shape — fill removed entirely:

    QPushButton:disabled {
        color: {{color.canon.muted}};
        border-color: {{color.canon.line}};
        background: transparent;
    }
    

    I checked before calling it a regression, and it is byte-identical to SpotHub's (DxClusterDialog.cpp), i.e. the ratified canon idiom, and the gating still reads: resolving both bundled themes, canon.muted on canon.ground is 6.71:1 dark / 5.94:1 light, well clear of docs/a11y.md's 4.5:1. So the #4231 concern is answered differently rather than reintroduced. One residue worth a line: the canon.line border is 4.03:1 against ground in dark but 1.55:1 in light, below the 3:1 non-text floor — so in the light theme the disabled Send is effectively text-only. That's a pre-existing property of the canon palette shared by every canon window, not something this PR introduces; it's only worth noting because this PR deletes the comment that guarded it.

  • tests/freedv_reporter_message_test.cpp:285-289 — the comment block is attached to the wrong function. Its first sentence describes the focus behaviour, which is testBandButtonsTakeTabFocusOnly() at :299; only the second belongs to testMinimumFitsBandRow(). Move the first two lines down one function.

  • tests/freedv_reporter_message_test.cpp:289 — testMinimumFitsBandRow() asserts minimumWidth() >= minimumSizeHint().width() on a never-shown window, which is the expression .cpp:280 just assigned, so it can only catch deletion of that line. Clipping is a post-show property; re-checking after show() (as the two cases above it do) would make it catch a polish-ordering regression too. The disclosed 780-vs-825 mutation measurement is the real evidence here, and the assertion is at least relational rather than a hard pixel figure, so it won't drift with platform fonts.

  • Body/evidence — the sibling PRs each disclose "CI does not run this test before merge." This one doesn't, and it's true: freedv appears nowhere in .github/workflows/ci.yml, so the 5/5 green on 24e0344 proves the new test compiles, not that its checks ran; they run post-merge in full-suite.yml. Worth stating, since Principle VIII is the stated frame.

  • src/core/ThemeManager.cpp:1544 — checkBoxIndicatorStyle()'s comment still lists FreeDvReporterDialog as a user ("RadioSetupDialog, DxClusterDialog, FreeDvReporterDialog"); after this PR it isn't one. One-word docs fix, out of scope to require.

  • Undisclosed (welcome) behaviour change — m_msgSendBtn has setAutoDefault(false) (.cpp:325) but the band buttons never did, so under PersistentDialog Return in the message field could also click the first band button. Kind::Workspace neuters default buttons, and the explicit connect(m_msgEdit, &QLineEdit::returnPressed, …) at .cpp:333 still fires, so Return now sends and nothing else. Worth a line in the body.

6. What I tried to break (and couldn't)

All findings are reasoned from code; I have no build, no test run and no way to drive the GUI, so nothing below was reproduced at runtime.

  • The lifetime trap. The most serious bug available here was a WA_DeleteOnClose canon window behind MainWindow's raw m_freedvReporterDialog pointer. Read CanonWindow's constructor, close-button connect, the QKeySequence::Close shortcut and hideEvent: no WA_DeleteOnClose anywhere, reject() hides, WA_QuitOnClose is cleared for Workspace. The hide-on-close claim and the raw pointer are consistent.
  • The sibling call site the diff doesn't show. Grepped every reference to FreeDvReporterDialog outside its own files: MainWindow_DigitalModes.cpp (direct show()), MainWindow.h, DxClusterDialog.cpp (uses only the MessageMaxLength constant). Nothing passed it to showOrRaisePersistent()/trackPersistentDialog(PersistentDialog*), so the base-class swap breaks no call site — the body's "MainWindow is unchanged" survives.
  • Token typos in a 130-line sheet. Extracted all 12 {{…}} tokens and resolved each against the flattened nested JSON of both bundled themes (a plain grep misses them): all 12 present. color.control.unavailable is the one non-canon.* token, and RadioSetupDialog.cpp:863,907 already uses it inside a canon window, so it's established, not a slip.
  • The unexercised row highlights. The body flags that TX/RX/message highlights on the canon surface are unseen, and FreeDvReporterModel.cpp:107-122 paints BackgroundRole/ForegroundRole through the non-widget-aware ThemeManager::instance().color(), i.e. root scope — which is what the theme-style-guide.md line this PR edits warns against. But neither theme's canon scope overrides color.highlight.* (tx #fc4500, rx #379baf, message #e58be5, fg #000000), so root and canon resolve identically and the highlights carry no colour regression. Unhighlighted rows fall through to the sheet's canon.ink on canon.nested.
  • The QWidget { background: transparent } cascade reaching the table viewport, and the absence of a QToolTip rule for the Band radio's tooltip. Both are identical to SpotHub's shipped sheet, so established behaviour rather than introduced.
  • An offscreen-screen trap in the new geometry test. testCanonToolWindow saves a legacy base64 rect of minimumSize() + (0,60); QOffscreenScreen is 800×600 and restoreGeometry() fits an oversized rect to the screen, which would defeat the assertion given the disclosed 825 px layout minimum. It survives because the width chosen is the minimum, so any clamp is undone by the window's own minimum — deliberate, and the inline comment says as much.
  • CodeGuard's 85 findings. None of the three flagged files (PanadapterStream.{cpp,h}, RadioModel.cpp) is in this diff — git diff --name-only returns seven files, none of them those. A read-only audit of every flagged line found no shell/command/network sink (no QProcess, system(), popen, QNetworkAccessManager in either file) and no literal credential: CS-VLN-CMDI-BACKTICK fires on backticks inside C++ comments quoting protocol verbs (// `stream remove 0x<id>`), CMDI-PIPE/AMP on ||/&&/bit-masks, SSRF-HEX on hex sentinels like 0xFFFFFFFF, SEC-PASSWORD on a runtime keychain read (IcomCredentials::sessionPassword()). Nothing attributable to this PR. Credit: security-audit (CodeGuard).
  • Couldn't check: anything visual (the canon sheet's rendered result, the opaque-menu grab, the 825 px measurement) and the docs claim that FreeDV Reporter is only the second canon window to hide on close — I verified it does hide, not that no third one already did.

No socket tests added or removed; the test is socket-free and never reaches qso.freedv.org.

7. Recommendation

Needs maintainer decision. The code is approve-with-nits: the migration is faithful to #6755, the mechanics a CanonWindow move owes are all present (FramelessResizer, geometry carry-over under the old key, no call-site conversion needed here), and the one lifetime hazard this pattern invites is absent. What it needs first is the pick — FreeDV Reporter is on none of #6239's lists, the only bullet covering it is the token-only re-skin, and the docs hunks assert a maintainer decision that isn't on the record in either thread.

Concretely: either add #6753's "don't merge before the pick" caveat to the body, or have @ten9876 rule the #6735/#6753/#6757 batch shape once — at which point this merges on the nits alone. Thanks for a clean, well-evidenced migration, and for keeping the geometry key and the object names stable.


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

@aethersdr-agent

Copy link
Copy Markdown
Contributor

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

  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/core/backends/flex/PanadapterStream.cpp /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.cpp:242
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/flex/PanadapterStream.cpp /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.cpp:907
  • [CRITICAL] CS-VLN-CMDI-PIPE — Vulnerability: CS-VLN-CMDI-PIPE in src/core/backends/flex/PanadapterStream.cpp /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.cpp:1574
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:280
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:300
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:145
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:153
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:155
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:164
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:202
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/core/backends/flex/PanadapterStream.h /tmp/aetherclaude/pr-6757/src/core/backends/flex/PanadapterStream.h:376
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:77
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:78
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:1679
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:3017
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:5533
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:5534
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:6841
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:6847
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:6883
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:7122
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:9840
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:10233
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:10743
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:11339
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:11340
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:11654
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:11662
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:11827
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:12014
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:12021
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:12099
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:13230
  • [MEDIUM] CS-VLN-SSRF-HEX — Vulnerability: CS-VLN-SSRF-HEX in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:13296
  • [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:10748
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:83
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:449
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:706
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:847
  • [CRITICAL] CS-VLN-CMDI-BACKTICK — Vulnerability: CS-VLN-CMDI-BACKTICK in src/models/RadioModel.cpp /tmp/aetherclaude/pr-6757/src/models/RadioModel.cpp:891

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.2814 · model: claude-opus-5

@w5jwp

w5jwp commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Addressed in 94105bc7 and the body:

  • "The maintainer reviewed the running window and kept the selected-band fill as drawn": removed. It had no source; no such review happened on record or off. The body now lists the selected-band fill as part of what's up for the maintainer's review.
  • Misplaced comment: the focus comment moved back above testBandButtonsTakeTabFocusOnly().
  • testMinimumFitsBandRow(): it now also shows the window at its minimum size and checks that no labelled control is narrower than its minimum size hint. Deleting the line that raises the minimum fails both checks: Track, Band and Freq clip (48 px for 58, 56 and 53).
  • Disabled Send: the rule regains a comment naming why the muted ink matters (Allow custom status message configuration in FreeDV Reporter panel #4231). The body gives its contrast (6.71 / 5.94) and discloses the light-theme canon.line border at 1.55:1 as a shared canon-palette property.
  • CI disclosure: added. The test isn't in the frozen per-PR gate.
  • The Return fix it made without saying so: added to the body. Return in the message field now only sends; under PersistentDialog, an auto-default band button could fire too.
  • checkBoxIndicatorStyle() comment: fixed. It names MidiMappingDialog, the one remaining user. Radio Setup and SpotHub had also stopped using it.

w5jwp and others added 2 commits October 24, 2026 22:07
…herRX and AetherTX. Principle VIII.

FreeDvReporterDialog moves from PersistentDialog to a CanonWindow
workspace (RFC aethersdr#6226, part of aethersdr#6239): the canon ground, header and corner
close button, a FramelessResizer, and the FreeDvReporterGeometry key, whose
base64 rectangle CanonWindow reads once and replaces with x,y,w,h. Closing
hides it, as with AetherTX, since MainWindow keeps feeding it stations.

One canon sheet under canon/freedvReporter replaces the per-widget sheets:
a nested station list with a cyan header row rounded to the table border,
the message field on control, canon indicators, and a selected band in aqua
on nested. The message field's right-click menu is a child of the field, so
QMenu gets its panel back. Band buttons take Tab focus only, so a band
clicked off does not keep the focus border that matches the checked one. The
minimum width is the larger of 780 and the layout's own, since an explicit
minimum overrides the layout and the semibold band row clipped Track and
Band. The bottom Close button and its separator go; the corner close
replaces them.

freedv_reporter_message_test gains socket-free checks for the canon
window, the resizer, legacy geometry restore and save, the opaque field
menu, the minimum width and the band focus policy. The style docs list the
window and its scope.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dow, and the disabled Send rule says why it stays legible. Principle VIII.

testMinimumFitsBandRow also shows the window at its minimum size and checks
no labelled control is narrower than its minimum size hint, so clipping
after the canon sheet polishes is caught, not only deletion of the line that
sets the minimum: without it, Track, Band and Freq clip (48 px for 58, 56
and 53). The comment that belonged to testBandButtonsTakeTabFocusOnly moves
back above it.

The disabled Send rule regains a comment naming why its muted ink matters:
Send must read as greyed-out, not absent, since that is how the row explains
its own gating (aethersdr#4231). checkBoxIndicatorStyle()'s comment names the one
dialog that still uses it.

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.

FreeDV Reporter: move to a CanonWindow workspace

1 participant