Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe bridge adds usage receipt collection and acknowledgment operations. It defines protobuf messages and RPCs, registers gRPC service methods, and adds REST and gRPC client wrappers. Receipt collection is documented as non-resetting, and acknowledgment releases a receipt after durable storage. ChangesUsage Receipt API
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to The receipt APIs appear ready to merge after normal checks; no actionable issue is established by the supplied evidence. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new client calls use the existing authenticated connections, and no new authentication bypass was found. Receipt accounting nevertheless depends on compatible node behavior, durable panel storage, and a coordinated activation sequence that this client change cannot enforce. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 6 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the receipt by moonlit glow, Comment |
|
Adds
collect_usageandacknowledge_usageto both REST and gRPC clients. The node owns a stable receipt ID and retains the receipt until the caller durably stores it and explicitly acknowledges it. Repeating collection or ACK is safe; this client never falls back to destructiveGetStats(reset=True)and never acknowledges automatically.The protobuf contract and generated bindings add
UsageRequest,UsageReceipt,UsageAck,CollectUsage, andAcknowledgeUsage. Existing stats APIs remain unchanged. An empty receipt ID with no stats means there is no new traffic and requires no ACK.Companion panel: PasarGuard/panel#946. Requires the companion node receipt protocol: PasarGuard/node#94. Merge/release the node and Bridge before activating receipt accounting in the panel; drain legacy collectors before activation. The Bridge release containing these methods is a runtime prerequisite of the panel change.
Validation: existing suite 46 passed plus 5 subtests; Ruff passed. External local tests exercised both transports against the actual Go controller handlers over TLS, killed/restarted the server between collection and ACK, checked identical replay, repeated ACK, empty polls, and rejection of legacy resets. Both transport scenarios passed. Two additional real Go → Bridge → panel/SQLite recovery scenarios also passed: all 1,111 bytes were recovered once after a panel staging failure and node process restart.
No test, documentation, or benchmark files are included. This is a reliability protocol addition, not a claim of faster RPC throughput.
Upstream CI has status
action_requiredand requires repository-maintainer approval before it can run: https://github.com/PasarGuard/node_bridge_py/actions/runs/36254487456 . It is not reported as passed; the local suite and real transport results above are the available validation. The companion node current-head Linux CI passed, including WireGuard integration.