Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe macOS packet tunnel now resolves fallback utun descriptors by matching configured IPv4 and IPv6 addresses. The change adds descriptor resolution, error handling, target wiring, and tests for matching, ambiguity, duplicates, and invalid addresses. ChangesTunnel descriptor resolution
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant openTunSync
participant TunnelFileDescriptor
participant NetworkInterfaces
openTunSync->>TunnelFileDescriptor: resolve configured addresses
TunnelFileDescriptor->>NetworkInterfaces: inspect utun descriptors and active addresses
NetworkInterfaces-->>TunnelFileDescriptor: matching candidates
TunnelFileDescriptor-->>openTunSync: selected descriptor and interface name
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The fallback selects the tunnel descriptor using the configured tunnel addresses and fails safely when resolution cannot identify one. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🟡 Changes recommended
Critical issues currently prevent compilation and break the non-auto-route startup path.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates macOS tunnel descriptor selection to match configured IPv4/IPv6 addresses, preventing stale or ambiguous interfaces from being used.
Changes:
- Added address-based descriptor resolution.
- Added regression tests for stale, duplicate, and ambiguous descriptors.
- Integrated the resolver and tests into the Xcode project.
- Updated tunnel replacement documentation.
File summaries
| File | Description |
|---|---|
macos/RunnerTests/TunnelFileDescriptorTests.swift |
Adds descriptor-selection regression tests. |
macos/Runner.xcodeproj/project.pbxproj |
Registers implementation and test files. |
macos/PacketTunnel/SingBox/TunnelFileDescriptor.swift |
Implements address-based interface matching. |
macos/PacketTunnel/SingBox/ExtensionProvider.swift |
Updates tunnel replacement documentation. |
macos/PacketTunnel/SingBox/ExtensionPlatformInterface.swift |
Uses the new descriptor resolver. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let addresses = | ||
| (settings.ipv4Settings?.addresses ?? []) + (settings.ipv6Settings?.addresses ?? []) | ||
| let candidate = try TunnelFileDescriptor.resolve(addresses: addresses) |
| while let current = next { | ||
| next = current.pointee.ifa_next | ||
| guard let address = current.pointee.ifa_addr, | ||
| address.pointee.sa_family == AF_INET || address.pointee.sa_family == AF_INET6, |
| ] | ||
| XCTAssertEqual(try TunnelFileDescriptor.select(candidates, addresses: [address]), live) | ||
| XCTAssertEqual( | ||
| try TunnelFileDescriptor.select(candidates.reversed(), addresses: [address]), live) |
A failed tunnel start can leave an old utun descriptor in the extension process. A later connection could select that descriptor and appear connected without passing traffic.
Match fallback descriptors to the IPv4/IPv6 addresses configured for the tunnel, instead of relying on descriptor order. If no interface matches, or multiple interfaces match, fail startup rather than risk using the wrong tunnel. Descriptor ownership remains with NetworkExtension.
Adds regression coverage for stale and reused descriptors, ambiguous matches, duplicate handles, and equivalent IPv6 addresses.
Refs getlantern/engineering#3781
Summary by CodeRabbit
Bug Fixes
Tests