[ten9876]

Part of #6214 (settings and logging; the widgets half is left open, see the end).

Root cause

The light tests deliberately avoid linking aethercore, so they compile the app sources they need themselves. Three of those sources were listed in dozens of test targets:

Source	Test targets compiling it
${AETHER_SETTINGS_SOURCES} (5 files: AppSettings, SettingsDatabase, SettingsPaths, SettingsBootstrap, SettingsSanitizer)	75
src/core/AsyncLogWriter.cpp	65
src/core/LogManager.cpp	63

CMake compiles a listed source once per target, and each test's own flags and include dirs differ, so ccache could not merge the copies. That came to about 500 compiles of the same seven files on every full-tree build.

Fix

• Compile once: each source is now an OBJECT library compiled once: aether_test_settings, aether_test_log_manager and aether_test_async_log_writer (tests/tests.cmake, "Shared objects" at the top).
• One-for-one swap: every use became a matching $<TARGET_OBJECTS:...> line, so each test links exactly the objects it linked before. Nothing else in the 77 blocks changed.
• OBJECT, not STATIC: this keeps the same semantics as listing the source. Every object is linked, so nothing that matters only through a static initializer can be dropped. It's the same reasoning as the existing aether_test_wisdom_isolation.
• Light tests only: a test linking aethercore/aetherdesktop_support must not take these. Next to libaethercore.so (AETHER_SHARED_CORE) that would be a second AppSettings singleton. None of the 77 does; I checked every target_link_libraries in the file.
• Guard: a configure check fails if a test compiles one of these from source again. Otherwise a block copied from an older test would quietly bring the recompiles back.
• Docs: tests/README.md and the CMake-contract line in .claude/skills/pr-review/SKILL.md describe the new form. That skill file is also touched by docs(skills): lean, targeted builds in /pr-review and /pr-land #6215, in a different hunk; whichever lands second may need a trivial rebase.

Evidence

Linux, GCC, Qt 6.12.0, static core. main at 03dcff47c and this branch were configured identically in sibling worktrees. Compile-job counts come from ninja -t commands, and timings are clean builds with CCACHE_DISABLE=1.

Primary measurement, at CI's build flags: RelWithDebInfo with -O2 -g1 -DNDEBUG (ci.yml:263-271):

	main	this PR
Compile jobs, full tree	3806	3322 (−484)
Compile jobs, the 77 affected targets	1015	524 (−48%)
Clean uncached build of those 77 targets	151 s	89 s (−41%)
Build dir after that build	0.86 GB	0.72 GB

At plain RelWithDebInfo (full -g) the job counts are the same; the build went from 192 s to 116 s (−40%), and the build dir from 3.05 to 2.34 GB.

• Tests: all 70 tests registered from the 77 affected targets pass on both main and this branch at full -g, and on this branch again at -g1 (ctest -R … --no-tests=error). The other 7 targets are tools with no registered test.
• Guard mutation: I swapped one $<TARGET_OBJECTS:aether_test_log_manager> back to src/core/LogManager.cpp. Configure failed at tests/tests.cmake:106 with the guard's message. After restoring the file (sha256 identical), configure passed.
• Checks: check_test_registration.py --strict, check_ci_test_gate.py and check_build_options.py pass.
• Sanitizer configure: configure passes with -DAETHERSDR_SANITIZER=address,undefined -DAETHER_SHARED_CORE=ON.

Not verified locally; CI covers it:

• The Windows and macOS compile of the OBJECT form. AUTOMOC on LogManager.h now runs once in the object library instead of per test.
• The sanitizer lane running these targets. I only configured that lane.

The agent automation bridge doesn't apply here: this is a build change with no runtime behaviour to drive.

Also: RelWithDebInfo defaults to -g1 on GCC/Clang (CMakeLists.txt, after the build-type default)

RelWithDebInfo is the default build type, and CMake's -g gives every binary full DWARF. Measured on main:

• the AetherSDR binary is 832 MB, about 680 MB of it debug info;
• 232 test binaries are over 100 MB each;
• the clean build of the 77 affected targets is 3.05 GB at -g and 0.86 GB at -g1.

-g1 keeps line tables and function names, so backtraces and core dumps still symbolize; locals and types need a Debug build. CI's Linux build leg and full-suite.yml already pass -g1 explicitly. I left those lines alone; they now restate the default.

Scope of the change:

• What it replaces: only CMake's untouched default (-O2 -g -DNDEBUG) for GCC/Clang (AppleClang included), and only when CFLAGS/CXXFLAGS set no debug level. The TSan lane injects -g3 through CXXFLAGS, and RelWithDebInfo's flags come after it on the command line, so a blanket -g1 would have overridden it.
• What still wins: an explicit -DCMAKE_<LANG>_FLAGS_RELWITHDEBINFO.
• Existing build trees: a tree configured before this change cached CMake's default -O2 -g -DNDEBUG. This change moves it to -g1 on the next reconfigure, so it rebuilds once after pulling, mostly missing ccache. That is expected, not a regression.
• Whitespace: the debug-level check accepts a space or a tab before -g ((^|[ \t])-g), so CXXFLAGS="-Wall<TAB>-g3" is left alone too.
• Reconfiguring with a debug level: the value is cached, so an internal _AETHER_<LANG>_RWD_G1 records that the project wrote it. If a later reconfigure adds a -g to CMAKE_<LANG>_FLAGS, CMake's -O2 -g -DNDEBUG comes back instead of the cached -g1 overriding it; drop the -g and -g1 returns. A RelWithDebInfo value anyone else set is never touched.
• What it never touches:
  • MSVC flags: the Windows installer's RelWithDebInfo is /Zi, which never matches.
  • Release, which the Linux AppImage and macOS DMG builds use.
  • One local path does change: packaging/macos/build-installer.sh (the manual .pkg script, which no workflow runs) configures RelWithDebInfo, so it now builds at -g1 under AppleClang.
• BUILD-OPTIONS.md gets a "Build type and debug info" section on getting full debug info. Spell it -g2, because a value identical to CMake's default can't be told apart from no choice.

Configure-only checks, each in a fresh build dir:

Case	CMAKE_CXX_FLAGS_RELWITHDEBINFO
Fresh, no options	-O2 -g1 -DNDEBUG, and the compile line for AppSettings.cpp carries -O2 -g1
No build type given	-O2 -g1 -DNDEBUG
Reconfigure of that tree	stays -O2 -g1 -DNDEBUG
Same tree reconfigured with -DCMAKE_CXX_FLAGS=-g3	-O2 -g -DNDEBUG (CMake's default restored; compile line -g3 -g, as before this PR)
…then with the flags cleared	-O2 -g1 -DNDEBUG again
Explicit RelWithDebInfo value (-g1 or -g2), then -g3	unchanged: the value is the user's
CFLAGS=-g3 CXXFLAGS=-g3 (TSan shape)	-O2 -g -DNDEBUG (untouched)
Explicit -O2 -g2 -DNDEBUG	-O2 -g2 -DNDEBUG (wins)
CFLAGS/CXXFLAGS = -Wall<TAB>-g3	-O2 -g -DNDEBUG (untouched)
main-configured tree, reconfigured with this branch	-O2 -g -DNDEBUG → -O2 -g1 -DNDEBUG

check_build_options.py --strict and its self-test pass. FORCE cache sets are outside its catalog, which is why the doc section is prose rather than a table row.

Not in this PR

#6214 also proposed a widgets library (DragValuePopup ×40, FramelessResizer ×29, FramelessWindowTitleBar ×23, PersistentDialog ×19). Those tests link differing mixes of aetherdesktop_support and Qt modules, so they need the per-target audit this PR did for settings and logging. I left them for a follow-up rather than guessing.

🤖 Generated with Claude Code

[ten9876]

Reviewed the updated head 7154f88 (including the new debug-default commit), compared with merge base 173c0d4. Recommendation: Approve with one nit, pending required build/platform checks. No blockers.

Issue fit and scope

#6214 asks to reduce repeated compilation without pulling the engine into light tests, changing settings isolation, or changing test/CI membership. The settings/logging portion meets that request; the widgets portion remains expressly open. The added debug-default change addresses the same binary-size problem and is now disclosed in both title and body. These are build/performance changes with no application UX or new dependency; GOVERNANCE.md does not require an RFC for them.

Scope	Change	Claimed?	Verdict
tests/tests.cmake	Three OBJECT libraries, matching source substitutions and configure guard	Yes	In scope
tests/README.md, .claude/skills/pr-review/SKILL.md	Document the new standalone settings/logging contract	Yes	In scope
CMakeLists.txt, BUILD-OPTIONS.md	Default untouched GNU/Clang RelWithDebInfo flags to g1; document overrides	Yes, updated body	In scope

No application source, CI filter, test registration, release file, or CHANGELOG change is bundled. Both commits are verified signed. The debug-default tradeoff is explicit: locals/types require Debug or an explicit g2 override.

Nit — CMakeLists.txt:285: recognize whitespace before a debug flag.

The condition promises to retain an existing debug-level choice in CFLAGS/CXXFLAGS, but (^| )-g recognizes only a literal space. In a fresh actual configure with CFLAGS and CXXFLAGS containing -Wall<TAB>-g3, CMake retains those flags but this block changes RelWithDebInfo to -O2 -g1 -DNDEBUG. The later g1 therefore overrides the requested higher level. Ordinary space-separated flags work; this is an edge-case override nit, not a blocker. Drop-in replacement:

       AND NOT CMAKE_${_lang}_FLAGS MATCHES "(^|[ \t])-g")

I exercised that replacement in an isolated CMake script using the actual block; it preserves the default in the tab-separated case. It also preserves the existing behavior for default flags, explicit g2, and ordinary g3 input. The original untouched -g is itself later than global -g3; this nit only addresses the new block's stated preservation condition, not an assertion that this PR introduced that older ordering issue.

What I tried to break / evidence

• Configured separate head and base trees with GCC/Qt 6.12, ASR/RADE disabled. Compile databases drop from 3603 to 3112 entries (491 fewer) in this configuration. All 77 old standalone consumers remain executable targets. Inspected their generated link edges: none gains aethercore or aetherdesktop_support. Tested the potential lost-definition trap: none of their old AppSettings compile commands had HAVE_KEYCHAIN, and the remaining target-specific definitions do not control these shared source implementations.
• Compared configured CTest names: 644 on each side, identical lists. The selected settings, async writer, logging filter and widget targets built on both sides. On each side the same 32 CTest entries produce 31 passes and one platform skip (explicit-profile-path-isolation). The initial widget startup failure was identical on base and head: the inherited GNOME/GTK plugin attempted desktop access in the sandbox. With QT_QPA_PLATFORMTHEME=generic QT_STYLE_OVERRIDE=Fusion, the widget test passes on both sides. No source modification was needed.
• Mutation-tested the new guard by replacing one logging object reference with src/core/LogManager.cpp: configure failed at tests/tests.cmake:106 with the intended message. Restored exact bytes; configure succeeds and the scratch source diff is clean.
• Fresh real configurations verify ordinary CFLAGS/CXXFLAGS g3 leaves RelWithDebInfo unchanged and explicit g2 wins; fresh default/reconfigure use g1. readelf --debug-dump=decodedline finds source line tables in the resulting AppSettings object. Registration, frozen CI gate, build-options strict checks and the build-options self-test pass.
• The latest-head static checks and sanitizer configure probe pass. Linux CI build passes; macOS and Windows checks are still pending. Latest runs were created at 21:51:27 UTC, after main 173c0d485 at 21:22:46 UTC, so they are not stale by that comparison. Earlier greens belonged to the superseded head and are not approval evidence for the debug-default commit.

Test boundary and honest limits

Before building, I inspected the changed targets' test sources. Four existing default-suite synthetic-peer tests are affected by source substitution: vkamp_connection_test binds TCP and UDP; pgxl_direct_protocol_test, pgxl_panel_test, and tgxl_direct_protocol_test listen on loopback TCP. I notified the operator and did not build or run those targets, as the review skill requires PR-specific direction. This PR adds no new socket behavior or peer; those existing boundary debts remain. They run in the default main/full-suite and weekly sanitizer graphs, not the frozen per-PR test gate.

I did not build the whole tree, reproduce the author's uncached wall-clock/size benchmarks, run all affected tests, execute sanitizer binaries, or locally verify Windows/macOS. The graph/target audit and focused behavioral tests support the shared-object change; they do not replace those omitted checks. No application GUI/bridge or radio session is needed for this build change. An invocable automated code-review skill was unavailable; this was a manual adversarial review.

Posted as COMMENT because the authenticated reviewer is also the author; GitHub does not permit self-approval.

[ten9876]

Reviewed 7154f88 as a pair with #6215 (head b2aafd9), against merge base 173c0d485. Recommendation: approve with nits. No blockers. I'm posting this as COMMENT because the reviewing account is the author.

Issue fit: #6214 asks to stop recompiling the most-repeated test sources. This PR does that for the settings store, LogManager and AsyncLogWriter, and leaves the widgets half explicitly open, so it delivers part of the issue as stated. The -g1 default is a separate change. #6214 does not ask for it, but the title and body disclose it, and it is aimed at the same binary-size problem. Whether it belongs in this PR is a maintainer call, and the maintainer is the author.

Scope

File	Change	Claimed?	Verdict
tests/tests.cmake	3 OBJECT libraries, 1:1 $<TARGET_OBJECTS> swaps in 77 blocks, configure guard	Yes	In scope (#6214)
tests/README.md, .claude/skills/pr-review/SKILL.md:324-326	Describe the new contract	Yes	In scope
CMakeLists.txt:274-289, BUILD-OPTIONS.md	RelWithDebInfo defaults to -g1 on GCC/Clang	Yes (title and body)	Separate change, disclosed; maintainer call

Findings (all non-blocking)

1. Pair: this PR makes #6215's test-selection grep miss every logging consumer. On main, grep 'LogManager.cpp' tests/tests.cmake hits 63 test blocks and AsyncLogWriter.cpp hits 65. With this PR, each grep returns only the one add_library(... OBJECT ...) line. #6215 tells reviewers to choose tests with exactly that grep. The fix belongs in #6215's text; the drop-in is in my review there. I'm noting it here so whichever PR lands second carries it.

2. CMakeLists.txt:274-289: every existing RelWithDebInfo tree recompiles once after this merges. The body describes this case only as "Reconfigure of that tree stays -g1". I reproduced the main-to-PR case: I configured a fresh tree with main's CMakeLists.txt, which cached -O2 -g -DNDEBUG, then reconfigured the same tree with this PR. The cache became -O2 -g1 -DNDEBUG. Every object is rebuilt and ccache misses on all of them. That is intended and correct, but worth one line in the body so nobody mistakes the long build after pulling main for a regression.

3. packaging/macos/build-installer.sh:15 is a third RelWithDebInfo path, and it now gets -g1. The body says the change never touches "Release, which the Linux AppImage and macOS DMG builds use". That is true; macos-dmg.yml uses Release. The local .pkg script, however, configures RelWithDebInfo under AppleClang and now builds at -g1. codeql.yml:32 says no workflow runs it, so this only needs a disclosure line.

4. Still open from the earlier review: CMakeLists.txt:285, (^| )-g misses a tab-separated -g3. Use (^|[ \t])-g.

5. Optional: the guard catches source re-listing, but nothing catches a test that links both these object libraries and aethercore. That combination is the second-singleton case the comment at tests/tests.cmake:76-80 warns about. Today the count is 0 (verified below). Whether it is worth a get_target_property(LINK_LIBRARIES) sweep in the deferred-retrofit section is your call.

What I tried to break

• -g1 default, configure-only checks run myself in fresh trees:
  • No flags gives -O2 -g1 -DNDEBUG.
  • CFLAGS=-g3 CXXFLAGS=-g3 gives -O2 -g -DNDEBUG (untouched).
  • A main-configured tree reconfigured with this PR gives -g1 (finding 2).
• Guard mutation, run myself: I swapped one $<TARGET_OBJECTS:aether_test_log_manager> back to src/core/LogManager.cpp. Configure failed with CMake Error at tests/tests.cmake:106 and the guard's message. After git checkout -- tests/tests.cmake, the tree was clean and configure passed again.
• Link edges, from the merged tree's build.ninja: 77 executables take the object libraries, and 0 of them also link libaethercore/libaetherdesktop_support. No aether_test_log_manager consumer is missing aether_test_settings.
• Behaviour, with #6215's lean recipe on main + both PRs (AETHER_SHARED_CORE=ON): I built split_qsy_settings_test (settings objects plus target-specific definitions), log_manager_filter_rules_test, async_log_writer_test, theme_manager_test, transmit_model_test and sim_backend_test. 6/6 passed with --no-tests=error.
• Merge: the pair merges cleanly in either order onto current main and gives an identical tree. The pr-review/SKILL.md hunks don't overlap.
• Test boundary: the TARGET_OBJECTS swaps also land in four existing socket-owning tests: vkamp_connection_test, pgxl_direct_protocol_test, pgxl_panel_test and tgxl_direct_protocol_test. This PR adds no socket behaviour. I did not build or run those four, and I flagged them to the operator.
• Not reproduced: the author's 151 s → 89 s and 3806 → 3322 job figures, the Windows/macOS compile (CI on this head is green, created after main's tip, so not stale), and sanitizer execution.
• Bridge: not applicable. This is a build change with no runtime surface.

[aethersdr-agent]

Issue fit

Partially — and deliberately so, which the body states up front. #6214 asked for the repeated app sources in tests/tests.cmake to be factored into shared libraries, listing eight sources. This PR does the settings store, LogManager and AsyncLogWriter, and explicitly leaves the widgets half (DragValuePopup ×40, FramelessResizer ×29, FramelessWindowTitleBar ×23, PersistentDialog ×19) open because those targets link differing mixes of aetherdesktop_support. That is a reasonable split and it is disclosed.

The constraints #6214 asked to keep all hold, and I checked each of them against the head checkout rather than taking the body's word:

• "Light tests stay light. Neither library pulls in aethercore." — aether_test_settings links Qt6::Core + aether_sqlite3, the two log libraries Qt6::Core only (tests/tests.cmake:84-94). Confirmed.
• "The AETHER_SETTINGS_SOURCES / AETHER_SETTINGS_CONSUMERS contract unchanged." — I parsed every add_executable block and diffed per-target source counts against main at 173c0d48. Every one of the 78 affected targets links exactly the same object set it did before; zero targets changed count. The settings-objects users and the AETHER_SETTINGS_CONSUMERS membership are an exact two-way match — no user missing from the list, no new gap.
• "No change to which tests exist, or to ci.yml filters." — confirmed, no test added, removed or renamed; no workflow touched.

The -g1 half is a second, independent change — see the scope table.

Scope

File / group	What it changes	Claimed by title/body?	Verdict
tests/tests.cmake	three OBJECT libraries + 77 one-for-one $<TARGET_OBJECTS:> swaps + a configure guard	yes, and it is #6214's actual ask	In scope
tests/README.md	documents the new form, incl. the two log libraries	yes	In scope
.claude/skills/pr-review/SKILL.md	rewrites the CMake-contract line to the new rule	yes, with the #6215 rebase conflict disclosed	In scope — the contract doc has to move with the contract
CMakeLists.txt:274-289	RelWithDebInfo now defaults to -g1 for every GCC/Clang build	yes — title, a body section, three config-test tables	Needs maintainer decision (below)
BUILD-OPTIONS.md	new "Build type and debug info" section	yes	In scope, for the -g1 change

No CHANGELOG.md entry. No new public surface, protocol verb, settings key or capability field. No removed guard: the - lines are all source-list entries and two stale comment sentences, and every explanatory comment that named a link-time reason (qrz_callsign_test's "LogManager provides lcQrz…", cwx_speed_modifier_test's #3949 note, mini_pan_widget_test's seed-TU warning) survives intact next to the new lines. No socket test added, modified or removed, and no Fake* peer — I read the added/changed tests.cmake blocks and there are no new test sources in this PR at all.

On the -g1 bundling. #6214's own Context paragraph scopes this out: it records that AETHER_SHARED_CORE=ON with -g1 is already in use by the review skills, and then says "This issue covers the remaining part, the per-target recompiles, which only a CMake change fixes." So no issue asks for the default debug level to change, and this one changes what every contributor and every agent gets from the recipe printed in README.md:214, AGENTS.md:189, docs/BUILDING.md:93/115/158 and docs/first-contribution-cheatsheet.md:302. It is fully disclosed, it is well evidenced, and it lives in two cleanly separable commits (7154f88, 2cf4532) — so this is not "unrelated → unbundle" and not a nit either. It is a maintainer call on whether a build-default change for the whole contributor base lands inside a test-refactor PR. The split would be a trivial cherry-pick either way.

Blockers

None. Nothing here breaks users, violates canon, or will fail on current main.

Nits

All non-blocking.

• tests/tests.cmake:72 — the settings count is off by one, and the body's "77 affected targets" with it. The comment reads (75 tests compile the settings store, 63 LogManager, 65 AsyncLogWriter). The file text gives 76 targets taking aether_test_settings (79 textual hits minus three in comments and the FATAL_ERROR string) and 78 taking at least one of the three (76 settings users, plus async_log_writer_test and issue_report_test, which take only aether_test_async_log_writer). 63/65 tie out exactly. The likely cause is that copy_assist_settings_dialog_test sits inside if (ENABLE_ASR), so a configured-target count on a tree without ASR sees 75/77 while a text count sees 76/78. Harmless, but the two methods are mixed in one parenthesis.

• tests/tests.cmake:95-118 — the guard cannot distinguish "someone brought the recompiles back" from "someone legitimately needs a private compile", and offers no escape hatch. The message is good, but it is a hard FATAL_ERROR on NOT _aether_shared_source_count EQUAL 3 with no documented way out. If a future test genuinely needs AppSettings compiled with its own definitions (control_credential_vault_test and icom_credentials_singleflight_test already do that shape of thing with HAVE_KEYCHAIN), the only path is to edit the guard, and nothing tells the author that is allowed or how to justify it. This file already has the convention for exactly this — the # not registered: <reason> marker. One sentence in the FATAL_ERROR body ("if a test truly needs its own compile, raise the expected count and say why on the line") would match it.

• tests/tests.cmake:100-103 — the guard works only because its own pattern string is double-escaped, and nothing says so. The regex literal in the file is "\\$\\{AETHER_SETTINGS_SOURCES\\}|src/core/LogManager\\.cpp|...", so the raw text it scans contains LogManager\\.cpp, not LogManager.cpp, and the pattern does not match itself. Rewrite it with single backslashes — which CMake also accepts as \$, \{ escapes — and the count becomes 4 and configure fails with a message blaming a test that is fine. It fails loud rather than silent, so this is cosmetic, but the hazard is invisible and worth a half-line comment. (Incidentally, green configure in CI proves all three alternatives match exactly once each, including the ${AETHER_SETTINGS_SOURCES} one your mutation test did not cover — if that alternative had silently never fired the count would be 2 and every configure would abort. Nice property, worth noting in the comment.)

• tests/tests.cmake:76-83 — the invariant that actually matters is the one left in prose. The comment correctly identifies the real hazard: "a second copy of AppSettings next to libaethercore.so (AETHER_SHARED_CORE) would be a second singleton." That is a worse failure than the recompiles the guard does catch, and it is enforced only by the audit you did by hand. I re-ran it independently — I parsed every target_link_libraries for all 78 users and zero of them link aethercore or aetherdesktop_support, so the tree is clean today — but nothing keeps it clean. A configure-time loop is mechanical here, since both facts are target properties:

  # Nothing may take these objects AND link a library that already contains
  # them: under AETHER_SHARED_CORE that is a second AppSettings singleton.
  foreach(_obj_lib aether_test_settings aether_test_log_manager
                   aether_test_async_log_writer)
      foreach(_tgt ${AETHER_SETTINGS_CONSUMERS})
          if(TARGET ${_tgt})
              get_target_property(_srcs ${_tgt} SOURCES)
              get_target_property(_libs ${_tgt} LINK_LIBRARIES)
              if(_srcs MATCHES "TARGET_OBJECTS:${_obj_lib}"
                 AND _libs MATCHES "aethercore|aetherdesktop_support")
                  message(FATAL_ERROR
                      "${_tgt} takes $<TARGET_OBJECTS:${_obj_lib}> and links "
                      "aethercore/aetherdesktop_support, which already contain "
                      "it. Drop the objects and rely on the library.")
              endif()
          endif()
      endforeach()
  endforeach()

  Worth having or worth declining — AETHER_SETTINGS_CONSUMERS only covers the settings users, so the log libraries would need their own list to be fully covered, and that may not be worth the bookkeeping.

• tests/tests.cmake:5587-5589 — the rewritten AETHER_SETTINGS_CONSUMERS header is narrower than the list. It now reads "Every standalone test/tool target that takes $<TARGET_OBJECTS:aether_test_settings> (rather than linking aethercore) needs the vendored SQLite engine." The list has 157 entries; only 76 take the objects. The other ~81 (reroute_dead_controls_test, anan_backend_test, tx_operation_integration_test, …) link aethercore and are in the list anyway. The old wording was equally narrow, so this is pre-existing imprecision the rewrite inherits rather than introduces — but since you were in the sentence, "or links a library that contains it" would make it true. Same sentence in .claude/skills/pr-review/SKILL.md:324-326.

• CMakeLists.txt:274-289 / BUILD-OPTIONS.md:37 — the -g1 default is documented only where a contributor will not look. Every build recipe in the repo (README.md:214, AGENTS.md:189, docs/BUILDING.md:93, docs/first-contribution-cheatsheet.md:302) prints -DCMAKE_BUILD_TYPE=RelWithDebInfo with no hint that locals and types are now gone from the debugger. A one-line pointer to the new BUILD-OPTIONS.md section from docs/BUILDING.md would save someone an afternoon wondering why p m_settings prints nothing.

• CMakeLists.txt:282 — ObjC/ObjCXX are not in the foreach. The loop covers C CXX. project() declares LANGUAGES C CXX and I found no enable_language(OBJCXX), so there is probably nothing to do; but if a .mm source ever gets its own language enabled, CMAKE_OBJCXX_FLAGS_RELWITHDEBINFO keeps full -g and the macOS half of the win quietly stops applying. Not worth changing now — worth a word in the comment so the next person knows it was considered.

What I tried to break

Everything below is reasoned from the head checkout at /tmp/aetherclaude/pr-6217 plus CI metadata. I did not build or run anything.

• "One-for-one swap: nothing else in the 77 blocks changed." Tested, held. I parsed every add_executable block in both main (173c0d48) and the head, computed per-target counts of ${AETHER_SETTINGS_SOURCES}+TARGET_OBJECTS:aether_test_settings and the same for both log sources, and diffed: zero targets differ. Totals are identical on both sides (76 / 63 / 65). Raw source references remaining in the file: exactly one each, the three add_library lines.
• The attack I expected to land: per-target compile definitions and include dirs the object libraries no longer see. An OBJECT library compiles with its own flags, not the consumer's, so any test that was relying on a private macro reaching AppSettings.cpp would silently change behavior. It does not happen here. Of the 78 users, five have target_compile_definitions — AETHER_SOURCE_DIR (×4) and HAVE_SERIALPORT (dstar_model_test) — and none of the seven shared sources reads either; their only non-platform conditional is #ifdef HAVE_KEYCHAIN (AppSettings.cpp:26,542), which is set only as target_compile_definitions(aethercore PUBLIC HAVE_KEYCHAIN) (CMakeLists.txt:2464) and no user links aethercore, so it was undefined before and is undefined now. Include dirs collapse to four sets (src ×55, src tests ×15, src + r8brain ×7, src src/core ×1) and all four contain src; no user uses BEFORE or tests/fakes, so there is no header-shim ordering to lose. Every Qt include across the seven sources is QtCore-only, matching the Qt6::Core-only link.
• -g1 eating the TSan lane's -g3. Checked and it holds, for the reason the body gives. sanitizers.yml:77 injects -fsanitize=thread -g3 … -O1 through both CFLAGS and CXXFLAGS (sanitizers.yml:117-127) on a RelWithDebInfo build, and CMAKE_<LANG>_FLAGS precedes CMAKE_<LANG>_FLAGS_<CONFIG> on the command line — so an unguarded -g1 would indeed have won. The NOT CMAKE_${_lang}_FLAGS MATCHES "(^|[ \t])-g" arm catches " -g3" for both languages. (^ inside the alternation is a real anchor here — CMake's regex engine treats ^ as magic after ( and | — and \t is a genuine tab escape in a CMake quoted argument.) The ASan lane is build_type: Debug, untouched.
• The sanitizer path the body does not mention: -DAETHERSDR_SANITIZER=. That one takes add_compile_options(-fsanitize=… -g3) at CMakeLists.txt:141, which bypasses the CMAKE_<LANG>_FLAGS guard entirely. It still comes out right: directory/target COMPILE_OPTIONS are appended after CMAKE_<LANG>_FLAGS_<CONFIG>, so -g3 wins over -g1 exactly as it won over -g. No finding, but it is a second path worth a clause in the comment.
• The other RelWithDebInfo consumers. ci.yml:269-271 and full-suite.yml:102-104 already pass -O2 -g1 -DNDEBUG explicitly, so the STREQUAL "-O2 -g -DNDEBUG" test does not fire and nothing changes for them. windows-installer.yml:251 is MSVC (/Zi, never matches) and create-msix.ps1:512's -RequirePdb is unaffected. The AppImage and DMG legs are Release. codeql.yml:111 is Release. The only behavior change outside CI is the one the body discloses, packaging/macos/build-installer.sh:15, and I found no dsymutil, objcopy --only-keep-debug or .dSYM step anywhere in .github/workflows/, packaging/ or scripts/ that would be degraded by -g1.
• tools/check_build_options.py skipping the new cache set. Verified rather than assumed: line 124 is elif "CACHE" in args and "FORCE" not in args, so a FORCE set is outside the catalog by construction and needs no BUILD-OPTIONS.md table row. The prose section is the right shape.
• Stale references to the old contract. Grepped the whole tree (*.md, *.py, *.sh, *.yml, *.cmake, CMakeLists.txt) for AETHER_SETTINGS_SOURCES and for src/core/LogManager.cpp / src/core/AsyncLogWriter.cpp: the only remaining uses are the definition at CMakeLists.txt:830, the engine's own use at :921, and the three add_library lines. No doc left behind describing the retired form.
• Ordering and CMake-version hazards. aether_sqlite3 is defined at CMakeLists.txt:1562/1569 and AETHER_SETTINGS_SOURCES at :830, both well before include(tests/tests.cmake) at :3343, so the top-of-file add_library + target_link_libraries resolve. cmake_minimum_required(VERSION 3.25) is far above the 3.12 floor for target_link_libraries on an OBJECT library. CMAKE_AUTOMOC ON is global (:14) and no user sets AUTOMOC OFF, so the moc TU moves into the object library cleanly and comes along in $<TARGET_OBJECTS:> — no target loses or doubles a moc.
• What CI does and does not prove, against your "Not verified locally; CI covers it". All 7 checks are green on 2cf4532. The Linux build leg runs cmake --build build -j$(nproc) on the full tree (ci.yml:223), so all 78 affected targets really did compile and link under GCC — that is strong. But check-windows and check-macos build only --target AetherSDR aether-dv-waveform thumbdv_queue_test dstar_model_test digital_voice_waveform_process_test cross_needle_meter_test (ci.yml:713, :796), of which exactly two (dstar_model_test, digital_voice_waveform_process_test) take the shared objects. So MSVC and AppleClang have exercised 2 of 78 consumers of the OBJECT+AUTOMOC form, not all of them. Green here is narrower than it looks. check-windows also configures Release (ci.yml:603), so it exercises none of the -g1 change — consistent with "MSVC unaffected", but it means no CI leg currently builds the new default at all; every lane passes -g1 explicitly or uses another build type.
• What I could not check. Whether the -g1 tree actually symbolizes backtraces and core dumps as claimed (no build, no debugger); the real wall-clock and disk numbers in the evidence tables; and whether the sanitizer lanes still run green, since only their configure is covered. The compile-job arithmetic is self-consistent with what I counted in the file, which is the part I can check.

Recommendation

Approve with nits on the tests.cmake half; needs a maintainer decision on the -g1 bundling. The refactor is the most carefully executed change of its kind I have reviewed on this repo — the swap is provably one-for-one across 78 targets, the hazard the OBJECT-vs-STATIC choice avoids is correctly identified, the no-aethercore audit reproduces exactly, and the guard against regression is self-verifying. The attack I expected to land (per-target definitions and include dirs that the shared objects no longer see) does not land, and I checked all five definition sites and all four include-dir sets to be sure.

The -g1 default is also correct and well evidenced, but it is a change #6214 explicitly scoped out, it alters what every contributor's documented build produces, and no issue asks for it. Concrete next step: a maintainer rules on whether to keep both halves here or cherry-pick 7154f88 + 2cf4532 into their own PR (trivial — the two commits touch only CMakeLists.txt and BUILD-OPTIONS.md). Either way the nits above are optional polish, not merge conditions. Thanks for the per-target audit and the guard-mutation test — both made this reviewable instead of a matter of trust.

---

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

[aethersdr-agent]

Antares Detector — candidate vulnerable file(s)

• tools/check_build_options.py

The collect_documented function reads BUILD-OPTIONS.md and parses Option/Setting rows by splitting on '|' and '|||'. It does not validate or sanitize the input, which can lead to buffer overruns/use-after-free or unexpected parsing behavior when maliciously crafted markdown is present. This is a format-string/memory-safety vulnerability in the option-parsing code.

_{Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting.}

---

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