rebase lneto diff#6516
Conversation
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates Wasm build and release settings to use the Changeswglneto build, module wiring, and wasm proxy split
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lneto.diff (1)
1-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the committed
lneto.diffartifact before merge.This file duplicates tracked changes and will become stale quickly; keeping only canonical source changes reduces drift and review noise.
Suggested cleanup
diff --git a/lneto.diff b/lneto.diff deleted file mode 100644🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lneto.diff` around lines 1 - 96, Remove the lneto.diff file from the repository as it is a diff artifact file that should not be committed. This file duplicates the actual tracked changes visible in the diff and will become stale. Simply delete the lneto.diff file entirely from the repository before merging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Around line 345-347: The replace directives for github.com/soypat/lneto and
golang.zx2c4.com/wireguard reference non-existent local directories (../lneto
and ../wg-nb2) that break builds in CI and clean checkouts. Either remove these
two replace directives entirely from go.mod if they are not needed, or replace
the local path references with proper remote module versions using git commit
hashes or tags, consistent with how other replace directives are configured in
the file.
---
Nitpick comments:
In `@lneto.diff`:
- Around line 1-96: Remove the lneto.diff file from the repository as it is a
diff artifact file that should not be committed. This file duplicates the actual
tracked changes visible in the diff and will become stale. Simply delete the
lneto.diff file entirely from the repository before merging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e7fe8c3-bea0-47f3-8930-43b2a55e7277
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
.github/workflows/wasm-build-validation.yml.gitignore.goreleaser.yamlgo.modlneto.diff
| replace github.com/soypat/lneto => ../lneto | ||
|
|
||
| replace golang.zx2c4.com/wireguard => ../wg-nb2 // github.com/netbirdio/wireguard-go v0.0.0-20260523085312-4b4a4e36017f |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Current replace directives"
rg -n '^replace (github.com/soypat/lneto|golang\.zx2c4\.com/wireguard) => ' go.mod
echo
echo "2) Presence of sibling directories from repo root"
for p in ../lneto ../wg-nb2; do
if [ -d "$p" ]; then
echo "FOUND: $p"
else
echo "MISSING: $p"
fi
done
echo
echo "3) Any workflow/release config that checks out or creates these modules?"
rg -n "lneto|wg-nb2|actions/checkout|path:" .github/workflows .goreleaser.yaml
echo
echo "4) Check for workspace mapping file"
fd -a 'go.work|go.work.sum'Repository: netbirdio/netbird
Length of output: 6227
🏁 Script executed:
# Check for documentation about local development setup
rg -i "lneto|wg-nb2|local.*module|replace.*directive" README.md CONTRIBUTING.md docs/ 2>/dev/null || true
# Check if there's a .gitignore entry for go.work files
cat -n .gitignore 2>/dev/null | grep -i "go.work" || echo "No go.work in .gitignore"
# Check for any build instructions mentioning these modules
rg -i "wireguard|lneto" Makefile 2>/dev/null || true
# See if release workflow or goreleaser handles this differently
head -50 .goreleaser.yamlRepository: netbirdio/netbird
Length of output: 1490
🏁 Script executed:
# Check for git submodules
cat -n .gitmodules 2>/dev/null || echo "No .gitmodules"
# Check for setup scripts
fd -a 'setup\.|install\.|Makefile' -x grep -l "lneto\|wg-nb2" 2>/dev/null || echo "No setup scripts for these modules"
# Check if these modules are external dependencies that should be in go.mod instead
rg "github.com/soypat/lneto|golang.zx2c4.com/wireguard" go.mod -A 1 -B 1Repository: netbirdio/netbird
Length of output: 733
🏁 Script executed:
# Check git history of go.mod to see when these replace directives were added
git log --oneline -20 -- go.mod
# Check if there are any recent comments in go.mod about these
tail -20 go.mod
# Verify if CI actually tries to build the wglneto tag which uses these replacements
rg -n "wglneto" .github/workflows/Repository: netbirdio/netbird
Length of output: 1284
Remove local replace directives for non-existent sibling directories from go.mod.
Lines 345 and 347 reference ../lneto and ../wg-nb2, which do not exist in the repository. These local replace directives break builds in CI and clean checkouts. The golang.zx2c4.com/wireguard replacement is on an active code path (WASM builds use the wglneto tag), making this a critical blocker for reproducibility.
No CI workflows set up these sibling directories, and no build documentation explains their purpose. These appear to have been accidentally committed. Either remove these replace directives if they are not needed, or replace them with remote module versions (similar to the other replace directives in go.mod).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` around lines 345 - 347, The replace directives for
github.com/soypat/lneto and golang.zx2c4.com/wireguard reference non-existent
local directories (../lneto and ../wg-nb2) that break builds in CI and clean
checkouts. Either remove these two replace directives entirely from go.mod if
they are not needed, or replace the local path references with proper remote
module versions using git commit hashes or tags, consistent with how other
replace directives are configured in the file.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/firewall/create.go`:
- Line 1: The noopFirewall implementation is missing two firewall.Manager
methods, so update the noopFirewall type in create_js.go to satisfy the full
interface by adding RemoveAllLegacyRouteRules() and GetLegacyManagement()
alongside the existing stubbed methods. Make both methods no-op implementations
consistent with the rest of noopFirewall, and verify the compile-time interface
conformance for firewall.Manager still passes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7efb7554-5876-486f-b2b2-71feb5af307a
📒 Files selected for processing (3)
.gitignoreclient/firewall/create.goclient/firewall/create_js.go
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- client/firewall/create_js.go
| @@ -1,4 +1,4 @@ | |||
| //go:build !linux || android | |||
| //go:build (!linux || android) && !js | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify noopFirewall implements all firewall.Manager interface methods
# Find the noopFirewall definition
rg -n 'func \(.*noopFirewall\)' client/firewall/
# List all methods in the Manager interface
rg -n '^\t[A-Z]\w+\(' client/firewall/manager/firewall.go | head -40
# Check create_js.go build constraint
head -5 client/firewall/create_js.goRepository: netbirdio/netbird
Length of output: 4328
noopFirewall still appears incomplete for firewall.Manager
client/firewall/create_js.go covers most interface methods, but firewall.Manager also includes RemoveAllLegacyRouteRules() and GetLegacyManagement(), and they are not present on noopFirewall.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/firewall/create.go` at line 1, The noopFirewall implementation is
missing two firewall.Manager methods, so update the noopFirewall type in
create_js.go to satisfy the full interface by adding RemoveAllLegacyRouteRules()
and GetLegacyManagement() alongside the existing stubbed methods. Make both
methods no-op implementations consistent with the rest of noopFirewall, and
verify the compile-time interface conformance for firewall.Manager still passes.
2 participants
WIP though almost ready
Summary by CodeRabbit
Chores / Build Updates
wglnetobuild tag.New Features
Bug Fixes / Compatibility