Skip to content

feat: olcRTC tunnel client (in-process gomobile sidecar)#110

Merged
hawkff merged 1 commit into
mainfrom
feat/olcrtc-client
Jun 29, 2026
Merged

feat: olcRTC tunnel client (in-process gomobile sidecar)#110
hawkff merged 1 commit into
mainfrom
feat/olcrtc-client

Conversation

@hawkff

@hawkff hawkff commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

Adds support for connecting through olcRTC, an encrypted TCP-over-WebRTC tunnel carried over common meet services. Modeled as a profile that starts olcRTC's in-process loopback SOCKS5 listener (gomobile libolcrtc.aar) which a sing-box socks outbound dials.

Built as its own gomobile .aar so olcRTC's heavy dependency graph (pion/webrtc, livekit, kcp, grpc, ...) stays out of libcore's pinned sing-box module graph. Clones upstream at an immutable pinned commit (no fork needed; the Android protect fd-hook is already upstream).

Changes

  • buildScript/lib/olcrtc.sh + ./run lib olcrtc: gomobile bind ./mobile -> libolcrtc.aar (pinned olcRTC + pinned MatsuriDayo gomobile commits)
  • CI: new olcrtc Namespace job + Depot build steps; libolcrtc-aar-ci wired into every :app-compiling job
  • OlcrtcBean / OlcrtcFmt (olcrtc:// URI incl. &cid= client-id extension), settings UI, TypeMap/ProxyEntity/KryoConverters
  • OlcrtcInstance: in-process start/waitReady/stop, VpnService.protect fd-hook, single-instance guard, readiness gating, loopback SOCKS auth, non-Google resolver default
  • DB v10 + AutoMigration(9,10) for the new olcrtcBean column (+ 9->10 migration test)
  • OlcrtcFmt round-trip unit test

Tested

  • spotlessKotlinCheck green locally (JVM-only; APK/AAR build is CI-only per repo policy)
  • CodeRabbit CLI + reviewer + second-opinion pass applied
  • libcore-style AAR build + compile/lint/unit/migration gates run on CI
  • End-to-end device verification pending a live olcRTC server

Greptile Summary

This PR adds olcRTC (encrypted TCP-over-WebRTC) as a new proxy profile type, shipping the client as a native sidecar binary (libolcrtc.so) built from a Go wrapper around the olcRTC mobile package, following the same child-process pattern as mieru/naive/masterdnsvpn.

  • New profile type end-to-end: OlcrtcBean, OlcrtcFmt (URI parser/emitter with olcrtc:// scheme and &cid= extension), settings UI, DB v10 AutoMigration, TypeMap/KryoConverters, and a unit + migration test suite.
  • Sidecar integration: BoxInstance wires loopback SOCKS credentials and a two-tier readiness timeout (15 s strict / 60 s live) matching the MasterDnsVPN pattern; ConfigBuilder forces the carrier signaling host to dns-direct to avoid fake-IP loops on fakeip VPNs.
  • Log redaction: Commandline gains flag-level and output-pattern redaction for key, room ID, clientId, and SOCKS credentials, with a dedicated CommandlineTest.

Confidence Score: 4/5

Safe to merge once the olcrtc.sh commit pin is aligned with the CI env value; all other integration points follow established patterns.

The olcrtc.sh script defaults to commit ca5828cf… while both CI workflow files pin OLCRTC_COMMIT=3e970ab8…. Developers who run ./run lib olcrtc locally without setting the env var build a different libolcrtc.so than what is shipped in released APKs, making local end-to-end testing of the sidecar against a production build unreliable. Everything else — DB migration, URI codec, sidecar startup, log redaction, and CI caching — is correctly implemented and well-tested.

buildScript/lib/olcrtc.sh (OLCRTC_COMMIT default out of sync with CI) and app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt (payloadEnd search edge case with '<' in room URL).

Important Files Changed

Filename Overview
buildScript/lib/olcrtc.sh Build script for the olcRTC sidecar binary; the default OLCRTC_COMMIT diverges from both CI workflow pins, causing locally built binaries to differ from what ships in released APKs.
app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt URI parser/emitter for olcrtc:// links; payloadEnd search can be tricked by '<' appearing inside a room URL, but round-trip validation and buildOlcrtcArgs are otherwise well-structured.
app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcBean.java Data model for olcRTC profiles; correctly initializes defaults, serializes/deserializes with a version prefix, and follows existing bean conventions.
buildScript/lib/olcrtc-src/main.go Go sidecar wrapper: installs a protected net.DefaultResolver and http.DefaultTransport before olcRTC starts, passes all credentials as CLI flags, and mirrors the MasterDnsVPN protect-socket pattern faithfully.
app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt Correctly wires olcRTC sidecar startup, credential injection, and two-tier readiness timeout (strict vs live); logic mirrors the MasterDnsVPN pattern.
app/src/main/java/io/nekohasekai/sagernet/utils/Commandline.kt Adds flag-level and output-level log redaction for sensitive fields (key, room, clientId, socksPass); well-tested with the accompanying CommandlineTest.
app/src/main/java/io/nekohasekai/sagernet/database/SagerDatabase.kt Bumps DB version to 10 and registers the AutoMigration(9,10); migration test confirms existing rows survive and the new nullable olcrtcBean column is present.
app/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.kt Forces carrier signaling host to dns-direct to prevent fake-IP loops, and generates loopback SOCKS credentials for olcRTC exactly as it does for NaiveBean.
.github/workflows/ci.yml Adds olcRTC build step and updates sidecar verification; OLCRTC_COMMIT env pin (3e970ab8…) diverges from the script default (ca5828cf…), making CI and local builds non-identical.
app/src/main/java/io/nekohasekai/sagernet/ui/profile/OlcrtcSettingsActivity.kt Settings UI for olcRTC profiles; validates key length, room ID, and clientId before saving; DataStore null-coalescing is handled correctly in serialize().

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant App as NekoBox App
    participant BI as BoxInstance
    participant CB as ConfigBuilder
    participant PM as PluginManager
    participant SC as libolcrtc.so sidecar
    participant olc as olcRTC mobile lib
    participant Carrier as Meet Service (Jitsi/etc)
    participant SB as sing-box (socks outbound)

    App->>CB: buildConfig(OlcrtcBean)
    CB->>CB: generate loopback SOCKS creds
    CB->>CB: force carrier host to dns-direct
    CB->>CB: NUL-join args into pluginConfigs[port]
    App->>BI: init() / startPlugin()
    BI->>PM: initPlugin(olcrtc-plugin)
    PM-->>BI: libolcrtc.so path
    BI->>SC: exec(libolcrtc.so -carrier jitsi -room ... -key ... -socks-port N ...)
    SC->>SC: installProtectedDefaults(protect_path, dns)
    SC->>olc: mobile.StartWithTransport(...)
    olc->>Carrier: WebRTC signaling (protected sockets)
    Carrier-->>olc: ICE / room join
    olc-->>SC: ready
    SC->>SC: bind 127.0.0.1:N SOCKS5 listener
    SC-->>olc: mobile.WaitReady()
    BI->>BI: waitForSidecars() (port probe)
    SB->>SC: SOCKS5 connect (with creds)
    SC->>olc: tunnel TCP over WebRTC
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant App as NekoBox App
    participant BI as BoxInstance
    participant CB as ConfigBuilder
    participant PM as PluginManager
    participant SC as libolcrtc.so sidecar
    participant olc as olcRTC mobile lib
    participant Carrier as Meet Service (Jitsi/etc)
    participant SB as sing-box (socks outbound)

    App->>CB: buildConfig(OlcrtcBean)
    CB->>CB: generate loopback SOCKS creds
    CB->>CB: force carrier host to dns-direct
    CB->>CB: NUL-join args into pluginConfigs[port]
    App->>BI: init() / startPlugin()
    BI->>PM: initPlugin(olcrtc-plugin)
    PM-->>BI: libolcrtc.so path
    BI->>SC: exec(libolcrtc.so -carrier jitsi -room ... -key ... -socks-port N ...)
    SC->>SC: installProtectedDefaults(protect_path, dns)
    SC->>olc: mobile.StartWithTransport(...)
    olc->>Carrier: WebRTC signaling (protected sockets)
    Carrier-->>olc: ICE / room join
    olc-->>SC: ready
    SC->>SC: bind 127.0.0.1:N SOCKS5 listener
    SC-->>olc: mobile.WaitReady()
    BI->>BI: waitForSidecars() (port probe)
    SB->>SC: SOCKS5 connect (with creds)
    SC->>olc: tunnel TCP over WebRTC
Loading

Comments Outside Diff (1)

  1. app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt, line 1033-1060 (link)

    P2 roomId not validated against the delimiter set before URI emission

    toUri() validates clientId and name against DELIMITERS before embedding them, but roomId is written verbatim into the URI. Jitsi room IDs are https://… URLs, and a URL with a query string containing $ (percent-decoded) would corrupt the parser's lastIndexOf('$') comment-stripping step. While $ is not legal unencoded in a URL path, a user might paste an already-decoded string. Adding require(roomId.none { it == '$' }) (the only delimiter that can realistically appear in a URL and silently mangle the parse) makes the contract explicit and symmetric with the name check.

Reviews (11): Last reviewed commit: "feat(olcrtc): add Android tunnel client" | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds olcRTC as a new proxy type with URI parsing/serialization, database and settings wiring, sidecar process integration, a Go-sidecar build pipeline, and CI updates for building and verifying libolcrtc.so.

Changes

olcRTC Proxy Type

Layer / File(s) Summary
OlcrtcBean data model and URI codec
app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcBean.java, app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt, app/src/main/java/io/nekohasekai/sagernet/fmt/KryoConverters.java, app/src/main/java/io/nekohasekai/sagernet/fmt/TypeMap.kt, app/src/main/java/io/nekohasekai/sagernet/Constants.kt, app/src/test/java/io/nekohasekai/sagernet/fmt/OlcrtcFmtTest.kt
Defines OlcrtcBean, adds olcrtc:// parsing and emission, registers type/converters/keys, and covers codec behavior with tests.
ProxyEntity and Room DB migration to v10
app/src/main/java/io/nekohasekai/sagernet/database/ProxyEntity.kt, app/src/main/java/io/nekohasekai/sagernet/database/SagerDatabase.kt, app/src/main/java/io/nekohasekai/sagernet/database/DataStore.kt, app/src/androidTest/java/io/nekohasekai/sagernet/database/SagerDatabaseMigrationTest.kt
Adds TYPE_OLCRTC, olcrtcBean, Room v10 auto-migration, matching DataStore properties, and a migration test that checks row preservation and the new nullable column.
Link parsing and SOCKS credential wiring
app/src/main/java/io/nekohasekai/sagernet/ktx/Formats.kt, app/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.kt
Adds olcrtc:// to link parsing and extends SOCKS loopback credential generation to include OlcrtcBean.
BoxInstance sidecar integration and process lifecycle
app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt, app/src/main/java/io/nekohasekai/sagernet/bg/Executable.kt, app/src/main/java/io/nekohasekai/sagernet/plugin/PluginManager.kt
Builds and launches the olcrtc-plugin sidecar, adjusts readiness handling for OlcrtcBean, and registers libolcrtc.so in executable/plugin lookup paths.
Settings UI and manifest registration
app/src/main/java/io/nekohasekai/sagernet/ui/profile/OlcrtcSettingsActivity.kt, app/src/main/AndroidManifest.xml, app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt, app/src/main/res/...
Adds the OLCRTC settings screen, manifest entry, add-profile action, and supporting resources for carriers, transports, labels, and preferences.
Go sidecar source
buildScript/lib/olcrtc-src/main.go, buildScript/lib/olcrtc-src/go.mod
Adds the olcrtc-socks Go sidecar entrypoint, transport/protector logic, and module dependencies.
Cross-compile build script and CI pipeline
buildScript/lib/olcrtc.sh, .gitignore, .depot/workflows/..., .github/workflows/ci.yml
Adds the native build script for four Android ABIs and updates workflow caching, build steps, artifact checks, and ignore rules for the new sidecar outputs.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • hawkff/NekoBoxForAndroid#17: Extends the same sidecars build and CI orchestration, plus native-library handling in BoxInstance.kt and artifact verification.
  • hawkff/NekoBoxForAndroid#18: Extends the same external-sidecar runtime path in BoxInstance with parallel launch/readiness handling.
  • hawkff/NekoBoxForAndroid#68: Modifies the same Room migration setup and adds instrumented migration coverage for a schema version bump.

Poem

🐇 I hopped through links and sidecar smoke,
And found a tunnel in a byte-sized cloak.
olcrtc:// sings in ABI light,
With Room set v10 and builds all tight.
I twitched my nose—new paths abound!
A cozy warren of code is found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding an olcRTC tunnel client as an in-process gomobile sidecar.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description accurately matches the PR’s olcRTC profile, build, CI, database, and UI changes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 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 @.github/workflows/ci.yml:
- Around line 137-155: The olcrtc build job is missing the JDK setup required by
`buildScript/lib/olcrtc.sh` and the `Native Build` step that runs
`gomobile-matsuri bind`. Add the same `actions/setup-java` step used by the
other gomobile AAR build jobs before `Install Golang`/`Native Build`, so a cache
miss does not rely on a preinstalled JDK. Keep the fix scoped to the `olcrtc`
job in the CI workflow.
- Around line 139-147: The olcRTC cache key is based only on file contents, so
metadata-only changes to scripts can reuse a stale libolcrtc.aar. Update the
olcRTC Status step in the ci workflow to generate the status file using git
ls-files -s over the buildScript inputs instead of cat ... | sha1sum, matching
the existing metadata-aware pattern used elsewhere. Keep the olcRTC Cache step
keyed off that status file so cache invalidation reflects both content and mode
changes.

In `@app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt`:
- Around line 110-115: The olcRTC singleton check in BoxInstance’s OlcrtcBean
handling is only scoped to the current instance, so it can miss overlaps across
reloads or profile switches. Move the guard to shared process-wide state used by
BoxInstance/OlcrtcInstance, check that global state before creating a new olcRTC
instance, and clear it when OlcrtcInstance is torn down in close() so only one
process-wide olcRTC can exist at a time.
- Around line 301-313: The olcRTC readiness wait in BoxInstance should respect
strict mode instead of always enforcing a 60s minimum. In the block that gathers
OlcrtcInstance values and calls waitReady, change the timeout selection so it
uses the same strict timeout cap as the URL-test logic when strict = true, while
keeping the longer minimum only for non-strict behavior. Reference the existing
timeout handling near the OlcrtcInstance readiness path and reuse the same
strict-mode rule already used elsewhere in this class.

In `@app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt`:
- Around line 57-60: Reject unsupported carriers in OlcrtcFmt parsing by
validating the extracted carrier against the only supported values modeled by
the bean. In OlcrtcFmt.parseLink and the corresponding import path around the
carrier check, keep the non-empty guard but add an explicit allowlist for jitsi,
telemost, and wbstream so typoed links fail during import with a clear
invalid-link error instead of being accepted and breaking later.
- Around line 40-46: In OlcrtcFmt’s export/parsing flow, unnamed profiles with
roomId values containing $ are being emitted in a way that parseOlcrtc() later
splits at the last $ and truncates the room ID. Update the export logic for the
unnamed-profile path so room IDs are escaped or otherwise encoded before
concatenating with the comment delimiter, and make the corresponding import
handling in parseOlcrtc() decode it back correctly. Apply the same fix to the
related code path referenced by the other occurrence so both export sites
preserve $ characters in room IDs.

In
`@app/src/main/java/io/nekohasekai/sagernet/ui/profile/OlcrtcSettingsActivity.kt`:
- Around line 40-58: The OlcrtcBean.serialize() path currently saves clientId
from DataStore without validating it, which allows invalid profiles to be
persisted. Add a fail-fast require check for clientId alongside the existing
roomId and keyHex validation in OlcrtcSettingsActivity.serialize(), using a
clear error message that matches the existing style so blank CLIENT_HELLO device
IDs cannot be saved.
🪄 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: ca99f7c4-5f61-4982-a59a-4c104b684c11

📥 Commits

Reviewing files that changed from the base of the PR and between c2aa01b and cc235bc.

📒 Files selected for processing (28)
  • .depot/workflows/android-instrumented.yml
  • .depot/workflows/build-apk.yml
  • .depot/workflows/lint.yml
  • .depot/workflows/unit-tests.yml
  • .github/workflows/ci.yml
  • .gitignore
  • app/src/androidTest/java/io/nekohasekai/sagernet/database/SagerDatabaseMigrationTest.kt
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/io/nekohasekai/sagernet/Constants.kt
  • app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt
  • app/src/main/java/io/nekohasekai/sagernet/bg/proto/OlcrtcInstance.kt
  • app/src/main/java/io/nekohasekai/sagernet/database/DataStore.kt
  • app/src/main/java/io/nekohasekai/sagernet/database/ProxyEntity.kt
  • app/src/main/java/io/nekohasekai/sagernet/database/SagerDatabase.kt
  • app/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.kt
  • app/src/main/java/io/nekohasekai/sagernet/fmt/KryoConverters.java
  • app/src/main/java/io/nekohasekai/sagernet/fmt/TypeMap.kt
  • app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcBean.java
  • app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt
  • app/src/main/java/io/nekohasekai/sagernet/ktx/Formats.kt
  • app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt
  • app/src/main/java/io/nekohasekai/sagernet/ui/profile/OlcrtcSettingsActivity.kt
  • app/src/main/res/menu/add_profile_menu.xml
  • app/src/main/res/values/arrays.xml
  • app/src/main/res/values/strings.xml
  • app/src/main/res/xml/olcrtc_preferences.xml
  • app/src/test/java/io/nekohasekai/sagernet/fmt/OlcrtcFmtTest.kt
  • buildScript/lib/olcrtc.sh

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +137 to +155
- name: Checkout
uses: actions/checkout@v5
- name: olcRTC Status
run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status
- name: olcRTC Cache
id: cache
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
with:
path: |
app/libs/libolcrtc.aar
key: ${{ hashFiles('.github/workflows/*', 'olcrtc_status') }}-${{ env.OLCRTC_COMMIT }}-olcrtc-ci
- name: Install Golang
if: steps.cache.outputs.cache-hit != 'true'
uses: actions/setup-go@v6
with:
go-version: '1.26.4'
- name: Native Build
if: steps.cache.outputs.cache-hit != 'true'
run: ./run lib olcrtc

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add the missing JDK setup to the olcrtc build job.

This is the only gomobile AAR build in this workflow without actions/setup-java. buildScript/lib/olcrtc.sh runs gomobile-matsuri bind, so a cache miss currently depends on Java already being present on the runner and can fail on a fresh image.

Proposed fix
   olcrtc:
     name: Native Build (olcRTC)
     runs-on: namespace-profile-nekoyay
     needs:
       - guard
     steps:
       - name: Checkout
         uses: actions/checkout@v5
+      - name: Setup Java
+        uses: actions/setup-java@v5
+        with:
+          distribution: 'temurin'
+          java-version: '17'
       - name: olcRTC Status
         run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout
uses: actions/checkout@v5
- name: olcRTC Status
run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status
- name: olcRTC Cache
id: cache
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
with:
path: |
app/libs/libolcrtc.aar
key: ${{ hashFiles('.github/workflows/*', 'olcrtc_status') }}-${{ env.OLCRTC_COMMIT }}-olcrtc-ci
- name: Install Golang
if: steps.cache.outputs.cache-hit != 'true'
uses: actions/setup-go@v6
with:
go-version: '1.26.4'
- name: Native Build
if: steps.cache.outputs.cache-hit != 'true'
run: ./run lib olcrtc
- name: Checkout
uses: actions/checkout@v5
- name: Setup Java
uses: actions/setup-java@v5
with:
distribution: 'temurin'
java-version: '17'
- name: olcRTC Status
run: cat buildScript/lib/olcrtc.sh buildScript/init/env.sh buildScript/init/env_ndk.sh | sha1sum > olcrtc_status
- name: olcRTC Cache
id: cache
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
with:
path: |
app/libs/libolcrtc.aar
key: ${{ hashFiles('.github/workflows/*', 'olcrtc_status') }}-${{ env.OLCRTC_COMMIT }}-olcrtc-ci
- name: Install Golang
if: steps.cache.outputs.cache-hit != 'true'
uses: actions/setup-go@v6
with:
go-version: '1.26.4'
- name: Native Build
if: steps.cache.outputs.cache-hit != 'true'
run: ./run lib olcrtc
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 137-138: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 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 @.github/workflows/ci.yml around lines 137 - 155, The olcrtc build job is
missing the JDK setup required by `buildScript/lib/olcrtc.sh` and the `Native
Build` step that runs `gomobile-matsuri bind`. Add the same `actions/setup-java`
step used by the other gomobile AAR build jobs before `Install Golang`/`Native
Build`, so a cache miss does not rely on a preinstalled JDK. Keep the fix scoped
to the `olcrtc` job in the CI workflow.

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +110 to +115
is OlcrtcBean -> {
// olcRTC runs in-process (gomobile lib), not as a GuardedProcess.
// It is a process-global singleton, so reject a second olcRTC entry.
if (externalInstances.values.any { it is OlcrtcInstance }) {
error("olcRTC: only one olcRTC profile can run at a time")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Make the olcRTC singleton guard process-global.

Line 113 only checks externalInstances on the current BoxInstance. That does not protect against overlapping BoxInstance lifecycles during reload/profile switches, so two instances can still race the process-global Olcrtc singleton and later stop each other in close(). Move this guard to shared process-wide state and release it when the olcRTC instance is torn down.

🤖 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 `@app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt` around
lines 110 - 115, The olcRTC singleton check in BoxInstance’s OlcrtcBean handling
is only scoped to the current instance, so it can miss overlaps across reloads
or profile switches. Move the guard to shared process-wide state used by
BoxInstance/OlcrtcInstance, check that global state before creating a new olcRTC
instance, and clear it when OlcrtcInstance is torn down in close() so only one
process-wide olcRTC can exist at a time.

Comment thread app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt Outdated
Comment on lines +40 to +46
// Trailing comment after the LAST '$' (room URLs do not contain '$').
var comment = ""
val dollar = body.lastIndexOf('$')
if (dollar >= 0) {
comment = body.substring(dollar + 1)
body = body.substring(0, dollar)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve $ in room IDs when exporting unnamed profiles.

parseOlcrtc() treats the last $ as the comment delimiter. When name is blank, a roomId containing $ is exported raw and re-imports as a truncated room plus a synthetic comment.

Proposed fix
-    if (name.isNotBlank()) sb.append('$').append(name)
+    if (name.isNotBlank() || '$' in roomId) sb.append('$').append(name)
     return sb.toString()

Also applies to: 138-139

🤖 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 `@app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt` around
lines 40 - 46, In OlcrtcFmt’s export/parsing flow, unnamed profiles with roomId
values containing $ are being emitted in a way that parseOlcrtc() later splits
at the last $ and truncates the room ID. Update the export logic for the
unnamed-profile path so room IDs are escaped or otherwise encoded before
concatenating with the comment delimiter, and make the corresponding import
handling in parseOlcrtc() decode it back correctly. Apply the same fix to the
related code path referenced by the other occurrence so both export sites
preserve $ characters in room IDs.

Comment thread app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt
Comment thread app/src/main/java/io/nekohasekai/sagernet/ui/profile/OlcrtcSettingsActivity.kt Outdated
Comment thread .gitignore
Comment thread .github/workflows/ci.yml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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 `@app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt`:
- Around line 322-330: The readiness check in BoxInstance should not treat
olcRTC/MasterDnsVPN as a global flag for every pending port. Update the logic
around hasMasterDnsVpn, hasOlcrtc, and the pending-port handling near the
readiness timeout/failure path so the ports are tracked per sidecar/bean and
only those specific ports get the extended window or hard-fail behavior. Use the
existing config.externalIndex, chain.values, and requireBean() flow to associate
each pending port with its owning MasterDnsVpnBean or OlcrtcBean, and keep
strict mode as the only case that hard-fails all remaining pending ports.
- Around line 116-121: The olcRTC setup in BoxInstance.buildOlcrtcArgs currently
falls back to empty SOCKS credentials via creds?.first ?: "" and creds?.second
?: "", which can start the loopback proxy without auth; change this path to fail
closed when localProxyCredentials[port] is missing or incomplete. Ensure the
code either verifies creds is present before calling buildOlcrtcArgs or
throws/returns an error so olcRTC is not launched insecurely, and keep the fix
localized around the creds lookup and buildOlcrtcArgs call site.

In `@app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt`:
- Around line 165-176: The Olcrtc argument builder is missing the app readiness
deadline flag, so the sidecar always uses its default timeout. Update
buildOlcrtcArgs() in OlcrtcFmt.kt to emit -ready-timeout-ms using the same
timeout value that BoxInstance/DataStore.connectionTestTimeout relies on, so the
Go wrapper and app wait on the same deadline. Keep the change localized to the
args list construction and ensure the new flag is included alongside the
existing transport and DNS options.

In `@buildScript/lib/olcrtc-src/main.go`:
- Around line 40-67: The readiness wait in main is still hardcoded to the
default instead of using the user-configured timeout, so `mobile.WaitReady`
should be driven by the parsed `-ready-timeout-ms` flag. Update the startup flow
in `main` to pass the existing `readyMillis` value through the
`mobile.WaitReady` call, keeping the `StartWithTransport` and `WaitReady`
symbols aligned with the app’s readiness setting.

In `@buildScript/lib/olcrtc.sh`:
- Around line 74-80: The scratch module setup in olcrtc.sh still leaves the
build dependent on network access because `go mod tidy` resolves external
dependencies like `golang.org/x/sys` at build time. Replace the tidy step in the
`BUILD` scratch-module flow with a pinned dependency source, such as committing
and using a checked-in `go.sum` or vendored dependencies, so `go build` can run
offline after the `go.mod edit -replace` step.
🪄 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: 6928c91b-0314-449b-90b1-144396722bb4

📥 Commits

Reviewing files that changed from the base of the PR and between dd1dd1f and ac1084f.

📒 Files selected for processing (13)
  • .depot/workflows/android-instrumented.yml
  • .depot/workflows/build-apk.yml
  • .depot/workflows/lint.yml
  • .depot/workflows/unit-tests.yml
  • .github/workflows/ci.yml
  • .gitignore
  • app/src/main/java/io/nekohasekai/sagernet/bg/Executable.kt
  • app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt
  • app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt
  • app/src/main/java/io/nekohasekai/sagernet/plugin/PluginManager.kt
  • buildScript/lib/olcrtc-src/go.mod
  • buildScript/lib/olcrtc-src/main.go
  • buildScript/lib/olcrtc.sh
💤 Files with no reviewable changes (3)
  • .depot/workflows/lint.yml
  • .depot/workflows/unit-tests.yml
  • .depot/workflows/android-instrumented.yml
✅ Files skipped from review due to trivial changes (2)
  • buildScript/lib/olcrtc-src/go.mod
  • .gitignore

Comment thread app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt Outdated
Comment thread app/src/main/java/io/nekohasekai/sagernet/bg/proto/BoxInstance.kt
Comment thread app/src/main/java/io/nekohasekai/sagernet/fmt/olcrtc/OlcrtcFmt.kt
Comment thread buildScript/lib/olcrtc-src/main.go
Comment thread buildScript/lib/olcrtc.sh
Comment on lines +74 to +80
# Stage the wrapper in a scratch build dir and point it at the cloned olcRTC via a
# replace directive, so `go build` never has to resolve a pseudo-version online.
BUILD="$(pwd)/.olcrtc-wrapper"
rm -rf "$BUILD"
mkdir -p "$BUILD"
cp "$SRC/main.go" "$SRC/go.mod" "$BUILD/"
( cd "$BUILD" && go mod edit -replace "github.com/openlibrecommunity/olcrtc=$WORK" && go mod tidy )

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== staging logic =='
sed -n '74,90p' buildScript/lib/olcrtc.sh

echo
echo '== wrapper module files =='
fd -HI '^(go\.mod|go\.sum|vendor)$' buildScript/lib/olcrtc-src

echo
echo '== wrapper go.mod =='
sed -n '1,120p' buildScript/lib/olcrtc-src/go.mod

Repository: hawkff/NekoBoxForAndroid

Length of output: 1091


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== olcrtc wrapper directory listing =='
find buildScript/lib/olcrtc-src -maxdepth 2 -type f | sort

echo
echo '== any vendor directories in repo =='
find . -type d -name vendor | sort

echo
echo '== any go.sum files near wrapper =='
find buildScript/lib -name 'go.sum' -o -name 'vendor' | sort

echo
echo '== all references to golang.org/x/sys in repo =='
rg -n "golang\.org/x/sys" -S .

Repository: hawkff/NekoBoxForAndroid

Length of output: 1448


go mod tidy still makes this build network-dependent. The scratch module has no checked-in go.sum or vendor tree, so a clean runner still has to resolve golang.org/x/sys at build time. Use a pinned lock/vendor set instead of tidying in the build.

🤖 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 `@buildScript/lib/olcrtc.sh` around lines 74 - 80, The scratch module setup in
olcrtc.sh still leaves the build dependent on network access because `go mod
tidy` resolves external dependencies like `golang.org/x/sys` at build time.
Replace the tidy step in the `BUILD` scratch-module flow with a pinned
dependency source, such as committing and using a checked-in `go.sum` or
vendored dependencies, so `go build` can run offline after the `go.mod edit
-replace` step.

transport = DataStore.olcrtcTransport
vp8Fps = DataStore.olcrtcVp8Fps
vp8BatchSize = DataStore.olcrtcVp8Batch
dnsServer = DataStore.olcrtcDnsServer

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 DataStore.olcrtcDnsServer is declared as String? (no default), so it returns null when the field has never been written. This null is stored directly in OlcrtcBean.dnsServer. Later, buildOlcrtcArgs calls dnsServer.ifBlank { dnsFallback } on what Kotlin sees as a platform type String!; if the value is actually null that call throws NullPointerException at connect time, silently crashing the sidecar start. All other optional-string fields (carrier, transport) have default values in their DataStore declarations; dnsServer needs an explicit null-coalesce here instead.

Suggested change
dnsServer = DataStore.olcrtcDnsServer
dnsServer = DataStore.olcrtcDnsServer ?: ""

@hawkff hawkff force-pushed the feat/olcrtc-client branch from 20c1dc5 to ae36a82 Compare June 29, 2026 18:17
Add olcRTC profile import/export, settings, config generation, and sidecar launch support.

Include URL-test readiness handling, sidecar CI wiring, and JVM coverage for sanitized diagnostics.
@hawkff hawkff force-pushed the feat/olcrtc-client branch from ae36a82 to 6b0471a Compare June 29, 2026 18:33
@hawkff hawkff merged commit d4ccf48 into main Jun 29, 2026
7 checks passed
@hawkff hawkff deleted the feat/olcrtc-client branch June 29, 2026 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant