Thanks for the thorough writeup and the real-account verification, @wa2n-code — I traced all three claims through the code and they hold up.

The parseXml() fix is correct

I confirmed the root cause. Every QRZ response is wrapped in <QRZDatabase>, which is the first StartElement the loop sees. Since it matches neither "Session" nor "Callsign", the pre-fix code fed it straight to r.readElementText(QXmlStreamReader::SkipChildElements) (QrzClient.cpp:222) — which, per Qt semantics, reads until the root's </QRZDatabase> and discards every child, including the <Session><Key>…</Key></Session> block, before the loop can enter it. block never leaves Block::None, so sessionKey/info come back empty for every response. The block == Block::None guard is the right minimal fix: the root (and any other pre-block start tag) is stepped over via readNext() instead of consumed, so the loop reaches <Session>/<Callsign> intact. Nested elements inside those blocks are still handled by SkipChildElements.

CMake / README changes are sound and in scope

The WIN32 + EXISTS-guarded POST_BUILD copy targets third_party/qtkeychain/bin/qt6keychain.dll, which is exactly where scripts/setup/setup-qtkeychain.ps1 installs it ($OutDir\bin\qt6keychain.dll, script lines 25/104) — so the path is consistent, and the EXISTS guard makes it a safe no-op when the DLL isn't staged. Adding the setup step to the README's Windows flow is a reasonable doc fix. No AppSettings/RAII concerns; nothing outside the stated scope.

One suggestion (non-blocking): lock this in with a regression test

You correctly note this shipped because nothing exercised parseXml() against a root-wrapped response — and it couldn't, since parseXml is private static (QrzClient.h:76) and qrz_callsign_test.cpp never touches it. Since the method is already a pure static with no side effects, promoting it to public (or adding a friend test seam) and asserting that a realistic <QRZDatabase><Session><Key>…</Key></Session></QRZDatabase> payload yields a non-empty sessionKey would prevent a future token-loop refactor from silently reintroducing this. Worth doing given it was a total end-to-end breakage that passing CI didn't catch.

Nice catch on a genuinely broken-since-#3990 path. LGTM once you decide on the test.

_{🤖 aethersdr-agent · cost: $7.8837 · model: claude-opus-4-8}

Thanks for the careful diagnosis here, @wa2n-code — this is a well-tracked root-cause fix, and the writeup made it easy to verify. I traced each change against the source; notes below.

The parseXml() fix is correct ✅

The diagnosis holds up. Every QRZ response is wrapped in a <QRZDatabase> root, which is the first StartElement the loop sees. With block == None it fell through to readElementText(SkipChildElements) on the root — which per Qt reads to the matching </QRZDatabase>, consuming the nested <Session>/<Callsign> blocks before the loop could inspect them. So sessionKey came back empty for every response, which matches the reported "no session key" on valid credentials.

Tracing the fixed loop confirms it: root <QRZDatabase> → block == None → continue, reader advances into <Session> → matched → block = Session, then <Key> is read via readElementText with the block set. Minimal and targeted. r.name() returning the local name means the default xmlns namespace on the root doesn't interfere, as before.

CMake + README changes are consistent ✅

• The POST_BUILD copy reads third_party/qtkeychain/bin/qt6keychain.dll, which is exactly where setup-qtkeychain.ps1 installs it (CMAKE_INSTALL_PREFIX=$OutDir → $OutDir\bin\qt6keychain.dll, confirmed at script line 104). The WIN32 AND EXISTS guard keeps it a no-op when keychain isn't staged.
• The README step is the missing companion to the existing find_package(Qt6Keychain) path (CMakeLists.txt:127 already prepends third_party/qtkeychain to CMAKE_PREFIX_PATH), so the instructions now actually populate what the configure step looks for.

One suggestion (non-blocking)

You correctly note that parseXml() never saw a realistic root-wrapped response in tests — which is precisely how this shipped. A small regression test asserting sessionKey is populated from a <QRZDatabase><Session><Key>…</Key></Session></QRZDatabase> fixture (plus a <Callsign> lookup fixture) would lock this in cheaply. Heads-up that parseXml is private static (QrzClient.h:76) and tests/qrz_callsign_test.cpp doesn't currently touch QrzClient, so you'd need a test seam — a friend test class, or promoting the parse to a public static / free function. Worth it given this is the exact gap that let the bug through.

Scope is coherent — the CMake/README changes are a logically separate concern from the XML parse, but both serve the "QRZ login end-to-end" goal and are small, so bundling is reasonable. CI is green across all six checks. Nice work.

_{🤖 aethersdr-agent · cost: $7.4797 · model: claude-opus-4-8}

Good catch and a clean, well-diagnosed fix — thank you for chasing it down against a real QRZ.com account rather than stopping at "Test Login works." I traced parseXml against both real response shapes (login = <Session> only; lookup = <Callsign> then <Session> siblings under the <QRZDatabase> root) and the guard does exactly what you describe: the root wrapper is skipped instead of having readElementText swallow the whole document. The genuinely valuable side effect is that QRZ's real error strings (Username/password incorrect) now reach the user instead of the generic "no session key." The default xmlns is a non-issue since r.name() returns the local name. I also verified the build half end-to-end: setup-qtkeychain.ps1 installs to third_party/qtkeychain via CMAKE_INSTALL_PREFIX and builds --config Release (so the DLL name matches, no debug suffix), find_package prepends that same path, and the README renumbering is self-contained with Linux/macOS parity intact.

Nothing here is blocking — this fixes a real end-to-end breakage. Two things I'd like before merge, then a couple of nits.

Please add before merge

1. A parseXml regression test. This is the one that matters: QRZ login was broken end-to-end and shipped precisely because no test drove parseXml against a root-wrapped response — and this PR fixes it without adding that test, so the exact failure can silently return on any future token-walk refactor. You already name the gap in the PR body; closing it is the difference between "demonstrated" and "demonstrated to stay fixed" (Principle VIII). parseXml is private static, so it needs a friend-test or a thin exposure. Assert sessionKey is extracted from a full <QRZDatabase version="1.34"><Session><Key>ABC123</Key></Session></QRZDatabase> login body, and sessionError from an error body (<Session><Error>Username/password incorrect</Error></Session>).

2. Reset block on the closing tag (inline suggestion). Your new if (block == None) continue; guard is correct but never re-armed, because block is never set back to None when a <Session>/<Callsign> ends. It's latent today (login/lookup roots contain only those two children), but the moment the client issues a DXCC/bio query — or QRZ adds a top-level sibling — that element's children get read under the stale block. One line makes the guard actually load-bearing.

Nits (optional)

• The DLL-copy guard couples "staged in third_party" to "DLL exists." If Qt6Keychain resolves from a system/vcpkg install, Qt6Keychain_FOUND is true but EXISTS(third_party/...) is false → no copy → the exact "qt6keychain.dll not found" crash you're fixing. Narrow (the README path stages third_party), but copying from the found target's own location — $<TARGET_FILE:Qt6Keychain::Qt6Keychain> — handles any provider and drops the hardcoded path. Inline note on the CMake block.
• Co-locate the copy with the other bundled-DLL staging. Every other Windows DLL (hidapi, fftw3/fftw3f, ONNX, deepfilter) is copied in one clustered section with a COMMENT progress line; this one sits ~200 lines away with no COMMENT. Cheap to keep the pattern single-sourced.

Housekeeping: @jensenpat is already assigned (infrastructure tier owns CMakeLists.txt + *.md per CODEOWNERS, so this routes to them, not full maintainer) — leaving this review as the maintainer-side pass, not a claim on the merge. Commit correctly cites Principle IV. 73 — reviewed with Claude Code on behalf of KK7GWY.

Approving — this fixes a real end-to-end breakage (QRZ login has been non-functional since #3990), verified against a live QRZ.com account, with the parseXml root-wrapper fix traced against both real response shapes and the Windows keychain build/DLL path confirmed consistent. CI green across build/macOS/Windows.

Merging with two non-blocking follow-ups tracked from my review above, to fold into a quick follow-up PR rather than gate this one:

• a parseXml regression test against a root-<QRZDatabase>-wrapped body (the guard that would keep this exact bug from silently recurring — Principle VIII);
• the one-line block reset on EndElement so the new root-guard is re-armed for future non-Session/Callsign top-level siblings.

The $<TARGET_FILE:Qt6Keychain::Qt6Keychain> DLL-copy robustness tweak is optional polish. Thanks for the careful diagnosis. 73 de KK7GWY (reviewed with Claude Code)