Skip to content

Commit 32162f5

Browse files
fix: pipe selection, CDP debugger stability, and E2E tests (#9)
- Pipe selection: try every discovered pipe with getInfo health check - CDP stability: detach-before-attach with auto-retry on detach errors - Debug logging: optional BRIDGE_DEBUG_LOG env var for MCP debugging - Tests: client and MCP E2E tests for session lifecycle and retry paths - CI: migrate golangci-lint config to v2 format
2 parents eadafdc + 986f5e8 commit 32162f5

11 files changed

Lines changed: 584 additions & 74 deletions

File tree

.golangci.yml

Lines changed: 44 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,63 @@
1+
version: "2"
2+
13
run:
2-
timeout: 3m
34
tests: true
4-
# Pin the analysis target below the latest released golangci-lint
5-
# toolchain (currently 1.24). The project's go.mod declares 1.26 for
6-
# the runtime toolchain, but no 1.25+ syntax is used; this just stops
7-
# golangci-lint from refusing to load the config.
8-
go: "1.24"
95

106
linters:
11-
disable-all: true
7+
default: none
128
enable:
139
- errcheck
14-
- gosimple
1510
- govet
1611
- ineffassign
1712
- staticcheck
1813
- unused
1914
- misspell
2015
- unconvert
2116
- revive
17+
settings:
18+
errcheck:
19+
check-type-assertions: false
20+
exclude-functions:
21+
- encoding/json.Unmarshal
22+
- encoding/json.Marshal
23+
- (*encoding/json.Encoder).Encode
24+
- fmt.Fprintln
25+
- fmt.Fprintf
26+
- fmt.Println
27+
- fmt.Printf
28+
revive:
29+
rules:
30+
- name: blank-imports
31+
- name: context-as-argument
32+
- name: error-return
33+
- name: error-strings
34+
- name: error-naming
35+
- name: exported
36+
arguments: ["disableStutteringCheck"]
37+
- name: increment-decrement
38+
- name: var-declaration
39+
- name: range
40+
- name: receiver-naming
41+
- name: time-naming
42+
- name: unexported-return
43+
- name: indent-error-flow
44+
- name: errorf
45+
- name: empty-block
46+
- name: superfluous-else
47+
- name: unused-parameter
48+
exclusions:
49+
rules:
50+
- path: _test\.go
51+
linters:
52+
- errcheck
53+
- revive
54+
- unused
55+
56+
formatters:
57+
enable:
2258
- gofmt
2359
- goimports
2460

25-
linters-settings:
26-
errcheck:
27-
check-type-assertions: false
28-
exclude-functions:
29-
- encoding/json.Unmarshal
30-
- encoding/json.Marshal
31-
- (*encoding/json.Encoder).Encode
32-
- fmt.Fprintln
33-
- fmt.Fprintf
34-
- fmt.Println
35-
- fmt.Printf
36-
37-
revive:
38-
rules:
39-
- name: blank-imports
40-
- name: context-as-argument
41-
- name: error-return
42-
- name: error-strings
43-
- name: error-naming
44-
- name: exported
45-
arguments: ["disableStutteringCheck"]
46-
- name: increment-decrement
47-
- name: var-declaration
48-
- name: range
49-
- name: receiver-naming
50-
- name: time-naming
51-
- name: unexported-return
52-
- name: indent-error-flow
53-
- name: errorf
54-
- name: empty-block
55-
- name: superfluous-else
56-
- name: unused-parameter
57-
5861
issues:
59-
exclude-rules:
60-
- path: _test\.go
61-
linters:
62-
- errcheck
63-
- revive
64-
- unused
6562
max-issues-per-linter: 0
6663
max-same-issues: 0

cmd/bridge/main.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"flag"
66
"fmt"
7+
"io"
78
"log"
89
"os"
910

@@ -27,6 +28,14 @@ func main() {
2728

2829
logger := log.New(os.Stderr, "[codex-bridge] ", log.LstdFlags)
2930

31+
// If BRIDGE_DEBUG_LOG is set, also tee logs to that file
32+
if debugPath := os.Getenv("BRIDGE_DEBUG_LOG"); debugPath != "" {
33+
if f, err := os.OpenFile(debugPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644); err == nil {
34+
logger = log.New(io.MultiWriter(os.Stderr, f), "[codex-bridge] ", log.LstdFlags)
35+
defer func() { _ = f.Close() }()
36+
}
37+
}
38+
3039
switch *mode {
3140
case "discover":
3241
runDiscover()
@@ -55,21 +64,25 @@ func runDiscover() {
5564
}
5665

5766
func runMCP(pipeName string, logger *log.Logger) {
67+
logger.Println("runMCP starting, discovering pipes...")
5868
c, err := client.Connect(pipeName, logger)
5969
if err != nil {
70+
logger.Printf("Failed to connect: %v", err)
6071
fmt.Fprintf(os.Stderr, "Failed to connect: %v\n", err)
6172
os.Exit(1)
6273
}
63-
defer c.Close()
74+
defer func() { _ = c.Close() }()
6475

65-
logger.Println("Connected to Codex browser pipe")
76+
logger.Println("Connected to Codex browser pipe, starting MCP server...")
6677

6778
srv := mcp.NewMCPServer(c)
6879
srv.SetVersion(version)
6980
if err := srv.Run(); err != nil {
81+
logger.Printf("MCP server error: %v", err)
7082
fmt.Fprintf(os.Stderr, "MCP server error: %v\n", err)
7183
os.Exit(1)
7284
}
85+
logger.Println("MCP server exited normally")
7386
}
7487

7588
func runCLI(pipeName string, logger *log.Logger) {
@@ -78,7 +91,7 @@ func runCLI(pipeName string, logger *log.Logger) {
7891
fmt.Fprintf(os.Stderr, "Failed to connect: %v\n", err)
7992
os.Exit(1)
8093
}
81-
defer c.Close()
94+
defer func() { _ = c.Close() }()
8295

8396
fmt.Println("Connected to Codex browser pipe")
8497
fmt.Println("Commands: tabs, create, close <id>, user-tabs, claim <id>, nav <id> <url>,")

internal/client/browser.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"fmt"
66
"strconv"
7+
"strings"
78
"time"
89
)
910

@@ -219,12 +220,40 @@ func (c *Client) attachTab(tabID int) error {
219220
return err
220221
}
221222

222-
// cdpWithAttach attaches the debugger and then executes a CDP command.
223+
// detachTab detaches the debugger from a tab.
224+
func (c *Client) detachTab(tabID int) error {
225+
_, err := c.SendRequest("detach", map[string]interface{}{
226+
"tabId": tabID,
227+
})
228+
return err
229+
}
230+
231+
// cdpWithAttach ensures the debugger is attached and then executes a CDP command.
232+
// It first tries to detach (to clear any stale attachment state), then attach
233+
// fresh, then execute. If execute fails with "not attached", it retries once.
223234
func (c *Client) cdpWithAttach(tabID int, method string, params map[string]interface{}) (json.RawMessage, error) {
235+
// Detach first to clear any stale debugger state from Chrome
236+
_ = c.detachTab(tabID)
224237
if err := c.attachTab(tabID); err != nil {
225238
return nil, fmt.Errorf("attach failed for tab %d: %w", tabID, err)
226239
}
227-
return c.executeCdp(tabID, method, params)
240+
raw, err := c.executeCdp(tabID, method, params)
241+
if err != nil {
242+
// If attach didn't take, try one more detach+attach+retry cycle
243+
if isDebuggerError(err) {
244+
_ = c.detachTab(tabID)
245+
if err2 := c.attachTab(tabID); err2 != nil {
246+
return nil, fmt.Errorf("retry attach failed for tab %d: %w", tabID, err2)
247+
}
248+
return c.executeCdp(tabID, method, params)
249+
}
250+
return nil, err
251+
}
252+
return raw, nil
253+
}
254+
255+
func isDebuggerError(err error) bool {
256+
return err != nil && strings.Contains(err.Error(), "not attached")
228257
}
229258

230259
// --- Playwright API (via CDP) ---

internal/client/browser_rpc_test.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,26 +72,29 @@ func TestNavigateSendsCDPNavigate(t *testing.T) {
7272
}
7373

7474
calls := rec.snapshot()
75-
if len(calls) < 2 {
76-
t.Fatalf("expected attach + executeCdp, got %d calls", len(calls))
75+
if len(calls) < 3 {
76+
t.Fatalf("expected detach + attach + executeCdp, got %d calls: %v", len(calls), calls)
7777
}
78-
if calls[0].method != "attach" {
79-
t.Errorf("first call %q, want attach", calls[0].method)
78+
if calls[0].method != "detach" {
79+
t.Errorf("first call %q, want detach", calls[0].method)
8080
}
81-
if calls[1].method != "executeCdp" {
82-
t.Errorf("second call %q, want executeCdp", calls[1].method)
81+
if calls[1].method != "attach" {
82+
t.Errorf("second call %q, want attach", calls[1].method)
8383
}
84-
target, _ := calls[1].params["target"].(map[string]interface{})
84+
if calls[2].method != "executeCdp" {
85+
t.Errorf("third call %q, want executeCdp", calls[2].method)
86+
}
87+
target, _ := calls[2].params["target"].(map[string]interface{})
8588
if target == nil {
86-
t.Fatalf("executeCdp missing nested target: %+v", calls[1].params)
89+
t.Fatalf("executeCdp missing nested target: %+v", calls[2].params)
8790
}
8891
if got, ok := target["tabId"].(float64); !ok || int(got) != 17 {
8992
t.Errorf("target.tabId = %v, want 17", target["tabId"])
9093
}
91-
if calls[1].params["method"] != "Page.navigate" {
92-
t.Errorf("CDP method = %v, want Page.navigate", calls[1].params["method"])
94+
if calls[2].params["method"] != "Page.navigate" {
95+
t.Errorf("CDP method = %v, want Page.navigate", calls[2].params["method"])
9396
}
94-
cmd, _ := calls[1].params["commandParams"].(map[string]interface{})
97+
cmd, _ := calls[2].params["commandParams"].(map[string]interface{})
9598
if cmd["url"] != "https://example.com" {
9699
t.Errorf("commandParams.url = %v", cmd["url"])
97100
}

internal/client/client.go

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ type Client struct {
3333
}
3434

3535
// Connect dials a named pipe and returns a ready-to-use Client.
36-
// If pipeName is empty, auto-discovers the first codex-browser-use-* pipe.
36+
// If pipeName is empty, auto-discovers codex-browser-use-* pipes and tries each
37+
// one until a successful connection is made.
3738
func Connect(pipeName string, logger *log.Logger) (*Client, error) {
3839
if pipeName == "" {
3940
pipes, err := discovery.DiscoverCodexPipes()
@@ -48,18 +49,46 @@ func Connect(pipeName string, logger *log.Logger) (*Client, error) {
4849
" 3. Codex Chrome Extension is installed and enabled\n" +
4950
" 4. The extension has connected to Codex Desktop (open a Codex chat once to trigger initialization)")
5051
}
51-
pipeName = pipes[0].Name
52-
if logger != nil {
53-
logger.Printf("auto-discovered pipe: %s", pipeName)
52+
53+
// Try each pipe until one connects AND passes health check
54+
var lastErr error
55+
for _, p := range pipes {
56+
path := discovery.PipePath(p.Name)
57+
conn, err := dialNamedPipe(path)
58+
if err != nil {
59+
lastErr = err
60+
if logger != nil {
61+
logger.Printf("pipe %s failed: %v, trying next...", p.UUID, err)
62+
}
63+
continue
64+
}
65+
c := NewFromConn(conn, logger)
66+
// Health check: send a quick getInfo to verify the pipe is usable
67+
result, err := c.SendRequest("getInfo", nil)
68+
if err != nil {
69+
_ = c.Close()
70+
lastErr = err
71+
if logger != nil {
72+
logger.Printf("pipe %s health check failed: %v, trying next...", p.UUID, err)
73+
}
74+
continue
75+
}
76+
if logger != nil {
77+
logger.Printf("auto-discovered pipe: %s (verified, info=%s)", p.Name, truncate(string(result), 120))
78+
}
79+
return c, nil
5480
}
81+
return nil, fmt.Errorf("all %d pipes failed; last error: %w. "+
82+
"Try: restart Codex Desktop, then re-open the Codex Chrome Extension",
83+
len(pipes), lastErr)
5584
}
5685

5786
path := discovery.PipePath(pipeName)
5887
conn, err := dialNamedPipe(path)
5988
if err != nil {
60-
return nil, fmt.Errorf("dial pipe %s: %w\n"+
61-
"This usually means the pipe is stale (Codex Desktop restarted) or the extension lost its host.\n"+
62-
"Try: restart Codex Desktop, then re-open the Codex Chrome Extension.", path, err)
89+
return nil, fmt.Errorf("dial pipe %s: %w. "+
90+
"This usually means the pipe is stale (Codex Desktop restarted) or the extension lost its host. "+
91+
"Try: restart Codex Desktop, then re-open the Codex Chrome Extension", path, err)
6392
}
6493

6594
return NewFromConn(conn, logger), nil

0 commit comments

Comments
 (0)