[huang195]

What

agentop's CONNECT fold skipped requests that a plugin redirected to another inference server, so a redirected session rendered one orphan tunnel-open row per request — blank METHOD, blank STATUS, naming the original server, with no response row (the inner request carries it). The timeline roughly doubled and read as if traffic still went to the host the client named.

Why

isBridgedInner matched the inner request's Host against the tunnel's CONNECT target. A redirected inner row's Host is the dial target (where the bytes went), so the match failed and the open stood alone.

A redirected row carries the client's host in RequestedHost — which is exactly the CONNECT's target — so that is now the second match arm. RequestedHost is present only on a redirected request, so an unredirected request still cannot fold onto someone else's tunnel; the pooling and host-mismatch guards keep their tests.

The fold also covers a tunnel whose first decrypted row is a denial rather than a request — the proxy defers a bridged open and records it with whichever row comes first, and a pipeline plugin's Reject (recorded before any request row) can be that row. Both match arms apply; the server-side timeline (core/session/timeline) already folded this shape, so the events pane now matches it.

Repro (before)

A session pinned to another server via the inference-router produced, per request:

 #     TIME   PHASE  METHOD  STATUS  HOST
 167   …      req                              ete-litellm.ai-mode…   ← orphan tunnel-open
 168   …      req    POST                    litellm.cb.ete.res.…   ← the real request
 168   …      resp   POST    200             litellm.cb.ete.res.…

After, rows 167+168 are one row carrying the tunnel in its detail pane.

Testing

• TestBuildEventRows_CollapsesBridgedConnectRedirected (new): redirected bridged pair folds to one row keyed on the inner request, tunnel attached.
• Existing TestBuildEventRows_CollapsesBridgedConnect / PassthroughTunnelStandsAlone unchanged and passing (the pooling guard caught a bad first draft of this change — the guards are load-bearing).
• go test ./tui/ green, gofmt/go vet clean.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2c81da2d-9895-46c4-a14c-8bd3a991abcf

Commits

Reviewing files that changed from the base of the PR and between 2a9152c and 387aee2.

Files selected for processing: 2

• cmd/agentop/tui/events_pane.go
• cmd/agentop/tui/events_pane_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[clawgenti]

Summary

The fix is correct and narrowly scoped. isBridgedInner now matches on either Host or RequestedHost against the CONNECT target, and it accepts a SessionDenied inner row. That matches what core/session/timeline/hosts.go already does in settle(), so agentop and the server-side timeline now agree. The guard ordering is safe: direction, phase and tunnel-open are checked before either host arm, and pipeline.Context.RequestedHost() returns "" unless a redirect actually moved the request. So the new arm can't fold an unredirected request onto a tunnel for a different host, and the existing pooling and host-mismatch tests still cover the old guards. I checked the denial-first case against tunnelLog.recordWith / recordOutboundRejectIn in the forward proxy. The deferred open really is recorded with whichever row comes first, so folding a denial is correct.

I have no blocking issues, only two small test suggestions inline.

Author: huang195 (MEMBER, maintainer)
Areas reviewed: Go (agentop TUI), tests, and a parity check against core/session/timeline and forwardproxy
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed off: yes
CI status: passing (all required checks green; CodeRabbit/Spellcheck skipped)

[clawgenti]

suggestion: The new arm has no negative test. Consider adding a case to TestBuildEventRows_PassthroughTunnelStandsAlone: a CONNECT to a.example:443 followed by a redirected request with Host: b.example and RequestedHost: c.example:443. It should not fold. Today the RequestedHost != "" / host-equality pairing is only tested for the positive case, so a later change that relaxes this arm (for example, dropping the equality) would still pass every test.

A minor point on parity: the server-side HostKey lowercases the host and strips only :443, while hostOnly is case-sensitive and strips any port. This was already true before this PR, so it isn't blocking. It does mean a mixed-case CONNECT target could fold on the server and not in agentop. An strings.EqualFold here would close that gap if you want it.

[clawgenti]

nit: The loop is over a map without t.Run, so the first t.Fatalf stops the remaining case and the failure shows up as one undifferentiated test. Wrapping the body in t.Run(name, func(t *testing.T) { ... }) lets each match arm pass or fail on its own (and then the %s: prefixes aren't needed). It would also match the table style of PassthroughTunnelStandsAlone below.