Fix(abctl): address CodeRabbit review — tunnel marker, raw method, footer text

Three review findings on PR #527:

- (Major) Don't infer CONNECT tunnels from host/extension shape. With
  unconditional recording, an ordinary unparsed outbound request can look
  like a tunnel-open (no protocol ext + host:port) and get wrongly folded
  with a following same-host request. Add an explicit Tunnel marker to
  SessionEvent, set by recordTunnelOpened, and key abctl's isTunnelOpen on
  it instead of the heuristic. (Drops the now-unused hasPort helper.)

- (Minor) eventMethod truncates to 22 chars for display but was used for
  pairing and search — long names sharing a 22-char prefix mis-paired, and
  truncated suffixes weren't searchable. Add eventMethodValue (raw) for
  logic; keep eventMethod (truncated) for rendering only.

- (Minor) The `s` footer hint said "hide skips" but the toggle also hides
  passthrough/no-plugin messages — now "[s] hide passthru/skip".

Tests: SessionEvent JSON round-trip covers Tunnel; recordTunnelOpened sets
the marker; abctl tunnel tests set Tunnel explicitly.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>

Author: huang195

authbridge/authlib/listener/forwardproxy/transparent.go

@@ -179,6 +179,9 @@ func (s *Server) recordTunnelOpened(pctx *pipeline.Context) {
 		Plugins:     plugins,
 		Identity:    pipeline.SnapshotIdentity(pctx),
 		Host:        pctx.Host,
+		// Explicit opaque-tunnel marker so abctl can fold this CONNECT into
+		// the decrypted inner request without inferring "tunnel" from shape.
+		Tunnel: true,
 	}
 	// Always record the tunnel-open so passthrough/non-bridged tunnels (no
 	// plugin activity) are still visible. For a TLS-bridged call abctl folds

authbridge/authlib/listener/forwardproxy/transparent_test.go

@@ -8,8 +8,33 @@ import (
 
 	"github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline"
 	"github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/plugintesting"
+	"github.com/kagenti/kagenti-extensions/authbridge/authlib/session"
 )
 
+// TestRecordTunnelOpened_SetsTunnelMarker locks the explicit producer marker:
+// a CONNECT / transparent-redirect tunnel-open is recorded with Tunnel=true so
+// consumers (abctl) fold it into the decrypted inner request without inferring
+// "tunnel" from host/extension shape.
+func TestRecordTunnelOpened_SetsTunnelMarker(t *testing.T) {
+	store := session.New(5*time.Minute, 100, 0)
+	defer store.Close()
+	s := &Server{Sessions: store}
+
+	s.recordTunnelOpened(&pipeline.Context{Direction: pipeline.Outbound, Host: "example.com:443"})
+
+	v := store.View(session.DefaultSessionID)
+	if v == nil || len(v.Events) != 1 {
+		t.Fatalf("expected 1 tunnel-open event, got %+v", v)
+	}
+	ev := v.Events[0]
+	if !ev.Tunnel {
+		t.Error("tunnel-open event must have Tunnel=true")
+	}
+	if ev.Direction != pipeline.Outbound || ev.Phase != pipeline.SessionRequest {
+		t.Errorf("tunnel-open = %v/%v, want Outbound/SessionRequest", ev.Direction, ev.Phase)
+	}
+}
+
 // HandleTransparentConn gates then blind-tunnels: with an allow-all pipeline it
 // must dial the recovered destination and copy bytes both ways, emitting no
 // proxy-protocol bytes of its own (the agent thinks it's talking to dst).

authbridge/authlib/pipeline/session.go

@@ -137,6 +137,14 @@ type SessionEvent struct {
 	// and for events recorded before the TLS handshake completed.
 	// Populated by the listener layer; plugins do not write to it.
 	TLS *EventTLS
+
+	// Tunnel marks an opaque CONNECT / transparent-redirect tunnel-open: the
+	// bytes are not HTTP, so there's no protocol parse. Set only by
+	// recordTunnelOpened. Consumers (abctl) use it to fold the tunnel into the
+	// decrypted inner request a TLS bridge produces — an explicit producer
+	// signal rather than inferring "tunnel" from host/extension shape, which
+	// an ordinary unparsed request could otherwise mimic.
+	Tunnel bool
 }
 
 // EventTLS describes the TLS state of a connection that produced a
@@ -215,6 +223,7 @@ type sessionEventWire struct {
 	Host        string                     `json:"host,omitempty"`
 	DurationMs  int64                      `json:"durationMs,omitempty"`
 	TLS         *EventTLS                  `json:"tls,omitempty"`
+	Tunnel      bool                       `json:"tunnel,omitempty"`
 }
 
 func (e SessionEvent) MarshalJSON() ([]byte, error) {
@@ -234,6 +243,7 @@ func (e SessionEvent) MarshalJSON() ([]byte, error) {
 		Host:        e.Host,
 		DurationMs:  e.Duration.Milliseconds(),
 		TLS:         e.TLS,
+		Tunnel:      e.Tunnel,
 	})
 }
 
@@ -261,6 +271,7 @@ func (e *SessionEvent) UnmarshalJSON(data []byte) error {
 		Host:        w.Host,
 		Duration:    time.Duration(w.DurationMs) * time.Millisecond,
 		TLS:         w.TLS,
+		Tunnel:      w.Tunnel,
 	}
 	return nil
 }

authbridge/authlib/pipeline/session_test.go

@@ -79,6 +79,7 @@ func TestSessionEvent_JSONRoundTrip(t *testing.T) {
 		Identity:   &EventIdentity{Subject: "alice", ClientID: "agent-1"},
 		StatusCode: 200,
 		Error:      &EventError{Kind: "upstream", Message: "timeout"},
+		Tunnel:     true,
 	}
 
 	first, err := json.Marshal(orig)

authbridge/cmd/abctl/tui/events_pane.go

@@ -206,14 +206,11 @@ func buildEventRows(events []pipeline.SessionEvent) []eventRow {
 }
 
 // isTunnelOpen reports whether e is a CONNECT / transparent-redirect
-// tunnel-open: an outbound request event with no protocol extension (the
-// bytes are opaque) whose host carries an explicit port. recordTunnelOpened
-// is the only producer of such events.
+// tunnel-open. It keys on the explicit Tunnel marker the producer
+// (recordTunnelOpened) sets — NOT on host/extension shape, which an ordinary
+// unparsed outbound request could mimic and get wrongly folded.
 func isTunnelOpen(e *pipeline.SessionEvent) bool {
-	return e.Direction == pipeline.Outbound &&
-		e.Phase == pipeline.SessionRequest &&
-		e.A2A == nil && e.MCP == nil && e.Inference == nil &&
-		hasPort(e.Host)
+	return e.Tunnel
 }
 
 // isBridgedInner reports whether inner is the decrypted request the TLS bridge
@@ -226,12 +223,9 @@ func isTunnelOpen(e *pipeline.SessionEvent) bool {
 // 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 NOT itself be a tunnel-open. 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. A
-//     genuine decrypted inner request is not opaque-with-a-port, so this only
-//     excludes the rare non-standard-port bridge whose inner had no parser
-//     match (still shown, just as its own row rather than folded).
+//   - 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 {
 	host := hostOnly(inner.Host)
 	return inner.Direction == pipeline.Outbound &&
@@ -250,12 +244,6 @@ func hostOnly(hostport string) string {
 	return hostport
 }
 
-// hasPort reports whether hostport parses as "host:port".
-func hasPort(hostport string) bool {
-	_, _, err := net.SplitHostPort(hostport)
-	return err == nil
-}
-
 // allInvocations returns every plugin invocation on an event, both
 // directions concatenated (inbound first). Returns nil when the event
 // carries no Invocations.
@@ -414,18 +402,28 @@ func shortPhase(p pipeline.SessionPhase) string {
 	return "?"
 }
 
-func eventMethod(e pipeline.SessionEvent) string {
+// eventMethodValue is the raw, untruncated method/model for an event — the A2A
+// method, inference model, or MCP method. Used for logic (pairing, filtering)
+// where truncation would conflate distinct names sharing a 22-char prefix or
+// hide searchable suffixes.
+func eventMethodValue(e pipeline.SessionEvent) string {
 	switch {
 	case e.A2A != nil:
-		return truncStr(e.A2A.Method, 22)
+		return e.A2A.Method
 	case e.Inference != nil:
-		return truncStr(e.Inference.Model, 22)
+		return e.Inference.Model
 	case e.MCP != nil:
-		return truncStr(e.MCP.Method, 22)
+		return e.MCP.Method
 	}
 	return ""
 }
 
+// eventMethod is the display form of the method/model — truncated to the
+// METHOD column width. Render-only; never compare or search on it.
+func eventMethod(e pipeline.SessionEvent) string {
+	return truncStr(eventMethodValue(e), 22)
+}
+
 func statusCell(e pipeline.SessionEvent) string {
 	if e.StatusCode == 0 {
 		return ""
@@ -504,7 +502,7 @@ func computeEventPairs(rows []eventRow) (map[*pipeline.SessionEvent]int, map[int
 			}
 			if ri.Direction != rj.Direction ||
 				hostOnly(ri.Host) != hostOnly(rj.Host) ||
-				eventMethod(*ri) != eventMethod(*rj) {
+				eventMethodValue(*ri) != eventMethodValue(*rj) {
 				continue
 			}
 			partner[i] = j
@@ -697,7 +695,7 @@ func eventMatchesDeny(e *pipeline.SessionEvent) bool {
 // caller identity, and protocol-specific content (A2A parts, MCP error, the
 // inference completion / finish reason).
 func eventHaystack(e *pipeline.SessionEvent) []string {
-	hay := []string{e.Host, eventMethod(*e)}
+	hay := []string{e.Host, eventMethodValue(*e)}
 	for _, iv := range allInvocations(e) {
 		hay = append(hay, iv.Plugin, string(iv.Action), iv.Reason, iv.Path)
 		// Plugin-specific diagnostic context — iterate keys + values so

authbridge/cmd/abctl/tui/events_pane_test.go

@@ -326,6 +326,7 @@ func TestBuildEventRows_CollapsesBridgedConnect(t *testing.T) {
 			Direction: pipeline.Outbound,
 			Phase:     pipeline.SessionRequest,
 			Host:      "api.anthropic.com:443",
+			Tunnel:    true,
 			Invocations: &pipeline.Invocations{
 				Outbound: []pipeline.Invocation{{Plugin: "jwt-validation", Action: pipeline.ActionSkip}},
 			},
@@ -380,7 +381,7 @@ func TestBuildEventRows_CollapsesBridgedConnect(t *testing.T) {
 // by a different-host request (host-mismatch guard).
 func TestBuildEventRows_PassthroughTunnelStandsAlone(t *testing.T) {
 	connect := func(host string) pipeline.SessionEvent {
-		return pipeline.SessionEvent{Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: host}
+		return pipeline.SessionEvent{Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: host, Tunnel: true}
 	}
 	cases := []struct {
 		name   string
@@ -429,7 +430,7 @@ func TestBuildEventRows_TunnelInvocationsFoldIntoRow(t *testing.T) {
 	events := []pipeline.SessionEvent{
 		// CONNECT tunnel-open — an egress gate explicitly ALLOWED it.
 		{
-			Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "api.example.com:443",
+			Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "api.example.com:443", Tunnel: true,
 			Invocations: &pipeline.Invocations{Outbound: []pipeline.Invocation{
 				{Plugin: "egress-policy", Action: pipeline.ActionAllow},
 			}},
@@ -642,21 +643,15 @@ func TestStatusCell(t *testing.T) {
 // TestHostOnly covers port stripping (used by collapse + pairing), including
 // the no-port and IPv6 cases.
 func TestHostOnly(t *testing.T) {
-	cases := []struct {
-		in, want string
-		port     bool
-	}{
-		{"example.com:443", "example.com", true},
-		{"example.com", "example.com", false},
-		{"[::1]:8443", "::1", true},
+	cases := []struct{ in, want string }{
+		{"example.com:443", "example.com"},
+		{"example.com", "example.com"},
+		{"[::1]:8443", "::1"},
 	}
 	for _, tc := range cases {
 		if got := hostOnly(tc.in); got != tc.want {
 			t.Errorf("hostOnly(%q) = %q, want %q", tc.in, got, tc.want)
 		}
-		if got := hasPort(tc.in); got != tc.port {
-			t.Errorf("hasPort(%q) = %v, want %v", tc.in, got, tc.port)
-		}
 	}
 }
 

authbridge/cmd/abctl/tui/keys.go

@@ -456,7 +456,7 @@ func (m *model) helpView() string {
 		}
 		return "[↑↓] nav  [↵] drill  [tab] pipeline  [/] filter  [p] pause  [q] quit"
 	case paneEvents:
-		skipHint := "[s] hide skips"
+		skipHint := "[s] hide passthru/skip"
 		if m.hideInactive {
 			skipHint = "[s] show all"
 		}