Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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: 39 additions & 8 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 @@ -120,15 +122,31 @@ func isAddressAlreadyInUse(err error) bool {
}

func startCallbackForwarder(port int, provider, targetBase string) (*callbackForwarder, error) {
var stale *callbackForwarder

callbackForwardersMu.Lock()
prev := callbackForwarders[port]
if prev != nil {
delete(callbackForwarders, port)
if existing := callbackForwarders[port]; existing != nil {
if existing.provider == provider && existing.target == targetBase {
select {
case <-existing.done:
delete(callbackForwarders, port)
stale = 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
}
} else {
delete(callbackForwarders, port)
stale = existing
}
}
callbackForwardersMu.Unlock()

if prev != nil {
stopForwarderInstance(port, prev)
if stale != nil {
stopForwarderInstance(port, stale)
}

addr := fmt.Sprintf("127.0.0.1:%d", port)
Expand Down Expand Up @@ -166,8 +184,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 +204,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
120 changes: 120 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,122 @@ 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 TestStartCallbackForwarderReplacesClosedSharedForwarder(t *testing.T) {
port := reserveFreePort(t)
target := "http://127.0.0.1:29317/codex/callback"

staleDone := make(chan struct{})
close(staleDone)
stale := &callbackForwarder{
provider: "codex",
target: target,
done: staleDone,
}

callbackForwardersMu.Lock()
callbackForwarders[port] = stale
callbackForwardersMu.Unlock()
t.Cleanup(func() {
callbackForwardersMu.Lock()
delete(callbackForwarders, port)
callbackForwardersMu.Unlock()
})

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

if fresh == stale {
t.Fatalf("expected a fresh forwarder instance, got stale pointer %p", fresh)
}

assertForwarderResponds(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