Add interactive TUI for browsing Honeybadger data#17
Add interactive TUI for browsing Honeybadger data#17stympy wants to merge 20 commits intoadd-more-endpointsfrom
Conversation
Adds a new `hb tui` command that launches an interactive terminal UI
for browsing Honeybadger data, similar to how e1s works for AWS ECS.
Features:
- Hierarchical navigation from accounts to projects to resources
- Browse faults with drill-down to notices and affected users
- View deployments, uptime sites (with outages/checks), check-ins
- View teams with members and invitations
- View project integrations
- Vim-style keybindings (j/k/h/l) plus arrow keys
- Help modal with ? key
- Refresh with r key
Navigation hierarchy:
Accounts
└── Account Menu (Projects, Teams, Users, Invitations)
├── Projects
│ └── Project Menu
│ ├── Faults → Notices, Affected Users
│ ├── Deployments
│ ├── Uptime Sites → Outages, Checks
│ ├── Check-ins
│ └── Integrations
└── Teams → Members, Invitations
Uses tview library for the terminal UI.
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive interactive TUI (Terminal User Interface) for browsing Honeybadger data. The implementation introduces a new hb tui command that provides hierarchical navigation through accounts, projects, faults, deployments, uptime sites, check-ins, teams, and related resources using Vim-style keybindings.
Key changes:
- New TUI framework using tview library with navigation stack and breadcrumb UI
- Complete view implementations for all major Honeybadger resources with drill-down capability
- Keyboard-driven interface with help modal and refresh functionality
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates Go version and adds TUI dependencies (tcell, tview, honeybadger-io/api-go) |
| go.sum | Adds checksums for new TUI dependencies |
| cmd/tui.go | Command entry point that initializes API client and launches TUI |
| tui/app.go | Core TUI application with navigation stack, input handling, and view lifecycle |
| tui/accounts.go | Views for browsing accounts, users, and invitations |
| tui/projects.go | Views for projects and integrations |
| tui/faults.go | Views for faults, notices, and affected users |
| tui/deployments.go | Views for deployments with detail drill-down |
| tui/checkins.go | Views for check-ins (cron monitoring) |
| tui/uptime.go | Views for uptime sites, outages, and checks |
| tui/teams.go | Views for teams, members, and invitations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (v *AccountsView) renderTable() { | ||
| // Clear existing rows (keep header) | ||
| for row := v.table.GetRowCount() - 1; row > 0; row-- { | ||
| v.table.RemoveRow(row) | ||
| } |
There was a problem hiding this comment.
This table clearing pattern is duplicated across multiple files (accounts.go, teams.go, uptime.go, faults.go, projects.go, deployments.go, checkins.go). Consider extracting this into a shared helper function to reduce code duplication and improve maintainability. For example, create a helper method like clearTableRows(table *tview.Table) in a shared utilities file.
| func (v *AccountsView) renderTable() { | |
| // Clear existing rows (keep header) | |
| for row := v.table.GetRowCount() - 1; row > 0; row-- { | |
| v.table.RemoveRow(row) | |
| } | |
| func clearTableRows(table *tview.Table) { | |
| for row := table.GetRowCount() - 1; row > 0; row-- { | |
| table.RemoveRow(row) | |
| } | |
| } | |
| func (v *AccountsView) renderTable() { | |
| // Clear existing rows (keep header) | |
| clearTableRows(v.table) |
| // HandleInput handles keyboard input | ||
| func (v *AccountsView) HandleInput(event *tcell.EventKey) *tcell.EventKey { | ||
| switch event.Rune() { | ||
| case 'j': | ||
| row, col := v.table.GetSelection() | ||
| if row < v.table.GetRowCount()-1 { | ||
| v.table.Select(row+1, col) | ||
| } | ||
| return nil | ||
| case 'k': | ||
| row, col := v.table.GetSelection() | ||
| if row > 1 { | ||
| v.table.Select(row-1, col) | ||
| } | ||
| return nil | ||
| case 'l': | ||
| row, _ := v.table.GetSelection() | ||
| if row > 0 && row <= len(v.accounts) { | ||
| account := v.accounts[row-1] | ||
| v.drillDown(account) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| if event.Key() == tcell.KeyEnter || event.Key() == tcell.KeyRight { | ||
| row, _ := v.table.GetSelection() | ||
| if row > 0 && row <= len(v.accounts) { | ||
| account := v.accounts[row-1] | ||
| v.drillDown(account) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| return event | ||
| } |
There was a problem hiding this comment.
The HandleInput keyboard navigation logic is heavily duplicated across all view types. Consider creating a base handler or helper functions for common navigation patterns (j/k for up/down in tables, j/k for list navigation, h for back). This would significantly reduce code duplication and make it easier to maintain consistent keyboard behavior across all views.
tui/app.go
Outdated
| // Refresh the view to load data | ||
| go func() { | ||
| if err := view.Refresh(); err != nil { | ||
| a.app.QueueUpdateDraw(func() { | ||
| a.ShowError(err) | ||
| }) | ||
| } | ||
| a.app.Draw() | ||
| }() |
There was a problem hiding this comment.
The goroutine spawned in Push method does not have a mechanism to be canceled when the view is popped before the refresh completes. If a user quickly navigates away from a view, the goroutine will continue running and potentially call ShowError on a view that's no longer visible. Consider using a cancellable context that gets canceled in the Pop method, or checking if the view is still active before updating the UI.
tui/app.go
Outdated
| client: client, | ||
| pages: tview.NewPages(), | ||
| navStack: make([]View, 0), | ||
| ctx: context.Background(), |
There was a problem hiding this comment.
The context is created using context.Background() with no cancellation mechanism. This means that if the application is stopped, any in-flight API requests will not be canceled and could continue running. Consider using a context that can be canceled when the application stops, and propagate this through the API calls to enable proper cleanup.
tui/uptime.go
Outdated
| if len(reason) > 40 { | ||
| reason = reason[:37] + "..." |
There was a problem hiding this comment.
String truncation logic is not UTF-8 aware and will panic if the string contains multi-byte characters and the slice index falls in the middle of a character. Use a UTF-8-aware truncation function or the runes package to safely truncate strings that may contain Unicode characters. For example, convert to runes first: runes := []rune(reason); if len(runes) > 40 { reason = string(runes[:37]) + "..." }
| if len(reason) > 40 { | |
| reason = reason[:37] + "..." | |
| runes := []rune(reason) | |
| if len(runes) > 40 { | |
| reason = string(runes[:37]) + "..." |
tui/faults.go
Outdated
| title := v.fault.Klass | ||
| if len(title) > 50 { | ||
| title = title[:47] + "..." | ||
| } | ||
| v.list.SetTitle(fmt.Sprintf(" %s ", title)). | ||
| SetBorder(true). | ||
| SetBorderColor(tcell.ColorDarkCyan) | ||
|
|
||
| v.list.AddItem("Details", "View fault details", 'd', func() { | ||
| detailsView := NewFaultDetailsView(v.app, v.projectID, v.fault.ID) | ||
| v.app.Push(detailsView) | ||
| }) | ||
|
|
||
| v.list.AddItem("Notices", fmt.Sprintf("View notices (%d total)", v.fault.NoticesCount), 'n', func() { | ||
| noticesView := NewNoticesView(v.app, v.projectID, v.fault.ID) | ||
| v.app.Push(noticesView) | ||
| }) | ||
|
|
||
| v.list.AddItem("Affected Users", "View affected users", 'u', func() { | ||
| usersView := NewAffectedUsersView(v.app, v.projectID, v.fault.ID) | ||
| v.app.Push(usersView) | ||
| }) | ||
|
|
||
| v.list.SetSelectedBackgroundColor(tcell.ColorDarkCyan) | ||
| } | ||
|
|
||
| // Name returns the view name | ||
| func (v *FaultMenuView) Name() string { | ||
| name := v.fault.Klass | ||
| if len(name) > 30 { | ||
| name = name[:27] + "..." | ||
| } |
There was a problem hiding this comment.
String truncation logic is not UTF-8 aware and will panic if the string contains multi-byte characters and the slice index falls in the middle of a character. Use a UTF-8-aware truncation function or the runes package to safely truncate strings that may contain Unicode characters.
tui/faults.go
Outdated
| if len(message) > 40 { | ||
| message = message[:37] + "..." |
There was a problem hiding this comment.
String truncation logic is not UTF-8 aware and will panic if the string contains multi-byte characters and the slice index falls in the middle of a character. Use a UTF-8-aware truncation function or the runes package to safely truncate strings that may contain Unicode characters. For example, convert to runes first: runes := []rune(message); if len(runes) > 40 { message = string(runes[:37]) + "..." }
| if len(message) > 40 { | |
| message = message[:37] + "..." | |
| runes := []rune(message) | |
| if len(runes) > 40 { | |
| message = string(runes[:37]) + "..." |
tui/uptime.go
Outdated
| if len(url) > 40 { | ||
| url = url[:37] + "..." |
There was a problem hiding this comment.
String truncation logic is not UTF-8 aware and will panic if the string contains multi-byte characters and the slice index falls in the middle of a character. Use a UTF-8-aware truncation function or the runes package to safely truncate strings that may contain Unicode characters. For example, convert to runes first: runes := []rune(url); if len(runes) > 40 { url = string(runes[:37]) + "..." }
| if len(url) > 40 { | |
| url = url[:37] + "..." | |
| runes := []rune(url) | |
| if len(runes) > 40 { | |
| url = string(runes[:37]) + "..." |
tui/deployments.go
Outdated
| revision := d.Revision | ||
| if len(revision) > 12 { | ||
| revision = revision[:12] | ||
| } | ||
|
|
||
| repo := d.Repository | ||
| if len(repo) > 30 { | ||
| repo = repo[:27] + "..." | ||
| } |
There was a problem hiding this comment.
String truncation logic is not UTF-8 aware and will panic if the string contains multi-byte characters and the slice index falls in the middle of a character. Use a UTF-8-aware truncation function or the runes package to safely truncate strings that may contain Unicode characters.
tui/faults.go
Outdated
| message := notice.Message | ||
| if len(message) > 50 { | ||
| message = message[:47] + "..." | ||
| } | ||
|
|
||
| id := notice.ID | ||
| if len(id) > 12 { | ||
| id = id[:12] + "..." | ||
| } |
There was a problem hiding this comment.
String truncation logic is not UTF-8 aware and will panic if the string contains multi-byte characters and the slice index falls in the middle of a character. Use a UTF-8-aware truncation function or the runes package to safely truncate strings that may contain Unicode characters.
- Add helpers.go with reusable functions for table clearing, string truncation, and keyboard navigation handling - Update truncateString to be UTF-8 aware using runes - Add context cancellation support to app goroutines to prevent leaks - Consolidate duplicate keyboard handling code across all view files - Net reduction of ~345 lines through code deduplication Addresses Copilot review comments on PR #17.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| f.Environment, | ||
| f.Component, | ||
| f.Action, | ||
| f.CreatedAt.Format("2006-01-02 15:04:05"), |
There was a problem hiding this comment.
Potential nil pointer dereference: The code calls f.CreatedAt.Format() without checking if CreatedAt is nil. If the API returns a fault with a nil CreatedAt field, this will cause a panic. Consider using the formatTime helper function (which handles nil) for this field as well, or add a nil check.
| f.CreatedAt.Format("2006-01-02 15:04:05"), | |
| formatTime(f.CreatedAt), |
tui/faults.go
Outdated
| v.table.SetCell(row, 1, tview.NewTableCell(message).SetExpansion(3)) | ||
| v.table.SetCell(row, 2, tview.NewTableCell(notice.EnvironmentName).SetExpansion(1)) | ||
| v.table.SetCell(row, 3, tview.NewTableCell(notice.Environment.Hostname).SetExpansion(2)) | ||
| v.table.SetCell(row, 4, tview.NewTableCell(notice.CreatedAt.Format("2006-01-02 15:04")).SetExpansion(2)) |
There was a problem hiding this comment.
Potential nil pointer dereference: The code calls notice.CreatedAt.Format() without checking if CreatedAt is nil. If the API returns a notice with a nil CreatedAt field, this will cause a panic. Consider adding a nil check or using a helper function similar to formatTime.
tui/teams.go
Outdated
| v.table.SetCell(row, 0, tview.NewTableCell(fmt.Sprintf("%d", inv.ID)).SetExpansion(1)) | ||
| v.table.SetCell(row, 1, tview.NewTableCell(inv.Email).SetExpansion(2)) | ||
| v.table.SetCell(row, 2, tview.NewTableCell(admin).SetExpansion(1)) | ||
| v.table.SetCell(row, 3, tview.NewTableCell(inv.CreatedAt.Format("2006-01-02")).SetExpansion(1)) |
There was a problem hiding this comment.
Potential nil pointer dereference: The code calls inv.CreatedAt.Format() without checking if CreatedAt is nil. If the API returns an invitation with a nil CreatedAt field, this will cause a panic. Consider adding a nil check or using a helper function similar to formatTime.
| v.table.SetCell(row, 0, tview.NewTableCell(fmt.Sprintf("%d", inv.ID)).SetExpansion(1)) | |
| v.table.SetCell(row, 1, tview.NewTableCell(inv.Email).SetExpansion(2)) | |
| v.table.SetCell(row, 2, tview.NewTableCell(admin).SetExpansion(1)) | |
| v.table.SetCell(row, 3, tview.NewTableCell(inv.CreatedAt.Format("2006-01-02")).SetExpansion(1)) | |
| created := "" | |
| if inv.CreatedAt != nil { | |
| created = inv.CreatedAt.Format("2006-01-02") | |
| } | |
| v.table.SetCell(row, 0, tview.NewTableCell(fmt.Sprintf("%d", inv.ID)).SetExpansion(1)) | |
| v.table.SetCell(row, 1, tview.NewTableCell(inv.Email).SetExpansion(2)) | |
| v.table.SetCell(row, 2, tview.NewTableCell(admin).SetExpansion(1)) | |
| v.table.SetCell(row, 3, tview.NewTableCell(created).SetExpansion(1)) |
tui/app.go
Outdated
|
|
||
| [green]Actions:[white] | ||
| r Refresh current view | ||
| / Search (in list views) |
There was a problem hiding this comment.
The help text mentions a search feature with "/" key, but there's no implementation for search functionality in the input capture handler (lines 76-116) or anywhere else in the codebase. Either remove this from the help text or implement the search feature to avoid misleading users.
| / Search (in list views) |
tui/uptime.go
Outdated
| upColor = tcell.ColorGreen | ||
| } | ||
|
|
||
| v.table.SetCell(row, 0, tview.NewTableCell(check.CreatedAt.Format("2006-01-02 15:04:05")).SetExpansion(2)) |
There was a problem hiding this comment.
Potential nil pointer dereference: The code calls check.CreatedAt.Format() without checking if CreatedAt is nil. If the API returns an uptime check with a nil CreatedAt field, this will cause a panic. Consider adding a nil check or using a helper function similar to formatTime.
| v.table.SetCell(row, 0, tview.NewTableCell(check.CreatedAt.Format("2006-01-02 15:04:05")).SetExpansion(2)) | |
| createdAt := "" | |
| if check.CreatedAt != nil { | |
| createdAt = check.CreatedAt.Format("2006-01-02 15:04:05") | |
| } | |
| v.table.SetCell(row, 0, tview.NewTableCell(createdAt).SetExpansion(2)) |
| go func() { | ||
| // Check if context is cancelled before starting | ||
| select { | ||
| case <-a.ctx.Done(): | ||
| return | ||
| default: | ||
| } | ||
|
|
||
| if err := view.Refresh(); err != nil { | ||
| // Check if context is cancelled before showing error | ||
| select { | ||
| case <-a.ctx.Done(): | ||
| return | ||
| default: | ||
| a.app.QueueUpdateDraw(func() { | ||
| a.ShowError(err) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Check if context is cancelled before drawing | ||
| select { | ||
| case <-a.ctx.Done(): | ||
| return | ||
| default: | ||
| a.app.Draw() | ||
| } | ||
| }() | ||
| } |
There was a problem hiding this comment.
There's a potential race condition here. The goroutine at line 128 accesses the view after it has been pushed to the navigation stack, but there's no guarantee that the view hasn't been popped off the stack (via Pop() called from another goroutine or user input) before the Refresh() completes. If a user rapidly navigates forward and back, the goroutine may attempt to update a view that is no longer active, or worse, call ShowError after the context is cancelled. While the context cancellation checks help, consider tracking which view is currently active and only showing errors for the current view.
tui/checkins.go
Outdated
| } | ||
|
|
||
| text += fmt.Sprintf("\n\n[yellow]Project ID:[white] %d", ci.ProjectID) | ||
| text += fmt.Sprintf("\n[yellow]Created:[white] %s", ci.CreatedAt.Format("2006-01-02 15:04:05")) |
There was a problem hiding this comment.
Potential nil pointer dereference: The code calls ci.CreatedAt.Format() without checking if CreatedAt is nil. If the API returns a check-in with a nil CreatedAt field, this will cause a panic. Consider adding a nil check or using a helper function similar to formatTime used elsewhere in the codebase.
| text += fmt.Sprintf("\n[yellow]Created:[white] %s", ci.CreatedAt.Format("2006-01-02 15:04:05")) | |
| createdAtStr := "" | |
| if ci.CreatedAt != nil { | |
| createdAtStr = ci.CreatedAt.Format("2006-01-02 15:04:05") | |
| } | |
| text += fmt.Sprintf("\n[yellow]Created:[white] %s", createdAtStr) |
| v.table.SetCell(row, 0, tview.NewTableCell(id).SetExpansion(1)) | ||
| v.table.SetCell(row, 1, tview.NewTableCell(message).SetExpansion(3)) | ||
| v.table.SetCell(row, 2, tview.NewTableCell(notice.EnvironmentName).SetExpansion(1)) | ||
| v.table.SetCell(row, 3, tview.NewTableCell(notice.Environment.Hostname).SetExpansion(2)) |
There was a problem hiding this comment.
Potential nil pointer dereference: The code accesses notice.Environment.Hostname without checking if notice.Environment is nil. If the API returns a notice with a nil Environment field, this will cause a panic. Consider adding a nil check before accessing nested fields.
| v.table.SetCell(row, 0, tview.NewTableCell(id).SetExpansion(1)) | |
| v.table.SetCell(row, 1, tview.NewTableCell(message).SetExpansion(3)) | |
| v.table.SetCell(row, 2, tview.NewTableCell(notice.EnvironmentName).SetExpansion(1)) | |
| v.table.SetCell(row, 3, tview.NewTableCell(notice.Environment.Hostname).SetExpansion(2)) | |
| hostname := "" | |
| if notice.Environment != nil { | |
| hostname = notice.Environment.Hostname | |
| } | |
| v.table.SetCell(row, 0, tview.NewTableCell(id).SetExpansion(1)) | |
| v.table.SetCell(row, 1, tview.NewTableCell(message).SetExpansion(3)) | |
| v.table.SetCell(row, 2, tview.NewTableCell(notice.EnvironmentName).SetExpansion(1)) | |
| v.table.SetCell(row, 3, tview.NewTableCell(hostname).SetExpansion(2)) |
tui/app.go
Outdated
| a.footer = tview.NewTextView(). | ||
| SetDynamicColors(true). | ||
| SetTextAlign(tview.AlignCenter). | ||
| SetText("[yellow]↑↓/jk[white] Navigate [yellow]Enter/→[white] Select [yellow]Esc/←[white] Back [yellow]r[white] Refresh [yellow]q[white] Quit [yellow]?[white] Help") |
There was a problem hiding this comment.
In the footer navigation text, there's an inconsistency between what keys are mentioned here and what is handled in the code. The footer shows "Esc/←" and "Enter/→" for Back and Select, but the code also handles 'h' for back and 'l' for select (Vim-style). Either include these in the footer help text or remove them from the description to avoid user confusion. The help modal at line 205 does mention these keys, but the persistent footer should too for discoverability.
| SetText("[yellow]↑↓/jk[white] Navigate [yellow]Enter/→[white] Select [yellow]Esc/←[white] Back [yellow]r[white] Refresh [yellow]q[white] Quit [yellow]?[white] Help") | |
| SetText("[yellow]↑↓/jk[white] Navigate [yellow]Enter/l/→[white] Select [yellow]Esc/h/←[white] Back [yellow]r[white] Refresh [yellow]q[white] Quit [yellow]?[white] Help") |
| d.Repository, | ||
| d.LocalUsername, | ||
| d.ProjectID, | ||
| d.CreatedAt.Format("2006-01-02 15:04:05"), |
There was a problem hiding this comment.
Potential nil pointer dereference: The code calls d.CreatedAt.Format() without checking if CreatedAt is nil. If the API returns a deployment with a nil CreatedAt field, this will cause a panic. Consider using the formatTime helper function defined at line 213 in this same file, which already handles nil values properly.
| d.CreatedAt.Format("2006-01-02 15:04:05"), | |
| formatTime(d.CreatedAt), |
- Remove unimplemented search feature from help text - Add h/l keybindings to footer help text for consistency - Add isCurrentView() check to prevent stale view updates when user navigates rapidly before goroutine completes
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds a new
hb tuicommand that launches an interactive terminal UI for browsing Honeybadger data, similar to how e1s works for AWS ECS.Features:
Navigation hierarchy:
Uses tview library for the terminal UI.