diff --git a/CLAUDE.md b/CLAUDE.md index d28c039a4..ec4c53954 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -799,7 +799,7 @@ Every event on `/v1/sessions/{id}` and `/v1/events` carries: - `dialAddr` — the address the egress guard checked and the proxy dialed, `"ip:port"`, on a tunnel's open row when `listener.egress_guard` is on. With `host` it is the audit trail against DNS rebinding: under the guard the proxy resolves once, checks that answer, and dials exactly it, so the two fields together say "asked for the name, reached this address". Absent when the guard is off and on rows that dialed nothing. - `httpMethod`, `httpPath` — the HTTP verb and path, so a request no parser recognized is still identifiable rather than showing only a host. Distinct from the `method` inside `a2a` / `mcp`, which is a protocol method name. On an opaque tunnel `httpMethod` is `CONNECT` and `httpPath` is absent — opaque bytes carry no request line. The path is query-stripped and percent-decoded, so query-borne credentials never reach the timeline, but a secret in a path *segment* (a bot token, a webhook path) does survive on this unauthenticated surface — worth knowing before exporting events off-box. - `subagent` — present on rows from a Claude Code subagent or agent-team teammate: the `X-Claude-Code-Agent-Id` the request carried. For a subagent that is also the `` in `~/.claude/projects///subagents/agent-.meta.json`; for a teammate the join is unverified. Resolved by `Context.Subagent`, pinned with the User-Agent by `ResolveClient` so no plugin can re-file a row, and **refused rather than sanitised** when unusable (control runes, over 128 bytes), because it is a join key and a cleaned-up id joins nothing. It does not move the row: a subagent's requests stay in their parent's session, so the header must not be added to `session.id_headers`. **It names an agent, it does not classify the row:** `inference.agentRole` stays the field that says whether a row is a subagent's, and a teammate's row is `agentRole: main` with `subagent` set — Claude Code writes `cc_is_subagent=true` only for `agentType: subagent`. -- `parentSubagent` — the agent that started `subagent`, from `X-Claude-Code-Parent-Agent-Id`, resolved with it in one memo (`Context.ParentSubagent`) and under the same rules. Claude Code sends it only when the spawner was itself an agent, so **absent on a row that names a `subagent` means the conversation started it**, not "unknown". The two keys are the tree's node and its edge. agentop's events pane groups each subagent's rows under a collapsible header anchored at its first row (#1047): the label is the subagent's description from the harvested `agent-.meta.json` (`SessionMetadata.Subagents`), falling back to the raw id; blocks are collapsed by default, `Enter` toggles, a sort disables grouping, and a live search opens every block. +- `parentSubagent` — the agent that started `subagent`, from `X-Claude-Code-Parent-Agent-Id`, resolved with it in one memo (`Context.ParentSubagent`) and under the same rules. Claude Code sends it only when the spawner was itself an agent, so **absent on a row that names a `subagent` means the conversation started it**, not "unknown". The two keys are the tree's node and its edge. agentop's events pane groups each subagent's rows under a collapsible header anchored at its first row (#1047): the label is the subagent's description from the harvested `agent-.meta.json` (`SessionMetadata.Subagents`), falling back to the raw id; blocks are collapsed by default, `Enter` toggles, an expanded block's rows render together under its header (pulled out of the chronological interleaving — the DIR gutter's `│` marks them), member exchanges are numbered from 1 within the block while main rows keep session-wide numbers, a sort disables grouping, and a live search opens every block. ### Invocation action vocabulary diff --git a/cmd/agentop/tui/events_blocks.go b/cmd/agentop/tui/events_blocks.go index f68106ea4..7168ce4f7 100644 --- a/cmd/agentop/tui/events_blocks.go +++ b/cmd/agentop/tui/events_blocks.go @@ -26,23 +26,32 @@ type subagentBlock struct { // has not landed contributes nothing yet, like its cells. tokens int cost float64 + // msgs is the block's exchange count — the unit the # column numbers member + // rows in, so the header's rollup and the last row's number agree. + msgs int // anchored is set when the header has been emitted for this rebuild, so the // header lands at the block's FIRST surviving row and never before it. anchored bool } // headerText is the block's collapsible line: the rollup the issue asked for — -// how many rows, how many tokens, what cost — behind the label. The caller wraps -// it in a one-cell table.Row (rendered as a section row spanning every column) -// and hands the SAME string to the search, so what the header displays is what -// the search matches — the label alone would hide the rollup figures from it. +// how many messages, how many tokens, what cost — behind the label. The caller +// wraps it in a one-cell table.Row (rendered as a section row spanning every +// column) and hands the SAME string to the search, so what the header displays +// is what the search matches — the label alone would hide the rollup figures +// from it. The count is in the # column's unit (exchanges, numbered from 1 +// inside the block), so the rollup and the rows it summarizes cannot disagree. func (b *subagentBlock) headerText(label string, open bool) string { glyph := "▸" if open { glyph = "▾" } - return fmt.Sprintf("%s─ %s · %d rows · %s tok · %s", - glyph, label, len(b.rows), humanizeCount(int64(b.tokens)), formatUSDTotal(b.cost)) + unit := "msgs" + if b.msgs == 1 { + unit = "msg" + } + return fmt.Sprintf("%s─ %s · %d %s · %s tok · %s", + glyph, label, b.msgs, unit, humanizeCount(int64(b.tokens)), formatUSDTotal(b.cost)) } // subagentLabel names a block. The wire carries only the 16-hex id, so a label a diff --git a/cmd/agentop/tui/events_blocks_test.go b/cmd/agentop/tui/events_blocks_test.go index 7d0707484..dc9950c6d 100644 --- a/cmd/agentop/tui/events_blocks_test.go +++ b/cmd/agentop/tui/events_blocks_test.go @@ -2,6 +2,7 @@ package tui import ( "encoding/json" + "slices" "strings" "testing" @@ -80,15 +81,17 @@ func TestBlocks_CollapsedByDefaultWithHeaderAtTheAnchor(t *testing.T) { if b := m.headerRows[3]; b == nil || b.id != "a2" { t.Errorf("header at 3 = %v, want a2", m.headerRows[3]) } - // The header carries the rollup: a1's response holds 900 tokens at $0.10. + // The header carries the rollup: a1 holds one exchange (2 rows) whose + // response carries 900 tokens at $0.10. h := m.eventsTbl.Rows()[1][0] - if !strings.Contains(h, "▸─") || !strings.Contains(h, "2 rows") || !strings.Contains(h, "900 tok") || !strings.Contains(h, "$0.10") { + if !strings.Contains(h, "▸─") || !strings.Contains(h, "1 msg") || !strings.Contains(h, "900 tok") || !strings.Contains(h, "$0.10") { t.Errorf("a1 header = %q", h) } } -// Enter on a header opens the block: member rows appear in time order, indented -// via the DIR gutter, and the header's glyph turns over. Enter again closes it. +// Enter on a header opens the block: member rows appear together under the +// header (see TestBlocks_ExpandedMembersStayTogether), marked via the DIR +// gutter, and the header's glyph turns over. Enter again closes it. func TestBlocks_EnterToggles(t *testing.T) { m := newBlockModel(t, blockEvents()) @@ -127,6 +130,188 @@ func TestBlocks_EnterToggles(t *testing.T) { } } +// An expanded block's rows render TOGETHER, directly under the header — pulled +// out of the chronological interleaving with the main conversation. Live traffic +// interleaves (a subagent's exchange spans several main-conversation turns), and +// rendering members at their own timestamps scattered the block across the pane, +// which read as "expanding did nothing" — the header said 56 rows and the rows +// below it were the main conversation's. +func TestBlocks_ExpandedMembersStayTogether(t *testing.T) { + events := []pipeline.SessionEvent{ + {Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "ete", RequestID: "a1", Subagent: "a1"}, + // The main conversation's row, chronologically INSIDE a1's exchange. + {Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "ete", RequestID: "m1"}, + {Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, Host: "ete", RequestID: "a1", Subagent: "a1"}, + {Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "ete", RequestID: "m2"}, + } + m := newBlockModel(t, events) + + // Collapsed: header anchored at a1's first row, then the main rows. + if got := len(m.eventsTbl.Rows()); got != 3 { + t.Fatalf("collapsed rows = %d, want 3 (header + m1 + m2)", got) + } + + m.eventsTbl.SetCursor(0) + if !m.toggleSubagentBlock() { + t.Fatal("Enter on the header row did not toggle") + } + + // Expanded: header, a1's whole exchange, THEN the main rows — a1's response + // must not sit at its chronological slot below m1. + want := []string{"a1", "a1", "m1", "m2"} + if got := len(m.eventsTbl.Rows()); got != 5 { + t.Fatalf("expanded rows = %d, want 5 (header + a1's pair + m1 + m2)", got) + } + for i, id := range want { + // Row 0 is the header, whose visibleRows entry holds no event. + if rid := m.visibleRows[i+1].event.RequestID; rid != id { + t.Errorf("expanded row %d = %s, want %s", i+1, rid, id) + } + } +} + +// Two subagents running concurrently, each interleaved with the other and with +// the main conversation — and a1's traffic resuming long after a2 finished its +// first exchange. Each block still renders as ONE contiguous segment under its +// own single header: membership is by subagent id over the whole session, not +// by time-contiguity, so interleaving cannot split a block into segments or +// emit a second header for the same agent. +func TestBlocks_ConcurrentSubagentsEachStayOneSegment(t *testing.T) { + req := func(id, sub string) pipeline.SessionEvent { + return pipeline.SessionEvent{Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "ete", RequestID: id, Subagent: sub} + } + resp := func(id, sub string) pipeline.SessionEvent { + return pipeline.SessionEvent{Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, Host: "ete", RequestID: id, Subagent: sub} + } + events := []pipeline.SessionEvent{ + req("a1.1", "a1"), + req("m1", ""), + req("a2.1", "a2"), // a2 starts while a1's first exchange is in flight + resp("a1.1", "a1"), + resp("a2.1", "a2"), + req("m2", ""), + resp("m2", ""), + req("a2.2", "a2"), // a2 continues after main turns + resp("a2.2", "a2"), + req("m3", ""), + resp("m3", ""), + req("a1.2", "a1"), // a1's traffic resumes long after it started + resp("a1.2", "a1"), + req("m4", ""), + } + m := newBlockModel(t, events) + m.subagentOpen = map[string]bool{"a1": true, "a2": true} + m.rebuildEventsTable() + + // Each subagent's rows render in arrival order within its segment. + order := map[string][]string{} + for _, vr := range m.visibleRows { + if vr.event != nil && vr.event.Subagent != "" { + order[vr.event.Subagent] = append(order[vr.event.Subagent], vr.event.RequestID) + } + } + if got := order["a1"]; !slices.Equal(got, []string{"a1.1", "a1.1", "a1.2", "a1.2"}) { + t.Errorf("a1's segment = %v, want its whole exchange run in order", got) + } + if got := order["a2"]; !slices.Equal(got, []string{"a2.1", "a2.1", "a2.2", "a2.2"}) { + t.Errorf("a2's segment = %v, want its whole exchange run in order", got) + } + // A subagent's rendered rows must form ONE contiguous run: their positions + // span exactly their count. A block split into segments — or a second + // header for the same agent — would leave gaps in that span. + for _, sub := range []string{"a1", "a2"} { + var pos []int + for i, vr := range m.visibleRows { + if vr.event != nil && vr.event.Subagent == sub { + pos = append(pos, i) + } + } + if len(pos) == 0 || pos[len(pos)-1]-pos[0]+1 != len(pos) { + t.Errorf("%s's rows at %v, not one contiguous run", sub, pos) + } + } + // Exactly two headers rendered. + if headers := len(m.headerRows); headers != 2 { + t.Errorf("headers = %d, want 2 (one per subagent, however interleaved)", headers) + } +} + +// Member rows number their exchanges from 1 WITHIN the block — the block reads +// as the subagent's own transcript — while main rows keep the session-wide +// numbers. A request and its response still share one number, and the block's +// count agrees with the last number shown inside it. +func TestBlocks_MembersNumberFromOneInsideTheBlock(t *testing.T) { + req := func(id, sub string) pipeline.SessionEvent { + return pipeline.SessionEvent{Direction: pipeline.Outbound, Phase: pipeline.SessionRequest, Host: "ete", RequestID: id, Subagent: sub} + } + resp := func(id, sub string) pipeline.SessionEvent { + return pipeline.SessionEvent{Direction: pipeline.Outbound, Phase: pipeline.SessionResponse, Host: "ete", RequestID: id, Subagent: sub} + } + events := []pipeline.SessionEvent{ + req("m1", ""), resp("m1", ""), // session exchange 1 + req("a1.1", "a1"), resp("a1.1", "a1"), // session 2 — block-local 1 + req("m2", ""), resp("m2", ""), // session 3 + req("a1.2", "a1"), resp("a1.2", "a1"), // session 4 — block-local 2 + } + m := newBlockModel(t, events) + m.eventsTbl.SetCursor(2) // a1's header: m1's pair, then the header + if !m.toggleSubagentBlock() { + t.Fatal("Enter on the header row did not toggle") + } + + // Rendered: m1 req, m1 resp, header, a1's two exchanges, m2's pair. The + // first cell of each EVENT row is the # column; the header row is one cell + // of its own, standing in for the position. + want := []string{"1", "1", "HEADER", "1", "1", "2", "2", "3", "3"} + rows := m.eventsTbl.Rows() + if len(rows) != len(want) { + t.Fatalf("expanded rows = %d, want %d", len(rows), len(want)) + } + for i, n := range want { + if n == "HEADER" { + continue // checked below, with the rollup + } + if got := rows[i][0]; got != n { + t.Errorf("row %d # = %q, want %q", i, got, n) + } + } + // The header's rollup counts exchanges, in the same unit the rows show. + if h := rows[2][0]; !strings.Contains(h, "2 msgs") { + t.Errorf("header = %q, want the exchange count (2 msgs)", h) + } +} + +// Two of one subagent's exchanges that overlap (A starts, B starts, A ends, B +// ends) keep their pairing: each row carries its own exchange's block-local +// number, not the count of exchanges seen before it. +func TestBlocks_OverlappingExchangesKeepTheirNumbers(t *testing.T) { + ev := func(id string, phase pipeline.SessionPhase) pipeline.SessionEvent { + return pipeline.SessionEvent{Direction: pipeline.Outbound, Phase: phase, Host: "ete", RequestID: id, Subagent: "a1"} + } + events := []pipeline.SessionEvent{ + ev("a1.1", pipeline.SessionRequest), ev("a1.2", pipeline.SessionRequest), + ev("a1.1", pipeline.SessionResponse), ev("a1.2", pipeline.SessionResponse), + } + m := newBlockModel(t, events) + m.subagentOpen = map[string]bool{"a1": true} + m.rebuildEventsTable() + + rows := m.eventsTbl.Rows() + if len(rows) != 5 { + t.Fatalf("expanded rows = %d, want 5 (header + 4)", len(rows)) + } + var got []string + for _, r := range rows[1:] { + got = append(got, r[0]) + } + if want := []string{"1", "2", "1", "2"}; !slices.Equal(got, want) { + t.Errorf("# cells = %v, want %v", got, want) + } + if h := rows[0][0]; !strings.Contains(h, "2 msgs") { + t.Errorf("header = %q, want 2 msgs", h) + } +} + // Enter on a message row is still the detail drill-down; only headers toggle. func TestBlocks_EnterOnAMessageOpensDetail(t *testing.T) { m := newBlockModel(t, blockEvents()) @@ -312,8 +497,8 @@ func TestBlocks_RollupFollowsTheInactiveFilter(t *testing.T) { t.Errorf("header for a2 at row %d", i) } } - // a1's header counts the rows expanding it would show: the kept exchange, 2. - if h := m.eventsTbl.Rows()[1][0]; !strings.Contains(h, "2 rows") { + // a1's header counts what expanding it would show: the kept exchange, 1 msg. + if h := m.eventsTbl.Rows()[1][0]; !strings.Contains(h, "1 msg") { t.Errorf("a1 header = %q, want the kept exchange counted", h) } } diff --git a/cmd/agentop/tui/events_columns.go b/cmd/agentop/tui/events_columns.go index a88a5e7ca..eeddf7ef8 100644 --- a/cmd/agentop/tui/events_columns.go +++ b/cmd/agentop/tui/events_columns.go @@ -51,6 +51,12 @@ type cellContext struct { // ids pairs a request event with its response, computed once per rebuild by // computeEventPairs. Carried here rather than recomputed per cell. ids map[*pipeline.SessionEvent]int + // blockIDs renumbers a subagent block's member exchanges from 1 (#1047): the + // block reads as the subagent's own transcript, so its first exchange is 1, + // not the session-wide number the main conversation's rows keep. Nil unless + // blocks were collected this rebuild; a lookup that misses falls through to + // ids, which is what a main row always takes. + blockIDs map[*pipeline.SessionEvent]int // invs is this row's invocations, computed once in the row loop. // // rebuildEventsTable already needs them for the hideInactive check, and @@ -239,8 +245,11 @@ const ( // display order. var eventColumns = []eventColumn{ {id: colIndex, width: 4, defaultOn: true, keep: keepHigh, - desc: "exchange number; a request and its response share one", + desc: "exchange number; a request and its response share one, numbered per subagent block", cell: func(c cellContext) string { + if id, ok := c.blockIDs[c.row.event]; ok { + return strconv.Itoa(id) + } if id, ok := c.ids[c.row.event]; ok { return strconv.Itoa(id) } diff --git a/cmd/agentop/tui/events_pane.go b/cmd/agentop/tui/events_pane.go index 2b13ab863..01675839c 100644 --- a/cmd/agentop/tui/events_pane.go +++ b/cmd/agentop/tui/events_pane.go @@ -187,6 +187,35 @@ func (m *model) rebuildEventsTable() { } else { m.subagentBlocks = nil } + // Member rows number their exchanges WITHIN their block, from 1 (#1047): the + // block is read as the subagent's own transcript, so its first exchange is 1 + // — the session-wide number the main rows keep would start a block at 94 and + // say nothing about how much the subagent itself did. The session-wide ids + // still decide what an exchange IS (a request and its response share one + // number there, and therefore here); only the counting restarts per block. + // Assigned into ctxs here, after the blocks exist and before any cell + // renders — the # column is not a measured one, so measureColumns above never + // read it. + var blockIDs map[*pipeline.SessionEvent]int + for _, b := range m.subagentBlocks { + if blockIDs == nil { + blockIDs = make(map[*pipeline.SessionEvent]int) + } + local := make(map[int]int, len(b.rows)) + for _, j := range b.rows { + e := ctxs[j].row.event + n, ok := local[ids[e]] + if !ok { + n = len(local) + 1 + local[ids[e]] = n + } + blockIDs[e] = n + } + b.msgs = len(local) + } + for i := range ctxs { + ctxs[i].blockIDs = blockIDs + } // keys is the sort value for each row that survived the filters, parallel to // rows and m.visibleRows. Collected inside the loop because the sortKey funcs // take the same cellContext the cells do — including the chronological index the @@ -210,6 +239,32 @@ func (m *model) rebuildEventsTable() { if q != "" { droppedCols = droppedColumns(selectedColumns(m.eventColumns), cols) } + // renderEventRow appends one event's cells to every parallel structure the + // pane keeps — the table's rows, visibleRows, and the search rows — so the + // two places a row can render from (its own loop turn, or its block's + // expansion below) cannot drift apart. + renderEventRow := func(j int) { + cc := ctxs[j] + row := make(table.Row, 0, len(cols)) + for _, c := range cols { + // A measured column's cell was evaluated for this row by measureColumns, + // so it is aligned and shown rather than evaluated a second time. + if vs, ok := measured[c.id]; ok { + row = append(row, c.align(vs[j])) + continue + } + // render, not cell: it sets cc.width from the column AND applies the + // column's declared alignment, which is the same field tableColumns pads + // the heading with. A cell that aligned itself could not tell the header, + // and for the numeric columns it never did. + row = append(row, c.render(cc)) + } + rows = append(rows, row) + m.visibleRows = append(m.visibleRows, cc.row) + if q != "" { + full = append(full, eventSearchRow(cols, droppedCols, cc)) + } + } for j, cc := range ctxs { // hideInactive (the `s` toggle) is off by default — every message is // shown, including passthrough/skip-only ones, per "I should see all @@ -227,9 +282,15 @@ func (m *model) rebuildEventsTable() { // Subagent blocks: a row naming an agent joins that agent's block, whose // header sits at the block's first surviving row. Collapsed (the default, // and always while no search is live) the row is not rendered at all — the - // header's rollup is what a collapsed block says. m.visibleRows keeps an - // empty eventRow for the header so it stays parallel to the rendered rows; - // headerRows is what says a rendered row is a header. + // header's rollup is what a collapsed block says. Expanded, the block's + // rows render TOGETHER, directly under the header, pulled out of the + // chronological interleaving with the main conversation — at their own + // timestamps they scatter across the pane (a subagent's exchange spans + // several main-conversation turns), and the rows under the header are the + // main conversation's, which reads as the expansion doing nothing. The + // timestamp and # columns still carry where each row sat. m.visibleRows + // keeps an empty eventRow for the header so it stays parallel to the + // rendered rows; headerRows is what says a rendered row is a header. if b := m.subagentBlocks[cc.row.event.Subagent]; b != nil { if !b.anchored { b.anchored = true @@ -243,44 +304,21 @@ func (m *model) rebuildEventsTable() { if q != "" { full = append(full, table.SearchRow{Cells: []string{text}}) } + if m.blockOpen(b.id, q != "") { + // b.rows holds exactly the member rows that survived the + // inactive filter, in arrival order — the same population the + // loop below renders — so nothing is double-counted or skipped. + for _, mj := range b.rows { + renderEventRow(mj) + } + } } - if !m.blockOpen(b.id, q != "") { - continue - } + // The member's cells went out with its block's header — or nowhere, + // while the block is collapsed. Either way not here. + continue } - // Cells come from the selected columns, in their order — see - // events_columns.go. Previously this was a positional table.Row literal - // parallel to a []table.Column slice, so inserting a column meant editing - // both in step and a mismatch shifted every later cell under the wrong - // heading. - // - // PHASE carries no bracket glyphs. They were box-drawing corners (┌/│/└) - // meant to visually connect a request to its response, and they could only - // ever be correct for exchanges that NEST. Concurrent requests cross - // instead: A starts, B starts, A ends, B ends — for which a tree has no - // notation, so both rows claimed to contain each other and the output was - // actively misleading. The # column pairs exchanges exactly (by the - // proxy-stamped RequestID), which is what the glyphs approximated. - row := make(table.Row, 0, len(cols)) - for _, c := range cols { - // A measured column's cell was evaluated for this row by measureColumns, - // so it is aligned and shown rather than evaluated a second time. - if vs, ok := measured[c.id]; ok { - row = append(row, c.align(vs[j])) - continue - } - // render, not cell: it sets cc.width from the column AND applies the - // column's declared alignment, which is the same field tableColumns pads - // the heading with. A cell that aligned itself could not tell the header, - // and for the numeric columns it never did. - row = append(row, c.render(cc)) - } - rows = append(rows, row) - m.visibleRows = append(m.visibleRows, cc.row) - if q != "" { - full = append(full, eventSearchRow(cols, droppedCols, cc)) - } + renderEventRow(j) // Keyed from the SAME cellContext that just rendered the row, so the value // sorted on and the value displayed cannot come apart. cc.width stays zero // here — render takes its own copy — and no sortKey reads it. @@ -291,6 +329,10 @@ func (m *model) rebuildEventsTable() { // the same thing in `partner` as it does here. The sort never reads it as a // position in `keys` — only compares two of them — so the filter's gaps are // harmless. + // + // Member rows take the block branch above and never reach this — safe + // because blocks exist only while no sort is active (see the collection + // above), so a row the sort needs cannot be skipped by a block. if sortCol != nil { keys = append(keys, rowKey{val: sortCol.sortKey(cc), exchange: j}) }