Pull request overview

Hardens the in-process automation bridge (AutomationServer) against client disconnects during nested Qt event loops (e.g., QMenu::exec()), and expands the bridge’s UI introspection/drive capabilities to include popup menu actions.

Changes:

• Guard QLocalSocket lifetime with QPointer, and avoid holding buffer references across request handling in onReadyRead().
• Extend dumpTree to serialize QMenu actions as synthetic nodes (with metadata + geometry when visible).
• Allow invoke to resolve and trigger visible popup QActions by label/metadata, with TX-safety guarding.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Thanks for this, @rfoust — solid, careful work. The lifetime hardening is the right shape: QPointer-guarded socket, iterator-based buffer lookups (no operator[] re-insertion), buffer mutation scoped before handleLine(), and a re-validate (null + m_buffers.find + UnconnectedState) gate before writing the response back. That correctly covers the disconnect-during-nested-event-loop crash path against the m_buffers.remove(sock) + deleteLater() in onDisconnected. CI is green across all six checks (build, macOS, Windows, CodeQL, a11y).

On the Copilot comments — all three are already handled in this revision

1. operator[] re-insertion in onReadyRead — false positive against the current head. The new code uses m_buffers.find(socket) with iterators throughout and returns on end(); there's no operator[] access left on the hot path, so no silent empty-buffer re-insertion. This is exactly the fix Copilot recommended.

2. matchMenuAction recursing into non-visible submenus — already restricted. The recursion is guarded: if (QMenu* submenu = action->menu(); submenu && submenu->isVisible()). Top-level matchActionRecursive also gates on menu->isVisible(), so the "visible popup" contract holds.

3. toggle undocumented — it is documented. docs/automation-bridge.md adds the row | trigger / click / toggle | visible QMenu QAction | — | to the invoke table, matching the code's accepted aliases.

Two minor, non-blocking observations

• setChecked on a QAction emits toggled(bool), not triggered(). Handlers wired to QAction::triggered (common for menu items) won't fire on the setChecked path the way they do for trigger/click. That's a reasonable distinction (set-state vs. activate), but worth a one-line doc note so a driver knows setChecked may not run the same slot as trigger for some actions.

• Each QAction is emitted into both actions and synthetic children on a QMenu node — intentional and documented, just duplicates payload in dumpTree. Fine as-is given the inspect-while-blocked use case.

I also like that isTransmitAction keeps the keyword fallback (mox/ptt/transmit/cwx) deliberately broad and logged, defaulting toward blocking — the correct fail-safe direction for Principle VI. Nice touch noting in the comment that genuine keying actions should still carry an explicit kTxKeyingProperty marker.

Nothing blocking from me. 👍

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

Rebased onto current main and pushed (force-with-lease) — head 820ead81, now MERGEABLE.

It was 14 commits behind and conflicted in AutomationServer.cpp against both since-merged automation PRs (#3727, #3738). The collision was in onReadyRead(), where two changes met:

• #3738 changed handleLine(line) → handleLine(line, sock) (the socket param for log subscribe).
• this PR added the QPointer/iterator socket-lifetime hardening + the revalidate-before-write guard.

Resolved to keep both: the final onReadyRead uses QPointer<QLocalSocket> sock, socket = sock.data() per iteration, find()/iterator buffer lookups (no operator[] re-insertion), the disconnect revalidate (!socket || find==end || UnconnectedState) before write — and the 2-arg handleLine(line, sock). Both Copilot-feedback refinements from 132b85d8 are preserved.

Verified locally: full app builds clean (AutomationServer.cpp compiles, link OK); diff vs main is +403/−14, identical footprint to the pre-rebase PR.

Re-approving (force-push reset the prior approval). Verdict unchanged: the socket-lifetime UAF fix is sound, and the QAction invoke path is TX-guarded by isTransmitAction (marker-first + narrow keyword fallback, enforced before trigger()). I'll merge once CI is green.

(Commits are committer-signed by me from the rebase; the squash-merge will be GitHub-signed.)