Fix: Make Enter in the agents picker always scope to the highlighted row

[huang195]

Summary

In agentop's AGENTS picker, Enter on the agent already scoped cleared the scope instead of applying it again. So one row meant "show me everything" while every other row meant "show me this agent". Picking claude-code while already scoped to it switched to every agent's sessions and a spend band that included Bobcoins. The footer did say [↵] all agents on that row, but it read as a description of the current view, not as what the key would do.

Enter now always applies the highlighted row, and a new first row, All agents, is the one way to clear the scope.

 AGENT          SESSIONS  REQUESTS  TOKENS  COST
▸All agents
 claude-code    14        2,705     417M    $253.76
 bob-shell      1         5         20k     0.04 Bobcoi…
 Other          1         0         0       —
[↑↓] nav  [↵] all agents  [esc] back  [?] keys  [q] quit

Changes

• tui/agents_pane.go: rebuildAgentsTable adds the All agents row. selectedAgentLabel becomes selectedAgentScope() (scope string, ok bool), where cursor 0 is "".
  • The row is added to the table, not to pickerRows. pickerRows stays the partition of the sessions list that agentsPaneApplies and the Other row reason about, and this row partitions nothing.
  • Its figures are blank. The band above already shows the window's total, and summed across agents billing in different units it would only read as mixed.
• tui/keys.go: Enter sets agentScope to the highlighted row's scope, and the toggle branch is gone. The footer reads [↵] all agents only on the All agents row and [↵] scope to this agent on every agent row, including the scoped one.
• tui/help_overlay.go, README.md: the binding now reads "scope to this agent; All agents clears".

Behaviour change at startup: the picker's cursor now opens on All agents, so Enter there keeps every agent in view, the same as esc. Picking an agent at startup takes one ↓ first.

Tests

• TestAgentsPane_EnterOnTheScopedAgentClearsTheScope is replaced by …KeepsTheScope. There are also new TestAgentsPane_EnterOnAllAgentsClearsTheScope and TestAgentsPane_FooterSaysWhatEnterWillShow (covering the scoped agent, another agent and All agents).
• Both new behaviour tests failed before the fix: Enter re-scoped instead of keeping the scope, and row 0 was an agent.
• Tests that index picker rows or cursor positions are shifted by one for the new row. None of them changed what they assert.
• GOWORK=off go vet ./... and go test ./... pass for cmd/agentop, and gofmt -l is clean. As in Fix: Add IBM Bob's rate to configs written before the preset had it #1219, TestRunExec_BeforeFirstStartRunsAndSaysWhatIsLost fails in a shell with SSL_CERT_FILE set by an installed Cortex and passes with it unset; that is unrelated to this change.

Also run against a live local proxy by driving the built binary through a pty. Picking claude-code, then pressing Enter on it again, stayed on agent=claude-code with a USD-only band. Enter on All agents brought back the bob-shell session and the Bobcoins.

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 25 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

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: aac11588-b61c-4cde-9ff1-4f3359fe3596

📥 Commits

Reviewing files that changed from the base of the PR and between 6d1acb9 and 910f1e6.

📒 Files selected for processing (8)

• cmd/agentop/README.md
• cmd/agentop/tui/agent_scope_test.go
• cmd/agentop/tui/agents_other_test.go
• cmd/agentop/tui/agents_pane.go
• cmd/agentop/tui/agents_pane_test.go
• cmd/agentop/tui/agents_scope_test.go
• cmd/agentop/tui/help_overlay.go
• cmd/agentop/tui/keys.go

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

Autopilot is currently an internal CodeRabbit preview.

---

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.}

[mrsabath]

Good fix for a genuine UX trap, and the reasoning in the new allAgentsLabel doc comment is the kind that makes the next change to this pane safe.

What I verified independently:

• The off-by-one is correct. selectedAgentScope maps i == 0 to the cleared scope and i in [1, len(picker)] to picker[i-1], with the upper bound as i > len(picker) rather than >= — right, since index len(picker) is the last valid element. All eight shifted test fixtures line up.
• The row placement is the load-bearing decision, and it is right. agentsPaneApplies gates on m.agents, not on table rows. Had All agents gone into pickerRows instead, a single-agent deployment would have had two rows and the picker would have begun opening at startup for every user — precisely the cost that function's doc says it exists to avoid. Putting it in rebuildAgentsTable, one layer further out than the Other row, is coherent: Other partitions the sessions list, All agents partitions nothing.
• No stale callers. Both production callers of the renamed selectedAgentLabel are updated, and nothing else referenced it.

Tests are assertive rather than decorative: EnterOnAllAgentsClearsTheScope checks both the first row's label and len(m.agents)+1, which guards the off-by-one directly.

One small test-coverage regression

The old TestAgentsPane_FooterSaysWhichWayEnterWillGo carried a negative assertion — an unscoped row must not say "all agents". The replacement FooterSaysWhatEnterWillShow is positive-only, so it would still pass if the footer somehow said both strings at once. Cheap to restore as a notWant field on the table.

On the startup change

The cursor opening on All agents is documented in both the README and the commit message, and it is defensible — ↵ and Esc now agree on that row. Worth watching for feedback, since picking an agent at startup is the common case and now costs an extra ↓.

---

Areas reviewed: Go (agentop TUI: picker rows, key handling, footer, help overlay), tests, docs, commit conventions
Commits: 1, signed off ✅
CI status: all passing (26 pass, 1 skipped)

[mrsabath]

suggestion: this row is now index 0, and the cursor is preserved across rebuilds by index — rebuildAgentsTable ends in setCursorVisible(&m.agentsTbl, cursor). Meanwhile agentRowsFromBuckets orders rows by usage.SortSeriesLabels (cost-ranked), so the order genuinely changes as spend accrues, and app.go:1385 rebuilds on a background refresh while the reader is standing on the pane.

Before this change that drift was harmless: it moved the cursor between agents, and Enter still scoped to whatever was highlighted. Now index 0 is a semantically different action, so drift can leave a reader on All agents where Enter clears the scope — the same surprise this PR removes, reached by another route. It is also reachable via Esc-then-A, since enterAgentsOrRefuse does not reset the cursor either.

Narrow, pre-existing in its mechanism, and still better than the toggle it replaces, so not blocking. But it might be worth re-anchoring the cursor on the currently scoped agent when the pane opens, which would make Esc-then-A-then-Enter a no-op the way a reader would expect.

[mrsabath]

nit: the All agents row appends the SESSIONS cell at the end, while the agent rows below insert it at position 1 (append(r[:1:1], append(table.Row{...}, r[1:]...)...)). Both render identically today because every added cell here is "", so this is cosmetic only — but the two rows in one table being built against different column orders is the kind of thing that stops being harmless the moment one of these cells gains content.