Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ refs/*
report.json
.runtime-cache/*
coverage*.out
mock_upstream
internal/api/modules/amp/*.tmp

# Storage backends
Expand Down
448 changes: 0 additions & 448 deletions coverage-amp-target.out

This file was deleted.

450 changes: 0 additions & 450 deletions coverage-core-auth-proxy.out

This file was deleted.

5,681 changes: 0 additions & 5,681 deletions coverage-critical.out

This file was deleted.

448 changes: 0 additions & 448 deletions coverage.audit.out

This file was deleted.

36 changes: 36 additions & 0 deletions docs/debug-runbook.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,42 @@ body, _ = sjson.DeleteBytes(body, "max_output_tokens")

---

### Bug-3:Codex WebUI 登录回调端口复用不稳定,重复授权会撞端口

**症状**:

- 管理端 `GET /codex-auth-url?is_webui=true` 在 WebUI 场景下偶发返回端口占用
- 同一轮登录还没完全释放时再次触发授权,可能出现 `1455` 端口冲突
- 已经存在的回调转发器有时会被新请求直接顶掉,导致前一轮会话提早失效

**根因**:

- `internal/api/handlers/management/auth_files.go` 里的 Codex callback forwarder 以前按“谁后到谁重建”处理
- 这会让同一 provider、同一 target 的重复授权请求反复重建监听器,而不是共享同一个本地回调转发器
- 结果就是:本来应该“共用一个门口排队”的请求,变成“每来一个人就拆掉旧门再装新门”

**修复**:

| 文件 | 修改 |
|------|------|
| `internal/api/handlers/management/auth_files.go` | `startCallbackForwarder` 对同 provider + target 改为复用已有 forwarder,并通过 `refs` 计数管理生命周期 |
| `internal/api/handlers/management/auth_files.go` | `stopCallbackForwarderInstance` 仅在最后一个引用释放时真正关闭监听器 |
| `internal/api/handlers/management/auth_files_codex_webui_test.go` | 新增端口占用冲突测试与 shared lifecycle 测试,覆盖“重复授权仍可复用同一 forwarder”的场景 |

**最小验证**:

```bash
go test ./internal/api/handlers/management/...
```

验证重点:

- 当 `1455` 已被占用时,管理端仍应返回明确的 `409 Conflict`
- 当同一 Codex WebUI 回调目标被重复申请时,forwarder 必须复用,而不是重建
- 只有最后一个引用释放后,本地回调监听器才允许真正关闭

---

## 6. 交付要求

调试类改动必须附带:
Expand Down
47 changes: 37 additions & 10 deletions internal/api/handlers/management/auth_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ const (

type callbackForwarder struct {
provider string
target string
server *http.Server
done chan struct{}
refs int
}

var (
Expand Down Expand Up @@ -121,14 +123,26 @@ func isAddressAlreadyInUse(err error) bool {

func startCallbackForwarder(port int, provider, targetBase string) (*callbackForwarder, error) {
callbackForwardersMu.Lock()
prev := callbackForwarders[port]
if prev != nil {
if existing := callbackForwarders[port]; existing != nil {
if existing.provider == provider && existing.target == targetBase {
select {
case <-existing.done:
delete(callbackForwarders, port)
callbackForwardersMu.Unlock()
stopForwarderInstance(port, existing)
default:
existing.refs++
refs := existing.refs
callbackForwardersMu.Unlock()
log.Infof("callback forwarder for %s reusing %s (refs=%d)", provider, fmt.Sprintf("127.0.0.1:%d", port), refs)
return existing, nil
}
}
delete(callbackForwarders, port)
}
callbackForwardersMu.Unlock()

if prev != nil {
stopForwarderInstance(port, prev)
callbackForwardersMu.Unlock()
stopForwarderInstance(port, existing)
} else {
callbackForwardersMu.Unlock()
}

addr := fmt.Sprintf("127.0.0.1:%d", port)
Expand Down Expand Up @@ -166,8 +180,10 @@ func startCallbackForwarder(port int, provider, targetBase string) (*callbackFor

forwarder := &callbackForwarder{
provider: provider,
target: targetBase,
server: srv,
done: done,
refs: 1,
}

callbackForwardersMu.Lock()
Expand All @@ -184,12 +200,23 @@ func stopCallbackForwarderInstance(port int, forwarder *callbackForwarder) {
return
}
callbackForwardersMu.Lock()
if current := callbackForwarders[port]; current == forwarder {
delete(callbackForwarders, port)
current := callbackForwarders[port]
if current != forwarder {
callbackForwardersMu.Unlock()
return
}
if current.refs > 1 {
current.refs--
refs := current.refs
provider := current.provider
callbackForwardersMu.Unlock()
log.Infof("callback forwarder for %s on port %d retained for shared sessions (refs=%d)", provider, port, refs)
return
}
delete(callbackForwarders, port)
callbackForwardersMu.Unlock()

stopForwarderInstance(port, forwarder)
stopForwarderInstance(port, current)
}

func stopForwarderInstance(port int, forwarder *callbackForwarder) {
Expand Down
84 changes: 84 additions & 0 deletions internal/api/handlers/management/auth_files_codex_webui_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/gin-gonic/gin"
internalconfig "github.com/router-for-me/CLIProxyAPI/v6/internal/config"
Expand Down Expand Up @@ -52,3 +53,86 @@ func TestRequestCodexToken_WebUIPortInUseReturnsConflict(t *testing.T) {
t.Fatalf("error message = %q, want contains callback port", body.Error)
}
}

func TestCallbackForwarderSharedLifecycleKeepsListenerUntilLastRelease(t *testing.T) {
port := reserveFreePort(t)
target := "http://127.0.0.1:28317/codex/callback"

forwarderA, err := startCallbackForwarder(port, "codex", target)
if err != nil {
t.Fatalf("startCallbackForwarder(first): %v", err)
}
t.Cleanup(func() {
stopCallbackForwarderInstance(port, forwarderA)
})

forwarderB, err := startCallbackForwarder(port, "codex", target)
if err != nil {
t.Fatalf("startCallbackForwarder(second): %v", err)
}
t.Cleanup(func() {
stopCallbackForwarderInstance(port, forwarderB)
})
if forwarderA != forwarderB {
t.Fatalf("expected shared forwarder instance, got %p and %p", forwarderA, forwarderB)
}

assertForwarderResponds(t, port)

stopCallbackForwarderInstance(port, forwarderA)
assertForwarderResponds(t, port)

stopCallbackForwarderInstance(port, forwarderB)
assertForwarderStops(t, port)
}

func reserveFreePort(t *testing.T) int {
t.Helper()
listener, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("net.Listen(reserveFreePort): %v", err)
}
defer listener.Close()

addr, ok := listener.Addr().(*net.TCPAddr)
if !ok {
t.Fatalf("listener.Addr() = %T, want *net.TCPAddr", listener.Addr())
}
return addr.Port
}

func assertForwarderResponds(t *testing.T, port int) {
t.Helper()

client := &http.Client{
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
},
Timeout: 2 * time.Second,
}
resp, err := client.Get(fmt.Sprintf("http://127.0.0.1:%d/auth/callback?state=test", port))
if err != nil {
t.Fatalf("GET forwarder: %v", err)
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusFound {
t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusFound)
}
}

func assertForwarderStops(t *testing.T, port int) {
t.Helper()

deadline := time.Now().Add(2 * time.Second)
for time.Now().Before(deadline) {
conn, err := net.DialTimeout("tcp", fmt.Sprintf("127.0.0.1:%d", port), 200*time.Millisecond)
if err != nil {
return
}
_ = conn.Close()
time.Sleep(50 * time.Millisecond)
}

t.Fatalf("port %d still accepts connections after releasing the last forwarder reference", port)
}
Binary file removed mock_upstream
Binary file not shown.
Loading