diff --git a/cmd/agentop/tui/events_pane.go b/cmd/agentop/tui/events_pane.go index fd2aebfeb..e2e9e9283 100644 --- a/cmd/agentop/tui/events_pane.go +++ b/cmd/agentop/tui/events_pane.go @@ -584,9 +584,10 @@ func (m *model) selectedEventRow() (eventRow, bool) { // buildEventRows turns the chronological event slice into display rows, one // per network message. The only folding is the TLS-bridge CONNECT pair: a // tunnel-open event (opaque, host:port) immediately followed by the decrypted -// inner request for the same host is rendered as a single row keyed on the -// inner request, with the tunnel attached for the detail pane. A non-bridged -// (passthrough) tunnel has no inner request following it, so it stands as its +// inner row for the same host — a request, or a denial when a plugin rejected +// it before any request row — is rendered as a single row keyed on the +// inner row, with the tunnel attached for the detail pane. A non-bridged +// (passthrough) tunnel has no inner row following it, so it stands as its // own row — it IS the whole message. func buildEventRows(events []pipeline.SessionEvent) []eventRow { closed := closedTunnels(events) @@ -654,19 +655,33 @@ func closedTunnels(events []pipeline.SessionEvent) map[string]bool { // host-part (port-insensitive) handles both the standard-443 case (inner host // = "example.com") and the non-standard case (inner host = "example.com:8443"). // +// A redirected inner request also folds: a plugin (inference-router) may have +// moved it to another host after decryption, so inner.Host is the dial target +// and the host the client named — the CONNECT's target — is RequestedHost. +// // Two guards keep unrelated events from folding: -// - inner must be another outbound REQUEST, never a response, so a plain -// request→response exchange isn't mistaken for a bridged pair. +// - inner must be another outbound REQUEST or DENIAL, never a response, so a +// plain request→response exchange isn't mistaken for a bridged pair. A +// denial folds too: the proxy defers a bridged open and records it with +// whichever row comes first, and a pipeline plugin's Reject can land +// first. // - inner must NOT itself be a tunnel-open (Tunnel marker). Two back-to-back // passthrough CONNECTs to the same host (common with connection pooling) // would otherwise fold — hiding one real message and mislabeling the other. func isBridgedInner(tunnel, inner *pipeline.SessionEvent) bool { + if inner.Direction != pipeline.Outbound || + (inner.Phase != pipeline.SessionRequest && inner.Phase != pipeline.SessionDenied) || + isTunnelOpen(inner) { + return false + } host := hostOnly(inner.Host) - return inner.Direction == pipeline.Outbound && - inner.Phase == pipeline.SessionRequest && - !isTunnelOpen(inner) && - host != "" && - host == hostOnly(tunnel.Host) + if host != "" && host == hostOnly(tunnel.Host) { + return true + } + // RequestedHost is present only when a redirect moved the request, so this + // arm cannot fold an unredirected request onto someone else's tunnel. + return inner.RequestedHost != "" && + hostOnly(inner.RequestedHost) == hostOnly(tunnel.Host) } // hostOnly returns the host portion of a "host:port" string, or the input diff --git a/cmd/agentop/tui/events_pane_test.go b/cmd/agentop/tui/events_pane_test.go index ea51008c3..73378e067 100644 --- a/cmd/agentop/tui/events_pane_test.go +++ b/cmd/agentop/tui/events_pane_test.go @@ -377,6 +377,96 @@ func TestBuildEventRows_CollapsesBridgedConnect(t *testing.T) { } } +// TestBuildEventRows_CollapsesBridgedConnectRedirected locks the fold under a +// plugin redirect: the CONNECT named the host the client asked for, the inner +// request went elsewhere, and RequestedHost carries the client's host. Without +// the RequestedHost match arm the open stood as its own blank-method row and +// the timeline read as if traffic still went to the original server. +func TestBuildEventRows_CollapsesBridgedConnectRedirected(t *testing.T) { + events := []pipeline.SessionEvent{ + // CONNECT tunnel-open to the client's configured inference server. + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionRequest, + Host: "ete-litellm.example.com:443", + Tunnel: true, + }, + // Inner request the inference-router redirected to another server: + // Host is the dial target, RequestedHost the host the client named. + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionRequest, + Host: "litellm.cb.example.com", + RequestedHost: "ete-litellm.example.com:443", + Inference: &pipeline.InferenceExtension{Model: "glm-4.6"}, + }, + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionResponse, + Host: "litellm.cb.example.com", + Inference: &pipeline.InferenceExtension{Model: "glm-4.6", TotalTokens: 500}, + StatusCode: 200, + }, + } + rows := buildEventRows(events) + if len(rows) != 2 { + t.Fatalf("redirected bridged call produced %d rows, want 2 (collapsed req + resp)", len(rows)) + } + if rows[0].event.Host != "litellm.cb.example.com" { + t.Errorf("collapsed row host = %q, want the redirected host", rows[0].event.Host) + } + if rows[0].tunnel == nil { + t.Fatalf("row 0 should carry the folded CONNECT tunnel") + } + if rows[0].tunnel.Host != "ete-litellm.example.com:443" { + t.Errorf("folded tunnel host = %q, want the client's original server", rows[0].tunnel.Host) + } +} + +// TestBuildEventRows_CollapsesBridgedConnectDenied locks the fold when the +// tunnel's first decrypted row is a DENIAL, not 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 first row. +// Each subtest covers one match arm: unredirected denial (Host) and redirected +// denial (RequestedHost). +func TestBuildEventRows_CollapsesBridgedConnectDenied(t *testing.T) { + for name, inner := range map[string]pipeline.SessionEvent{ + "unredirected": { + Direction: pipeline.Outbound, + Phase: pipeline.SessionDenied, + Host: "ete-litellm.example.com", + StatusCode: 403, + }, + "redirected": { + Direction: pipeline.Outbound, + Phase: pipeline.SessionDenied, + Host: "litellm.cb.example.com", + RequestedHost: "ete-litellm.example.com:443", + StatusCode: 403, + }, + } { + events := []pipeline.SessionEvent{ + { + Direction: pipeline.Outbound, + Phase: pipeline.SessionRequest, + Host: "ete-litellm.example.com:443", + Tunnel: true, + }, + inner, + } + rows := buildEventRows(events) + if len(rows) != 1 { + t.Fatalf("%s: denied bridged call produced %d rows, want 1 (the denial carrying the tunnel)", name, len(rows)) + } + if rows[0].tunnel == nil { + t.Fatalf("%s: denial row should carry the folded CONNECT tunnel", name) + } + if rows[0].event.Phase != pipeline.SessionDenied { + t.Errorf("%s: folded row phase = %q, want the denial kept", name, rows[0].event.Phase) + } + } +} + // TestBuildEventRows_PassthroughTunnelStandsAlone covers every shape in which a // non-bridged CONNECT tunnel-open must remain its own row rather than fold: // a lone trailing CONNECT, two back-to-back CONNECTs to the SAME host