Skip to content

docs(config): audit test configs and add full.yaml reference#1338

Merged
ibigbug merged 7 commits into
masterfrom
feat/config-cleanup
Apr 26, 2026
Merged

docs(config): audit test configs and add full.yaml reference#1338
ibigbug merged 7 commits into
masterfrom
feat/config-cleanup

Conversation

@ibigbug
Copy link
Copy Markdown
Member

@ibigbug ibigbug commented Apr 26, 2026

Audit all YAML configs in clash-bin/tests/data/config/:

  • Add one-line purpose comment to every client config file
  • Remove stale 'experimental.ignore-resolve-fail' (field not in config def; the Experimental struct only has tcp_buffer_size)
  • Strip copy-pasted commented-out DNS boilerplate from protocol-specific configs (hysteria2, vless, wg, ss, uot) that obscured their actual content
  • Remove unreachable 'ss-02' proxy from tor.yaml
  • Collapse noisy commented-out rule/proxy entries in server.yaml, socks5.yaml
  • Fix rules.yaml experimental section to use the real tcp-buffer-size field

Add clash-bin/tests/data/config/full.yaml (781 lines):

  • Comprehensive reference covering every clash-rs config option
  • Sections: ports, network, mode, logging, external-controller, geo-dbs, profile, hosts, experimental, DNS (all listeners + modes), TUN, listeners, all proxy types (ss/vmess/vless/trojan/socks5/wg/hysteria2/tuic/anytls/ shadowquic/ssh/tor/tailscale), all group types, proxy-providers, rule-providers, all rule types
  • Marked as NOT a working config; values are examples/placeholders

What does this PR do?

Type

  • Bug fix
  • New feature
  • Refactoring / cleanup
  • Other

Checklist

  • Tests added or not needed
  • Docs updated or not needed

Summary by CodeRabbit

  • Documentation

    • Added descriptive header comments and a comprehensive "full reference" configuration documenting supported options.
  • Tests

    • Removed many legacy sample/test configuration files and simplified remaining fixtures.
    • Updated test harnesses to generate required server/config payloads at runtime (temporary/generated configs) and standardized TLS asset locations for test containers.
    • Cleaned up obsolete prebuilt/public test web assets.

Audit all YAML configs in clash-bin/tests/data/config/:
- Add one-line purpose comment to every client config file
- Remove stale 'experimental.ignore-resolve-fail' (field not in config def;
  the Experimental struct only has tcp_buffer_size)
- Strip copy-pasted commented-out DNS boilerplate from protocol-specific
  configs (hysteria2, vless, wg, ss, uot) that obscured their actual content
- Remove unreachable 'ss-02' proxy from tor.yaml
- Collapse noisy commented-out rule/proxy entries in server.yaml, socks5.yaml
- Fix rules.yaml experimental section to use the real tcp-buffer-size field

Add clash-bin/tests/data/config/full.yaml (781 lines):
- Comprehensive reference covering every clash-rs config option
- Sections: ports, network, mode, logging, external-controller, geo-dbs,
  profile, hosts, experimental, DNS (all listeners + modes), TUN, listeners,
  all proxy types (ss/vmess/vless/trojan/socks5/wg/hysteria2/tuic/anytls/
  shadowquic/ssh/tor/tailscale), all group types, proxy-providers,
  rule-providers, all rule types
- Marked as NOT a working config; values are examples/placeholders

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6b51d332-5d65-4d90-bd05-e43dbe940808

📥 Commits

Reviewing files that changed from the base of the PR and between 9f7b89c and 03d058c.

⛔ Files ignored due to path filters (11)
  • clash-bin/tests/data/config/public/apple-touch-icon-precomposed.png is excluded by !**/*.png
  • clash-bin/tests/data/config/public/assets/inter-latin-400-normal.0364d368.woff2 is excluded by !**/*.woff2
  • clash-bin/tests/data/config/public/assets/inter-latin-400-normal.3ea830d4.woff is excluded by !**/*.woff
  • clash-bin/tests/data/config/public/assets/inter-latin-800-normal.a51ac27d.woff2 is excluded by !**/*.woff2
  • clash-bin/tests/data/config/public/assets/inter-latin-800-normal.d08d7178.woff is excluded by !**/*.woff
  • clash-bin/tests/data/config/public/assets/roboto-mono-latin-400-normal.7295944e.woff2 is excluded by !**/*.woff2
  • clash-bin/tests/data/config/public/assets/roboto-mono-latin-400-normal.dffdffa7.woff is excluded by !**/*.woff
  • clash-bin/tests/data/config/public/yacd-128.png is excluded by !**/*.png
  • clash-bin/tests/data/config/public/yacd-64.png is excluded by !**/*.png
  • clash-bin/tests/data/config/public/yacd.ico is excluded by !**/*.ico
  • clash-bin/tests/data/config/wg/peer1/peer1.png is excluded by !**/*.png
📒 Files selected for processing (53)
  • clash-bin/tests/data/config/listeners/tunnel.yaml
  • clash-bin/tests/data/config/public/CNAME
  • clash-bin/tests/data/config/public/_headers
  • clash-bin/tests/data/config/public/assets/Config.39d8d2ef.css
  • clash-bin/tests/data/config/public/assets/Config.c09e8dbe.js
  • clash-bin/tests/data/config/public/assets/Connections.e48eac36.js
  • clash-bin/tests/data/config/public/assets/Connections.fb8ea59b.css
  • clash-bin/tests/data/config/public/assets/Fab.a0a7e573.css
  • clash-bin/tests/data/config/public/assets/Fab.ef67ff10.js
  • clash-bin/tests/data/config/public/assets/Logs.4b8e75d1.css
  • clash-bin/tests/data/config/public/assets/Logs.ac990610.js
  • clash-bin/tests/data/config/public/assets/Proxies.16b46af4.js
  • clash-bin/tests/data/config/public/assets/Proxies.3fa3509d.css
  • clash-bin/tests/data/config/public/assets/Rules.70e6962f.js
  • clash-bin/tests/data/config/public/assets/Rules.e03c54a8.css
  • clash-bin/tests/data/config/public/assets/Select.1e55eba1.css
  • clash-bin/tests/data/config/public/assets/Select.6c389032.js
  • clash-bin/tests/data/config/public/assets/TextFitler.61537a57.js
  • clash-bin/tests/data/config/public/assets/TextFitler.b21c0577.css
  • clash-bin/tests/data/config/public/assets/chart-lib.a8ad03fd.js
  • clash-bin/tests/data/config/public/assets/chevron-down.dd238e96.js
  • clash-bin/tests/data/config/public/assets/debounce.c2d20996.js
  • clash-bin/tests/data/config/public/assets/en.fb34eaf7.js
  • clash-bin/tests/data/config/public/assets/index.171f553a.js
  • clash-bin/tests/data/config/public/assets/index.8bb012c6.js
  • clash-bin/tests/data/config/public/assets/index.92e2d967.js
  • clash-bin/tests/data/config/public/assets/index.b38debfc.css
  • clash-bin/tests/data/config/public/assets/index.esm.e4dd1508.js
  • clash-bin/tests/data/config/public/assets/logs.43986220.js
  • clash-bin/tests/data/config/public/assets/play.7b1a5f99.js
  • clash-bin/tests/data/config/public/assets/useRemainingViewPortHeight.7395542b.js
  • clash-bin/tests/data/config/public/assets/zh.9b79b7bf.js
  • clash-bin/tests/data/config/public/index.html
  • clash-bin/tests/data/config/public/manifest.webmanifest
  • clash-bin/tests/data/config/public/registerSW.js
  • clash-bin/tests/data/config/public/sw.js
  • clash-bin/tests/data/config/rule-set-classical.yaml
  • clash-bin/tests/data/config/rule-set.yaml
  • clash-bin/tests/data/config/rules.yaml
  • clash-bin/tests/data/config/server.yaml
  • clash-bin/tests/data/config/wg/.donoteditthisfile
  • clash-bin/tests/data/config/wg/coredns/Corefile
  • clash-bin/tests/data/config/wg/peer1/peer1.conf
  • clash-bin/tests/data/config/wg/peer1/presharedkey-peer1
  • clash-bin/tests/data/config/wg/peer1/privatekey-peer1
  • clash-bin/tests/data/config/wg/peer1/publickey-peer1
  • clash-bin/tests/data/config/wg/server/privatekey-server
  • clash-bin/tests/data/config/wg/server/publickey-server
  • clash-bin/tests/data/config/wg/templates/peer.conf
  • clash-bin/tests/data/config/wg/templates/server.conf
  • clash-bin/tests/data/config/wg/wg_confs/wg0.conf
  • clash-lib/src/proxy/wg/mod.rs
  • clash-lib/tests/data/config/client/rules.yaml
💤 Files with no reviewable changes (37)
  • clash-bin/tests/data/config/public/CNAME
  • clash-bin/tests/data/config/rule-set-classical.yaml
  • clash-bin/tests/data/config/rule-set.yaml
  • clash-bin/tests/data/config/public/assets/Logs.4b8e75d1.css
  • clash-bin/tests/data/config/public/assets/Select.1e55eba1.css
  • clash-bin/tests/data/config/public/assets/chevron-down.dd238e96.js
  • clash-bin/tests/data/config/public/assets/Config.39d8d2ef.css
  • clash-bin/tests/data/config/public/assets/Rules.e03c54a8.css
  • clash-bin/tests/data/config/public/assets/en.fb34eaf7.js
  • clash-bin/tests/data/config/public/assets/play.7b1a5f99.js
  • clash-bin/tests/data/config/public/assets/TextFitler.b21c0577.css
  • clash-bin/tests/data/config/public/assets/Config.c09e8dbe.js
  • clash-bin/tests/data/config/public/registerSW.js
  • clash-bin/tests/data/config/public/manifest.webmanifest
  • clash-bin/tests/data/config/public/assets/index.b38debfc.css
  • clash-bin/tests/data/config/public/_headers
  • clash-bin/tests/data/config/public/index.html
  • clash-bin/tests/data/config/public/assets/Select.6c389032.js
  • clash-bin/tests/data/config/public/assets/Connections.fb8ea59b.css
  • clash-bin/tests/data/config/public/assets/debounce.c2d20996.js
  • clash-bin/tests/data/config/rules.yaml
  • clash-bin/tests/data/config/public/assets/Rules.70e6962f.js
  • clash-bin/tests/data/config/public/assets/Fab.a0a7e573.css
  • clash-bin/tests/data/config/public/assets/index.92e2d967.js
  • clash-bin/tests/data/config/public/assets/Proxies.16b46af4.js
  • clash-bin/tests/data/config/public/assets/useRemainingViewPortHeight.7395542b.js
  • clash-bin/tests/data/config/public/assets/Connections.e48eac36.js
  • clash-bin/tests/data/config/public/assets/Proxies.3fa3509d.css
  • clash-bin/tests/data/config/public/assets/Logs.ac990610.js
  • clash-bin/tests/data/config/server.yaml
  • clash-bin/tests/data/config/public/assets/zh.9b79b7bf.js
  • clash-bin/tests/data/config/listeners/tunnel.yaml
  • clash-bin/tests/data/config/public/assets/TextFitler.61537a57.js
  • clash-bin/tests/data/config/public/sw.js
  • clash-bin/tests/data/config/public/assets/index.esm.e4dd1508.js
  • clash-bin/tests/data/config/public/assets/logs.43986220.js
  • clash-bin/tests/data/config/public/assets/Fab.ef67ff10.js
✅ Files skipped from review due to trivial changes (2)
  • clash-lib/src/proxy/wg/mod.rs
  • clash-lib/tests/data/config/client/rules.yaml

📝 Walkthrough

Walkthrough

Many test fixture files were removed or simplified; numerous dockerized Rust test runners were updated to generate server configs at runtime (write embedded config to NamedTempFile and mount into containers); a comprehensive non-runnable reference YAML full.yaml was added. No public API or exported symbols changed.

Changes

Cohort / File(s) Summary
Deleted test fixtures (YAML/JSON/TOML)
clash-bin/tests/data/config/config.test.yaml, clash-bin/tests/data/config/dns.yaml, clash-bin/tests/data/config/hysteria2.yaml, clash-bin/tests/data/config/shadowquic.yaml, clash-bin/tests/data/config/simple.yaml, clash-bin/tests/data/config/socks5.yaml, clash-bin/tests/data/config/ss.yaml, clash-bin/tests/data/config/tor.yaml, clash-bin/tests/data/config/tproxy.yaml, clash-bin/tests/data/config/tun.yaml, clash-bin/tests/data/config/uot.yaml, clash-bin/tests/data/config/vless-tcp-reality.yaml, clash-bin/tests/data/config/vless.yaml, clash-bin/tests/data/config/wg.yaml, clash-bin/tests/data/config/anytls.json, clash-bin/tests/data/config/hysteria.json, clash-bin/tests/data/config/socks5-auth.json, clash-bin/tests/data/config/socks5-noauth.json, clash-bin/tests/data/config/ss.json, clash-bin/tests/data/config/trojan-grpc.json, clash-bin/tests/data/config/trojan-ws.json, clash-bin/tests/data/config/tuic.toml, clash-bin/tests/data/config/vless-ws-tls.json, clash-bin/tests/data/config/vmess-grpc.json, clash-bin/tests/data/config/vmess-http2.json, clash-bin/tests/data/config/vmess-ws.json
Removed many static test fixture files (network listeners, DNS, proxies, rules). These are deletions of test data assets only.
Public/static web assets removed/trimmed
clash-bin/tests/data/config/public/*, clash-bin/tests/data/config/public/assets/*
Deleted/emptied numerous prebuilt UI assets and static site files (HTML, JS bundles, CSS, service worker, manifest). These are test/static assets removals.
Modified/simplified YAML fixtures + added reference
clash-bin/tests/data/config/empty.yaml, clash-bin/tests/data/config/listeners/tunnel.yaml, clash-bin/tests/data/config/rules.yaml, clash-bin/tests/data/config/server.yaml, clash-bin/tests/data/config/full.yaml
Simplified several YAML fixtures and added a large non-runnable full.yaml documenting supported config fields; some small comment/paths updates.
Test harness — generate configs at runtime
clash-lib/src/proxy/anytls/mod.rs, clash-lib/src/proxy/hysteria2/mod.rs, clash-lib/src/proxy/shadowquic/mod.rs, clash-lib/src/proxy/shadowsocks/outbound/mod.rs, clash-lib/src/proxy/socks/outbound/mod.rs, clash-lib/src/proxy/trojan/mod.rs, clash-lib/src/proxy/tuic/mod.rs, clash-lib/src/proxy/vless/mod.rs, clash-lib/src/proxy/vmess/mod.rs, clash-lib/src/proxy/wg/mod.rs
Dockerized test modules now embed server configs as inline constants, write them to tempfile::NamedTempFile (using std::io::Write), mount the temp file into container config paths, update cert/key mount paths to certs/..., and drop temp file handles before returning the built runner. Changes scoped to #[cfg(test)] docker-test code.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Test as Test harness
    participant Temp as NamedTempFile
    participant Docker as Docker container
    participant Runner as DockerTestRunner

    Test->>Temp: create NamedTempFile + write embedded config (io::Write)
    Test->>Docker: mount temp file at container config path (e.g. /etc/...)
    Test->>Docker: mount cert/key from `certs/...`
    Test->>Runner: await builder().await to start container
    Test->>Temp: drop file handle
    Runner-->>Test: return built runner
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • Itsusinn

Poem

🐇 I scribbled configs in memory, then hopped them to a temp,
I mounted certs in neat little nests where containers dream and slept,
Old files gone, new runners hum, containers wake and play,
A rabbit's cheer for tests that run — hop, build, and pass away!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'docs(config): audit test configs and add full.yaml reference' clearly and specifically summarizes the main changes: auditing test configurations and adding a comprehensive reference file.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/config-cleanup

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@clash-bin/tests/data/config/full.yaml`:
- Around line 200-205: The example value for the device-id key is inconsistent:
it currently uses "dev://utun1989" (mixing Linux dev:// with macOS utun naming).
Update the device-id example to a correct macOS form by changing the value to
"utun1989" (or, if you prefer the Linux example, change it to "dev://tun0");
modify the device-id entry so it matches one of the documented formats (refer to
the device-id key and the example values shown).
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5bf5b21c-6121-4324-9edf-6d60806c8340

📥 Commits

Reviewing files that changed from the base of the PR and between cd7b7f7 and 7fd2c1e.

📒 Files selected for processing (19)
  • clash-bin/tests/data/config/config.test.yaml
  • clash-bin/tests/data/config/dns.yaml
  • clash-bin/tests/data/config/empty.yaml
  • clash-bin/tests/data/config/full.yaml
  • clash-bin/tests/data/config/hysteria2.yaml
  • clash-bin/tests/data/config/listeners/tunnel.yaml
  • clash-bin/tests/data/config/rules.yaml
  • clash-bin/tests/data/config/server.yaml
  • clash-bin/tests/data/config/shadowquic.yaml
  • clash-bin/tests/data/config/simple.yaml
  • clash-bin/tests/data/config/socks5.yaml
  • clash-bin/tests/data/config/ss.yaml
  • clash-bin/tests/data/config/tor.yaml
  • clash-bin/tests/data/config/tproxy.yaml
  • clash-bin/tests/data/config/tun.yaml
  • clash-bin/tests/data/config/uot.yaml
  • clash-bin/tests/data/config/vless-tcp-reality.yaml
  • clash-bin/tests/data/config/vless.yaml
  • clash-bin/tests/data/config/wg.yaml

Comment on lines +200 to +205
# Device identifier:
# "utun1989" — macOS (utun prefix required by the OS)
# "dev://tun0" — Linux explicit device
# "fd://3" — use an existing open file descriptor
device-id: "dev://utun1989"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix inconsistent TUN device-id example format.

Line 204 mixes the Linux dev:// prefix with a macOS utun identifier (dev://utun1989), which conflicts with the formats documented in Lines 201-203.

Suggested fix
-  device-id: "dev://utun1989"
+  device-id: "utun1989"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clash-bin/tests/data/config/full.yaml` around lines 200 - 205, The example
value for the device-id key is inconsistent: it currently uses "dev://utun1989"
(mixing Linux dev:// with macOS utun naming). Update the device-id example to a
correct macOS form by changing the value to "utun1989" (or, if you prefer the
Linux example, change it to "dev://tun0"); modify the device-id entry so it
matches one of the documented formats (refer to the device-id key and the
example values shown).

ibigbug and others added 3 commits April 26, 2026 07:04
…yaml

Nameservers support URL fragment parameters:
  proxy=<name>       route DNS queries through a named outbound proxy
  interface=<name>   bind to a specific network interface
  <bare name>        shorthand for proxy=<name>

These apply to nameserver, fallback, nameserver-policy values, and
proxy-server-nameserver. Adds examples showing DNS-over-HTTPS via proxy.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e audit

Remove hallucinated fields not in config structs; add missing supported
fields found in def.rs and implementation code.

- interface-name → interface: the Config struct field is `pub interface`,
  which serde maps to the YAML key `interface` (not `interface-name`);
  the old name was silently dropped by serde
- dns.listen.dot.hostname removed: DoTConfig (clash-dns/src/lib.rs) has
  no `hostname` field — only DoHConfig and DoH3Config do; the key was
  accepted but silently ignored
- listeners: document `allow-lan` and `fw-mark` from CommonInboundOpts
  (config/internal/listener.rs) which were missing from all examples

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…se audit

Remove hallucinated proxy fields; add missing supported options found in
converter structs and config def. Correct rule type names.

Changes:
- proxy/vmess: use primary field name 'server-name' (not alias 'servername');
  fix comment direction — alias is 'servername', primary is 'server-name'
  (OutboundVmess: #[serde(alias = "servername")] pub server_name)
- proxy/vless: same server-name fix for vless-ws-tls, vless-grpc, vless-reality
  (OutboundVless: #[serde(alias = "servername")] pub server_name)
- proxy-providers: remove hallucinated 'timeout' from health-check block —
  HealthCheck struct has no timeout field (enable/url/interval/lazy only)
- rule-providers: add 'format' field (yaml | text | mrs) — supported by
  both FileRuleProvider and HttpRuleProvider as Option<RuleSetFormat>
- rules: add explicit IP-CIDR6 example — supported as alias for IP-CIDR
  in rule parser ('IP-CIDR' | 'IP-CIDR6' both handled by RuleType::IpCidr)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… unused test data files

- Delete 17 unused client YAML/data files (manual test artifacts, mmdb auto-downloaded)
- Move cert files to certs/ subdirectory and update all Rust references
- Inline all docker server configs (JSON/TOML/YAML) as Rust const strings
  written to NamedTempFile for container mounting
- Result: clash-bin/tests/data/config/ contains only certs/, ssh/, wg_config/,
  listeners/, public/, empty.yaml, full.yaml, server.yaml, rules.yaml, and
  rule-set*.yaml

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
clash-lib/src/proxy/socks/outbound/mod.rs (1)

334-383: Build the auth fixture from USER / PASSWORD instead of duplicating literals.

Right now the container config hardcodes "user" and "password" while the client side uses USER and PASSWORD. That makes this test easy to break with a one-sided edit.

♻️ Possible cleanup
-    const SOCKS5_AUTH_SERVER_CONFIG: &str = r#"{
-    "log": {
-        "loglevel": "debug"
-    },
-    "inbounds": [
-        {
-            "port": 10002,
-            "listen": "0.0.0.0",
-            "protocol": "socks",
-            "settings": {
-                "auth": "password",
-                "accounts": [
-                    {
-                        "user": "user",
-                        "pass": "password"
-                    }
-                ],
-                "udp": true,
-                "ip": "0.0.0.0"
-            }
-        }
-    ],
-    "outbounds": [
-        {
-            "protocol": "freedom"
-        }
-    ]
-}"#;
+    fn socks5_auth_server_config() -> String {
+        format!(
+            r#"{{
+    "log": {{
+        "loglevel": "debug"
+    }},
+    "inbounds": [
+        {{
+            "port": 10002,
+            "listen": "0.0.0.0",
+            "protocol": "socks",
+            "settings": {{
+                "auth": "password",
+                "accounts": [
+                    {{
+                        "user": "{USER}",
+                        "pass": "{PASSWORD}"
+                    }}
+                ],
+                "udp": true,
+                "ip": "0.0.0.0"
+            }}
+        }}
+    ],
+    "outbounds": [
+        {{
+            "protocol": "freedom"
+        }}
+    ]
+}}"#
+        )
+    }
@@
-        let config = if auth {
-            SOCKS5_AUTH_SERVER_CONFIG
+        let config = if auth {
+            socks5_auth_server_config()
         } else {
-            SOCKS5_NOAUTH_SERVER_CONFIG
+            SOCKS5_NOAUTH_SERVER_CONFIG.to_owned()
         };
@@
-        tmp.write_all(config.as_bytes())?;
+        tmp.write_all(config.as_bytes())?;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clash-lib/src/proxy/socks/outbound/mod.rs` around lines 334 - 383, The
SOCKS5_AUTH_SERVER_CONFIG literal duplicates the credentials instead of using
the test USER/PASSWORD constants, which can cause one-sided edits to break
tests; update get_socks5_runner so the auth server config is generated from the
shared USER and PASSWORD (e.g., build the JSON string or serde_json value using
USER and PASSWORD) rather than hardcoded "user"/"password", ensuring the
container config and the client use the same credentials.
clash-lib/src/proxy/vless/mod.rs (1)

258-331: Consider moving the embedded VLESS server fixture into test-only code.

This JSON block is now large enough that it makes the handler module harder to scan. A docker test file under clash-lib/tests/ or a shared test fixture helper would keep the runtime module focused.

Based on learnings: Place integration tests in clash_lib/tests/ directory with separate files for smoke_tests, api_tests, and Docker-based protocol tests.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clash-lib/src/proxy/vless/mod.rs` around lines 258 - 331, Extract the large
JSON const VLESS_WS_TLS_SERVER_CONFIG out of the runtime handler (mod.rs) and
into test-only code: create a dedicated test fixture module (annotated with
#[cfg(test)]) or a shared test helper in the tests crate and move the string
there, replacing the in-module const with a lightweight reference or a private
accessor used only by tests; update any test imports to use the new fixture
symbol and remove the embedded JSON from the production handler so mod.rs no
longer contains the large VLESS_WS_TLS_SERVER_CONFIG constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@clash-lib/src/proxy/anytls/mod.rs`:
- Around line 844-860: The temp file created with tempfile::NamedTempFile (tmp)
is dropped right after DockerTestRunnerBuilder::build().await, which can remove
the bind-mounted config before the container process reads it; keep the temp
file alive for the container lifetime by moving the NamedTempFile into the
runner state instead of dropping it (e.g. store tmp on the DockerTestRunner
struct or return a wrapper that owns the NamedTempFile), or alternatively write
the config to a stable test fixture directory rather than using NamedTempFile;
update the code that builds/returns the runner
(DockerTestRunnerBuilder::build()/DockerTestRunner) to hold ownership of the
file so drop(tmp) is removed and the file remains on disk while the container
runs.

---

Nitpick comments:
In `@clash-lib/src/proxy/socks/outbound/mod.rs`:
- Around line 334-383: The SOCKS5_AUTH_SERVER_CONFIG literal duplicates the
credentials instead of using the test USER/PASSWORD constants, which can cause
one-sided edits to break tests; update get_socks5_runner so the auth server
config is generated from the shared USER and PASSWORD (e.g., build the JSON
string or serde_json value using USER and PASSWORD) rather than hardcoded
"user"/"password", ensuring the container config and the client use the same
credentials.

In `@clash-lib/src/proxy/vless/mod.rs`:
- Around line 258-331: Extract the large JSON const VLESS_WS_TLS_SERVER_CONFIG
out of the runtime handler (mod.rs) and into test-only code: create a dedicated
test fixture module (annotated with #[cfg(test)]) or a shared test helper in the
tests crate and move the string there, replacing the in-module const with a
lightweight reference or a private accessor used only by tests; update any test
imports to use the new fixture symbol and remove the embedded JSON from the
production handler so mod.rs no longer contains the large
VLESS_WS_TLS_SERVER_CONFIG constant.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 303ce47b-9d3f-452c-b9f2-18d89036b679

📥 Commits

Reviewing files that changed from the base of the PR and between 91f1ad8 and 9f7b89c.

⛔ Files ignored due to path filters (2)
  • clash-bin/tests/data/config/certs/example.org-key.pem is excluded by !**/*.pem
  • clash-bin/tests/data/config/certs/example.org.pem is excluded by !**/*.pem
📒 Files selected for processing (40)
  • clash-bin/tests/data/config/Country-asn.mmdb
  • clash-bin/tests/data/config/GeoLite2-ASN.mmdb
  • clash-bin/tests/data/config/anytls.json
  • clash-bin/tests/data/config/certs/dns.crt
  • clash-bin/tests/data/config/certs/dns.key
  • clash-bin/tests/data/config/config.test.yaml
  • clash-bin/tests/data/config/dns.yaml
  • clash-bin/tests/data/config/full.yaml
  • clash-bin/tests/data/config/hysteria.json
  • clash-bin/tests/data/config/hysteria2.yaml
  • clash-bin/tests/data/config/shadowquic.yaml
  • clash-bin/tests/data/config/simple.yaml
  • clash-bin/tests/data/config/socks5-auth.json
  • clash-bin/tests/data/config/socks5-noauth.json
  • clash-bin/tests/data/config/socks5.yaml
  • clash-bin/tests/data/config/ss.json
  • clash-bin/tests/data/config/ss.yaml
  • clash-bin/tests/data/config/tor.yaml
  • clash-bin/tests/data/config/tproxy.yaml
  • clash-bin/tests/data/config/trojan-grpc.json
  • clash-bin/tests/data/config/trojan-ws.json
  • clash-bin/tests/data/config/tuic.toml
  • clash-bin/tests/data/config/tun.yaml
  • clash-bin/tests/data/config/uot.yaml
  • clash-bin/tests/data/config/vless-tcp-reality.yaml
  • clash-bin/tests/data/config/vless-ws-tls.json
  • clash-bin/tests/data/config/vless.yaml
  • clash-bin/tests/data/config/vmess-grpc.json
  • clash-bin/tests/data/config/vmess-http2.json
  • clash-bin/tests/data/config/vmess-ws.json
  • clash-bin/tests/data/config/wg.yaml
  • clash-lib/src/proxy/anytls/mod.rs
  • clash-lib/src/proxy/hysteria2/mod.rs
  • clash-lib/src/proxy/shadowquic/mod.rs
  • clash-lib/src/proxy/shadowsocks/outbound/mod.rs
  • clash-lib/src/proxy/socks/outbound/mod.rs
  • clash-lib/src/proxy/trojan/mod.rs
  • clash-lib/src/proxy/tuic/mod.rs
  • clash-lib/src/proxy/vless/mod.rs
  • clash-lib/src/proxy/vmess/mod.rs
💤 Files with no reviewable changes (26)
  • clash-bin/tests/data/config/simple.yaml
  • clash-bin/tests/data/config/anytls.json
  • clash-bin/tests/data/config/hysteria.json
  • clash-bin/tests/data/config/vmess-http2.json
  • clash-bin/tests/data/config/trojan-ws.json
  • clash-bin/tests/data/config/socks5-noauth.json
  • clash-bin/tests/data/config/socks5-auth.json
  • clash-bin/tests/data/config/vmess-ws.json
  • clash-bin/tests/data/config/ss.json
  • clash-bin/tests/data/config/vmess-grpc.json
  • clash-bin/tests/data/config/dns.yaml
  • clash-bin/tests/data/config/trojan-grpc.json
  • clash-bin/tests/data/config/socks5.yaml
  • clash-bin/tests/data/config/vless-ws-tls.json
  • clash-bin/tests/data/config/tor.yaml
  • clash-bin/tests/data/config/wg.yaml
  • clash-bin/tests/data/config/ss.yaml
  • clash-bin/tests/data/config/tproxy.yaml
  • clash-bin/tests/data/config/tuic.toml
  • clash-bin/tests/data/config/vless.yaml
  • clash-bin/tests/data/config/tun.yaml
  • clash-bin/tests/data/config/config.test.yaml
  • clash-bin/tests/data/config/shadowquic.yaml
  • clash-bin/tests/data/config/vless-tcp-reality.yaml
  • clash-bin/tests/data/config/hysteria2.yaml
  • clash-bin/tests/data/config/uot.yaml
✅ Files skipped from review due to trivial changes (1)
  • clash-bin/tests/data/config/full.yaml

Comment on lines +844 to 860
let mut tmp = tempfile::NamedTempFile::new()?;
tmp.write_all(ANYTLS_SERVER_CONFIG.as_bytes())?;

let result = DockerTestRunnerBuilder::new()
.image(IMAGE_SINGBOX)
.cmd(&["run", "-c", "/etc/sing-box/config.json"])
.mounts(&[
(conf.to_str().unwrap(), "/etc/sing-box/config.json"),
(tmp.path().to_str().unwrap(), "/etc/sing-box/config.json"),
(cert.to_str().unwrap(), "/etc/ssl/v2ray/fullchain.pem"),
(key.to_str().unwrap(), "/etc/ssl/v2ray/privkey.pem"),
])
.host_port(host_port, 10002)
.build()
.await
.await;
drop(tmp);
result
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate docker runner source files =="
fd -i 'docker_runner.*\.rs$'

echo
echo "== Inspect builder/runner lifecycle symbols =="
rg -n --type=rust -C6 \
  '\bstruct\s+DockerTestRunnerBuilder\b|\bimpl\s+DockerTestRunnerBuilder\b|\basync\s+fn\s+build\b|\bfn\s+build\b|\basync\s+fn\s+start\b|\bfn\s+start\b|\bmounts\s*\(|\bhost_port\s*\(' \
  $(fd -i 'docker_runner.*\.rs$')

echo
echo "== Inspect where docker process/container start actually occurs =="
rg -n --type=rust -C6 \
  'Command::new\("docker"\)|\bdocker\b.*\brun\b|\bcreate\b|\bstart\b|\bspawn\b' \
  $(fd -i 'docker_runner.*\.rs$')

Repository: Watfaq/clash-rs

Length of output: 4415


🏁 Script executed:

cat -n clash-lib/src/proxy/utils/test_utils/docker_utils/docker_runner.rs | sed -n '100,250p'

Repository: Watfaq/clash-rs

Length of output: 6307


🏁 Script executed:

cat -n clash-lib/src/proxy/utils/test_utils/docker_utils/docker_runner.rs | sed -n '80,180p'

Repository: Watfaq/clash-rs

Length of output: 4445


🏁 Script executed:

rg -n --type=rust -A50 'async fn try_new' clash-lib/src/proxy/utils/test_utils/docker_utils/docker_runner.rs

Repository: Watfaq/clash-rs

Length of output: 2106


🏁 Script executed:

cat -n clash-lib/src/proxy/anytls/mod.rs | sed -n '820,880p'

Repository: Watfaq/clash-rs

Length of output: 2516


🏁 Script executed:

rg -n --type=rust -B10 -A15 'DockerTestRunnerBuilder::new' clash-lib/src/proxy/anytls/mod.rs | head -80

Repository: Watfaq/clash-rs

Length of output: 1124


Keep temp config file alive until container is fully started.

At line 858, drop(tmp) removes the temp config file immediately after build().await. This is unsafe because Docker's start_container() API only signals the container to start—it does not block until the container process has read the config file. The file must remain on the filesystem for the bind mount to work when the container process actually starts.

Move the temp file into the runner state or convert it to a persistent test fixture so it remains alive for the container's lifetime:

  • Store NamedTempFile in DockerTestRunner or wrap the runner to keep the file alive
  • Or write the config to a stable test directory instead of using a temporary file
Current code
        let mut tmp = tempfile::NamedTempFile::new()?;
        tmp.write_all(ANYTLS_SERVER_CONFIG.as_bytes())?;

        let result = DockerTestRunnerBuilder::new()
            .image(IMAGE_SINGBOX)
            .cmd(&["run", "-c", "/etc/sing-box/config.json"])
            .mounts(&[
                (tmp.path().to_str().unwrap(), "/etc/sing-box/config.json"),
                (cert.to_str().unwrap(), "/etc/ssl/v2ray/fullchain.pem"),
                (key.to_str().unwrap(), "/etc/ssl/v2ray/privkey.pem"),
            ])
            .host_port(host_port, 10002)
            .build()
            .await;
        drop(tmp);
        result
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clash-lib/src/proxy/anytls/mod.rs` around lines 844 - 860, The temp file
created with tempfile::NamedTempFile (tmp) is dropped right after
DockerTestRunnerBuilder::build().await, which can remove the bind-mounted config
before the container process reads it; keep the temp file alive for the
container lifetime by moving the NamedTempFile into the runner state instead of
dropping it (e.g. store tmp on the DockerTestRunner struct or return a wrapper
that owns the NamedTempFile), or alternatively write the config to a stable test
fixture directory rather than using NamedTempFile; update the code that
builds/returns the runner (DockerTestRunnerBuilder::build()/DockerTestRunner) to
hold ownership of the file so drop(tmp) is removed and the file remains on disk
while the container runs.

…er certs/ move

- Delete listeners/, public/, rules.yaml, server.yaml, rule-set*.yaml from
  clash-bin/tests/data/config/ — none referenced by any automated test
- Fix clash-lib/tests/data/config/client/rules.yaml: update 6 dns.crt/dns.key
  relative paths to certs/dns.crt and certs/dns.key after the certs/ rename

Final clash-bin/tests/data/config/ layout:
  certs/    — shared TLS certs and keys
  ssh/      — SSH docker test host keys
  wg_config/ — WireGuard docker test keys and config
  empty.yaml — minimal clash config for docker test framework
  full.yaml  — reference config documentation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 26, 2026

📊 Proxy Throughput Results

Transport Payload Upload (Mbps) Download (Mbps)
ss-obfs-http 32 MB 12771.4 11898.7
ss-obfs-tls 32 MB 13996.2 11287.7
ss-plain 32 MB 3882.2 6715.3
ss-shadow-tls-v3 32 MB 15285.6 15563.6
ss-v2ray-plugin-ws-tls 32 MB 3913.8 5105.9

Tests ran 5 variant(s) in parallel; each direction transfers the full payload.

Full test log

Download the throughput-results artifact for the full log.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ibigbug ibigbug merged commit 1c62e02 into master Apr 26, 2026
33 checks passed
@ibigbug ibigbug deleted the feat/config-cleanup branch April 26, 2026 22:38
ibigbug added a commit that referenced this pull request Apr 26, 2026
- Two-way sync barrier prevents TCP coalescing from biasing download timer
- Fix cert paths to use certs/ subdir (post-#1338 cleanup)
- Require container_ip() for .no_port() runners (socks5, ssh, tuic, vless)
- netem sidecar: disable auto_remove to reliably detect exit failure
- format_throughput.py: canonical transport ordering + run URL always appended

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ibigbug added a commit that referenced this pull request Apr 27, 2026
- Two-way sync barrier prevents TCP coalescing from biasing download timer
- Fix cert paths to use certs/ subdir (post-#1338 cleanup)
- Require container_ip() for .no_port() runners (socks5, ssh, tuic, vless)
- netem sidecar: disable auto_remove to reliably detect exit failure
- format_throughput.py: canonical transport ordering + run URL always appended

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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