Enhance NextTrace API v4 GeoIP support and improve MTR handling - #371
Merged
Conversation
- 仅在 NEXTTRACE_API_V4_TOKEN 存在时接管 LeoMoeAPI - 保留未配置 v4 token 时的 v3 WebSocket 默认路径
- 为 -x 生成 eval/iex 可执行的当前 shell token 设置脚本 - stdout 为终端时避免直接打印 token-bearing 语句
- 将抽象 API v4 文件和符号改为 NextTrace API v4 - 同步 CLI flag、文档和测试命名
- 在 setup 流程中从 stdin/TTY 读取 token - 保持 stdout/stderr 分离并避免回显 token
- 将 -x 从父进程 env 写入脚本调整为 session token 文件 - 启动时从 session token 文件加载 NEXTTRACE_API_V4_TOKEN
- 额外写入 latest token 文件作为不同进程树 fallback - 读取 session token 失败或缺失时尝试 latest token
- 将 v4 lookup timeout 最低提升到 2s - 对网络错误、408 和 5xx 做保守重试
- 让 v4 retry 总耗时受调用方 timeout 限制 - 对成功和错误响应 body 增加读取上限 - 写 token 文件时校验目录并使用同目录临时文件原子替换
- Unix-like 平台继续校验 0700/0600 和 owner - Windows/other 平台仅校验可表达的目录、symlink 和 rename 约束
- Unix-like 平台在缺少 Stat_t 时拒绝 token 目录 - 调整 pipe cleanup 测试避免资源泄漏
- 读取 session/latest token 前拒绝 symlink、目录和宽松权限 - 父目录缺失仍按未保存 session token 处理
- 用专用隐藏输入 reader 处理 Ctrl-C/Ctrl-D/退格 - 中断时返回 130 并恢复终端状态
- nil HTTP client 默认使用带超时的 Geo client - NEXTTRACE_DEBUG 输出 token 文件读取错误和候选路径 - 合并 TTY read/restore 错误并保留 interrupted sentinel
- session token 写成功后 latest fallback 写失败不再导致 setup 失败 - NEXTTRACE_DEBUG 诊断写入 stderr，避免污染 stdout/JSON 输出
fix(trace): 修复 MTR TCP/UDP GeoIP 补全
[codex] 支持 NextTrace API v4 GeoIP token 模式
Cache NextTrace API v4 client for performance improvements
为 API v4 接入 FastIP 预热与直连优化
Split MTR metadata lookups and tune async limits
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough该 PR 添加 NextTrace API v4 支持：交互式会话 token 设置与安全会话文件读写、v4 HTTP GeoIP 客户端（包含 FastIP、重试与缓存）、命令行与服务运行时集成、DNS 名称规范化、以及将 MTR 元数据查询拆分为 Geo/Host 两条路径并配套大量测试。 ChangesNextTrace API v4 Token & GeoIP Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds NextTrace API v4 HTTP GeoIP support (including session-scoped token setup/storage and a dedicated HTTP client with FastIP prewarm), and refactors MTR async metadata handling to better separate GeoIP vs PTR lookups with independent retries and concurrency controls.
Changes:
- Introduce session token setup (
nexttrace -x) and secure temp-file token loading viaNEXTTRACE_API_V4_TOKEN. - Add NextTrace API v4 HTTP GeoIP client with caching, retry behavior, quota parsing, FastIP/GeoDNS integration, and improved token redaction.
- Improve MTR async metadata scheduling (split host vs geo pipelines, bounded contexts, DN42 host/geo ordering, and additional tests).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
util/nexttrace_api_v4_token_file.go |
Implements temp-file token read/write with symlink/perm/owner hardening. |
util/nexttrace_api_v4_token_file_unix.go |
Enforces strict perms and directory ownership checks on Unix. |
util/nexttrace_api_v4_token_file_unix_test.go |
Adds unit test for Unix owner-check error path. |
util/nexttrace_api_v4_token_file_other.go |
Provides non-Unix fallback (no strict perms/owner enforcement). |
util/env.go |
Adds NEXTTRACE_API_V4_TOKEN support, secret-safe env logging, and token file fallback loading. |
util/env_test.go |
Tests secret env logging + token file fallback behavior and symlink/perm rejection. |
util/dns_resolver.go |
Normalizes Geo DNS resolver names and exposes current resolver for cache keys. |
util/dns_resolver_test.go |
Tests resolver name normalization and nested resolver behavior. |
trace/trace.go |
Adds GeoLookupOffset, clamps retry attempt selection, and avoids unnecessary metadata goroutines. |
trace/trace_runtime_test.go |
Ensures “no geo/RDNS” path doesn’t allocate placeholder Geo. |
trace/mtr_scheduler_runtime.go |
Refactors async metadata into separate Geo/Host pipelines with retries + slot limits. |
trace/mtr_scheduler_test.go |
Adds extensive async metadata tests (ordering, DN42 behavior, concurrency, retry semantics). |
trace/mtr_metadata.go |
Splits MTR metadata lookup into host and geo helpers; makes PTR lookup injectable for tests. |
trace/geo_lookup_test.go |
Tests clamping behavior for large geo lookup offsets. |
ipgeo/nexttrace_api_v4.go |
Adds NextTrace API v4 HTTP client, caching/eviction, retry/redaction, FastIP and proxy handling. |
ipgeo/nexttrace_api_v4_test.go |
Comprehensive tests for client behavior (timeouts, retries, redaction, cache eviction, proxy/FastIP). |
ipgeo/ipgeo.go |
Routes LeoMoeAPI source selection to v4 HTTP when token is configured. |
ipgeo/ipgeo_test.go |
Tests Leo source switching based on v4 token configuration. |
internal/service/service.go |
Uses v4 FastIP prewarm instead of Leo WS init when v4 token is present. |
internal/service/service_test.go |
Tests service runtime chooses v4 FastIP path when token is configured. |
cmd/nexttrace_api_v4_token_setup.go |
Implements interactive token setup flow with hidden TTY input. |
cmd/nexttrace_api_v4_token_setup_test.go |
Tests setup behavior, error handling, and no token leakage. |
cmd/cmd.go |
Adds -x/--setup-api-v4-token flag and skips v3 WS init when v4 token is configured. |
cmd/cmd_test.go |
Tests websocket init is skipped and FastIP prewarm is invoked when token is set. |
README.md |
Documents v4 token setup, behavior, and environment variable. |
README_zh_CN.md |
Chinese documentation for v4 token setup and behavior. |
AGENTS.md |
Documents required Conventional Commits policy for the repo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1264
to
+1276
| rootCtx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) | ||
| defer stop() | ||
| util.SrcDev = "" | ||
| if *setupNextTraceAPIV4Token { | ||
| if err := runNextTraceAPIV4TokenSetup(nextTraceAPIV4TokenSetupOptions{ | ||
| stdin: os.Stdin, | ||
| stdout: os.Stdout, | ||
| stderr: os.Stderr, | ||
| }); err != nil { | ||
| os.Exit(handleNextTraceAPIV4TokenSetupError(os.Stderr, err)) | ||
| } | ||
| return | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05787a8935
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+643
to
+644
| func (rt *mtrSchedulerRuntime) metadataRetriesExhausted(attempts map[string]int, ip string) bool { | ||
| return attempts[ip] >= mtrAsyncMetadataMaxRetries+1 |
There was a problem hiding this comment.
The new retry gate permanently suppresses Geo/PTR lookups for an IP once attempts >= mtrAsyncMetadataMaxRetries+1, so a temporary DNS/Geo outage during early probes can leave that hop without metadata for the rest of a long-running MTR session. Because maybeLaunchGeoMetadataLookup/maybeLaunchHostMetadataLookup stop launching once exhausted and the counters are only cleared on success/reset, there is no time-based recovery path anymore (the previous backoff logic retried later). Consider adding a cooldown-based reset of exhausted counters so metadata can recover after transient upstream failures.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@cmd/cmd.go`:
- Around line 888-891: The current branch short-circuits when
ipgeo.NextTraceAPIV4TokenConfigured() is true but ignores errors from
prepareNextTraceAPIV4FastIPFn(ctx, true), which causes skipping the Leo WS
fallback on preheat failure; change the logic so you capture the error returned
by prepareNextTraceAPIV4FastIPFn, and only return nil when that call succeeds,
otherwise log the error (or surface it) and continue with the original WS
initialization path (the existing Leo WS / WS init code) so preheat failures do
not remove the fallback.
In `@internal/service/service.go`:
- Around line 444-448: 当前分支在 ipgeo.NextTraceAPIV4TokenConfigured() 为真时直接忽略
prepareNextTraceAPIV4FastIPFn 的返回值并放弃建立 Leo WS，导致预热失败时没有回退通道；改为接收并检查
prepareNextTraceAPIV4FastIPFn(ctx, false) 的错误返回（例如 err :=
prepareNextTraceAPIV4FastIPFn(...)），若返回非 nil 则调用 ensureLeoMoeConnectionFn(ctx)
回退到 Leo WS，否则按原逻辑继续；保留对 ipgeo.NextTraceAPIV4TokenConfigured() 的判断，只在成功时不调用
ensureLeoMoeConnectionFn。
In `@ipgeo/nexttrace_api_v4.go`:
- Around line 240-251: The proxy-detection logic is inconsistent:
prepareNextTraceAPIV4FastIP skips FastIP warmup when httpproxy.FromEnvironment()
indicates a proxy, but newNextTraceAPIV4HTTPClient only checks util.GetProxy(),
causing dialNextTraceAPIV4 to be installed incorrectly when a proxy exists in
the environment; update newNextTraceAPIV4HTTPClient to call the same detection
(e.g., nextTraceAPIV4ProxyConfigured(endpoint) or httpproxy.FromEnvironment())
and return the client early when a proxy is configured so the FastIP DialContext
branch is not installed (leave util.NewGeoHTTPClient’s Transport.DialContext
handling as-is).
In `@util/env_test.go`:
- Around line 121-143: In TestGetNextTraceAPIV4TokenDebugLogsReadErrorAndPaths,
remove the OS-specific assertion that checks for the exact text "not a
directory" and instead only assert stable log pieces: keep assertions that
output contains EnvNextTraceAPIV4TokenKey, the log prefix "session token file
read failed", and the sessionPath and latestPath variables; ensure you also keep
the NotContains check for the secret token value so platform-dependent
filesystem error messages are not relied upon.
🪄 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: e49c8e5b-c9c1-43f6-951c-72957ba229d4
📒 Files selected for processing (27)
AGENTS.mdREADME.mdREADME_zh_CN.mdcmd/cmd.gocmd/cmd_test.gocmd/nexttrace_api_v4_token_setup.gocmd/nexttrace_api_v4_token_setup_test.gointernal/service/service.gointernal/service/service_test.goipgeo/ipgeo.goipgeo/ipgeo_test.goipgeo/nexttrace_api_v4.goipgeo/nexttrace_api_v4_test.gotrace/geo_lookup_test.gotrace/mtr_metadata.gotrace/mtr_scheduler_runtime.gotrace/mtr_scheduler_test.gotrace/trace.gotrace/trace_runtime_test.goutil/dns_resolver.goutil/dns_resolver_test.goutil/env.goutil/env_test.goutil/nexttrace_api_v4_token_file.goutil/nexttrace_api_v4_token_file_other.goutil/nexttrace_api_v4_token_file_unix.goutil/nexttrace_api_v4_token_file_unix_test.go
Comment on lines
+162
to
+167
| if err := rejectNextTraceAPIV4Symlink(path); err != nil { | ||
| return err | ||
| } | ||
| if err := os.Rename(tmpPath, path); err != nil { | ||
| return err | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
util/nexttrace_api_v4_token_file.go:205
- Same formatting issue here: permissions are reported with %s on a FileMode but the expectation is shown as numeric 0700. Prefer formatting the perm bits in octal (e.g. %04o) for clarity and consistency.
if info.Mode().Perm() != 0o700 {
return fmt.Errorf("NextTrace API v4 token directory permissions are %s, want 0700", info.Mode().Perm())
}
| return err | ||
| } | ||
| if strictNextTraceAPIV4TokenPerms() && info.Mode().Perm() != 0o700 { | ||
| return fmt.Errorf("NextTrace API v4 token directory permissions are %s, want 0700", info.Mode().Perm()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
新增功能
文档
测试
增强