From 77f96e5d684efede87304283921093b82f9b3f6e Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Sun, 18 Oct 2026 10:29:03 +0300 Subject: [PATCH 1/2] Add Qodo review guidelines in .pr_agent.toml Qodo's free open-source tier reviews every PR but custom Rules are paid-only; what it does honour is a repo-level .pr_agent.toml. Encode the codebase's real review rules there: bounds checks in native packet parsing, WfbngLink device-map and thread-lifecycle rules, JNI/R8 keep requirements, UI-thread blocking, tools/ script hygiene, and the module coupling and path conventions from CLAUDE.md. Inline comments are limited to High/Medium findings. Co-Authored-By: Claude Fable 5.1 --- .pr_agent.toml | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 1 + 2 files changed, 58 insertions(+) create mode 100644 .pr_agent.toml diff --git a/.pr_agent.toml b/.pr_agent.toml new file mode 100644 index 00000000..38e47323 --- /dev/null +++ b/.pr_agent.toml @@ -0,0 +1,57 @@ +# Qodo (free open-source tier) review configuration for PixelPilot. +# Repo-level file: takes precedence over wiki and org settings. +# Only keys honoured by the free tier are used; custom Rules / Review Standards are paid-only. + +[review_agent] +# Inline comments only for High and Medium findings; everything still appears in the summary. +inline_comments_severity_threshold = 2 + +issues_user_guidelines = """ +Project: Android FPV ground station. Hot path is C++ behind thin JNI wrappers; a USB Realtek +adapter is driven in userspace (devourer) and wfb-ng / 802.11 frames are parsed in-process. + +Prioritise these classes of bugs, in order: +1. Native packet parsing (devourer RX callbacks, RxFrame, StaLink, wfb-ng aggregators): every read + of an 802.11 / LLC / IP / UDP / TCP / ICMP header or payload must be bounds-checked against the + remaining buffer length. Flag pointer arithmetic on packet data without a length check, + reassembly that does not track byte coverage, and any input-sized allocation or frame + injection without an upper bound. +2. WfbngLink threading: rtl_devices, current_fd, the aggregators and the worker/adaptive-link + threads are shared between the run thread, Java-called stop()/setters, and native workers. + Flag map mutation or device erase/reset without a mutex, raw pointers kept across a possible + erase, and .at() on an fd that another thread may have removed. devourer's IRtlDevice expects + a single control thread per device: flag SetTxPower / SetMonitorChannel / FastRetune calls + from a second thread. +3. Thread lifecycle: every std::thread started in native code must be joinable and joined on + stop() and on USB detach; flag detached threads, blocking calls that can hang stopAll(), and + condition-variable waits without a predicate or timeout. +4. JNI/R8: Java_com_openipc___ names and signatures must match the Java + declarations; any Java class, interface or field reached from C++ (reflection, callbacks) + needs @Keep or a rule in app/proguard-rules.pro, otherwise release builds crash. +5. Android: no blocking socket waits, Thread.join or synchronous camera HTTP on the UI thread in + VideoActivity or dialogs; runtime permissions and foreground-service rules for API 26-34. +6. Shell scripts under tools/: must validate every option argument, use set -euo pipefail, and + make changes on the camera atomic (backup first, restore on any later failure). +Do not report pure style issues; clang-format 18.1.8 is enforced by CI. +""" + +compliance_user_guidelines = """ +Repository conventions to check on every PR: +- Modules (app, videonative, wfbngrtl8812, mavlink) communicate only over loopback UDP + (5600 video, 14550 MAVLink, 8000/8001 VPN) and the 10.5.0.0/24 TUN; new cross-module native + calls or new hard-coded ports without a named constant are violations. +- No new hard-coded app-private paths such as /data/user/0/com.openipc.pixelpilot/...; pass the + Context files dir from Java. +- app/mavlink/src/main/cpp/mavlink/ is generated MAVLink code and must not be hand-edited. +- Changes to app/wfbngrtl8812/src/main/cpp/CMakeLists.txt that touch the devourer source list + must come with a matching devourer submodule bump, and submodule bumps must state what changed. +- Submodule checkouts in workflows stay non-recursive (devourer's reference/ submodules are not + buildable); do not add submodules: recursive. +- Log tag is "pixelpilot" in Java and C++ (devourer logs under "devourer"). +- Native code is C++20, formatted with scripts/format_sources.sh (clang-format 18.1.8). +- PRs that add a Java type touched from JNI must add @Keep or a proguard keep rule. +""" + +[github_app] +pr_commands = ["/agentic_describe", "/agentic_review"] +handle_push_trigger = false diff --git a/CLAUDE.md b/CLAUDE.md index fb43972f..b02902ed 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,7 @@ TUN 10.5.0.0/24 egress -> WfbNgVpnService -> :8001 -> TxFrame (FEC k=1,n=5, radi ## Conventions - Log tag is `"pixelpilot"` across Java and most C++; devourer logs under `"devourer"`. +- Qodo reviews every PR; its repo-level guidelines live in `.pr_agent.toml` and mirror the rules above. - JNI symbols follow `Java_com_openipc___`. Release builds run R8: `app/proguard-rules.pro` keeps native-method classes and the mavlink types, and classes that native code reflects on (`WfbNGStats`, `DecodingInfo`, callback interfaces) are `@Keep`. Any new From e642392db4878181d78abc406d5f8f948d52179b Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Sun, 18 Oct 2026 10:34:53 +0300 Subject: [PATCH 2/2] .pr_agent.toml: tighten three compliance guidelines Addresses Qodo's review of the config: the video receiver intentionally binds all interfaces on 5600, several native components use their own log tags, and only wfbngrtl8812/videonative set CXX_STANDARD 20. Co-Authored-By: Claude Fable 5.1 --- .pr_agent.toml | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/.pr_agent.toml b/.pr_agent.toml index 38e47323..c5cf5844 100644 --- a/.pr_agent.toml +++ b/.pr_agent.toml @@ -37,9 +37,11 @@ Do not report pure style issues; clang-format 18.1.8 is enforced by CI. compliance_user_guidelines = """ Repository conventions to check on every PR: -- Modules (app, videonative, wfbngrtl8812, mavlink) communicate only over loopback UDP - (5600 video, 14550 MAVLink, 8000/8001 VPN) and the 10.5.0.0/24 TUN; new cross-module native - calls or new hard-coded ports without a named constant are violations. +- Modules (app, videonative, wfbngrtl8812, mavlink) communicate through UDP sockets, not direct + native calls: wfb-ng aggregators send to 127.0.0.1:5600 (video), :14550 (MAVLink) and :8000 + (VPN ingress), the VPN service feeds :8001 to the transmitter, and the TUN is 10.5.0.0/24. The + video receiver deliberately binds INADDR_ANY:5600 so Wi-Fi senders work without an adapter. + New cross-module native calls or new hard-coded ports without a named constant are violations. - No new hard-coded app-private paths such as /data/user/0/com.openipc.pixelpilot/...; pass the Context files dir from Java. - app/mavlink/src/main/cpp/mavlink/ is generated MAVLink code and must not be hand-edited. @@ -47,8 +49,12 @@ Repository conventions to check on every PR: must come with a matching devourer submodule bump, and submodule bumps must state what changed. - Submodule checkouts in workflows stay non-recursive (devourer's reference/ submodules are not buildable); do not add submodules: recursive. -- Log tag is "pixelpilot" in Java and C++ (devourer logs under "devourer"). -- Native code is C++20, formatted with scripts/format_sources.sh (clang-format 18.1.8). +- Log tag is "pixelpilot" in Java and in most project-owned C++; a few native components use + their own tag (e.g. FecChangeController, SignalQualityCalculator, TXFrame, wfb-ng) and devourer + logs under "devourer". New code should use "pixelpilot" or an existing component tag. +- wfbngrtl8812 and videonative are C++20 (CXX_STANDARD 20 in their CMakeLists); the mavlink + target sets no standard, so keep it to features the NDK default supports. All project C++ is + formatted with scripts/format_sources.sh (clang-format 18.1.8). - PRs that add a Java type touched from JNI must add @Keep or a proguard keep rule. """