Merged
Conversation
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant CLI as Client (CLI)
participant Cmd as Status Command
participant Config as Config Loader
participant Filter as Check Filterer
participant Model as TUI Model
participant Runner as Check Runner
participant Tea as Bubble Tea Runtime
participant Alerts as Alert Loader
participant View as Terminal View
CLI->>Cmd: run "status" with flags
Cmd->>Config: load config path
Config-->>Cmd: config
Cmd->>Filter: apply --only / --group
Filter-->>Cmd: filtered checks
Cmd->>Model: NewModel(config, checks, watch)
Model->>Alerts: LoadRecentAlerts()
Alerts-->>Model: alert lines
Model->>Tea: start program (Init)
Tea->>Model: Init -> runChecksCmd
Model->>Runner: RunChecks(timeout)
Runner-->>Model: CheckResult[]
loop if watch
Tea->>Model: tickMsg (timer)
Model->>Runner: RunChecks()
Runner-->>Model: CheckResult[]
end
Model->>View: View() -> render groups, alerts
Tea-->>CLI: program exit / final state
Cmd->>Model: Results() (copy)
Cmd->>Cmd: AllPassed() check (if not watch)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
internal/tui/alerts.go (1)
31-67: Consider memory efficiency for large alert logs.The current implementation reads all lines into memory before slicing the last
limitentries. For typical alert logs this is fine, but if logs grow large, this could become inefficient.For a future optimization, consider reading the file in reverse or using a ring buffer to only keep the last N entries in memory:
// Ring buffer approach - only keeps last `limit` entries parsed := make([]AlertLine, 0, limit) for scanner.Scan() { // ...parse... if len(parsed) >= limit { parsed = append(parsed[1:], entry) // shift and append } else { parsed = append(parsed, entry) } }This is a minor optimization and can be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/alerts.go` around lines 31 - 67, LoadRecentAlerts currently accumulates all parsed AlertLine entries into parsed before slicing the last limit, which can use excessive memory for large logs; update the loop in LoadRecentAlerts to use a fixed-size ring buffer behavior (keep parsed with capacity limit and when at capacity drop the oldest entry before appending) so only the last `limit` entries are retained in memory (or alternatively read file in reverse), ensuring scanner.Err() handling and the same return semantics remain unchanged.internal/tui/alerts_test.go (1)
37-47: Consider adding a test forlimit=0.The
LoadRecentAlertsfunction has a guard clause for non-positive limits. Adding a test case forlimit=0would verify this boundary condition:func TestLoadRecentAlertsZeroLimitReturnsEmpty(t *testing.T) { t.Parallel() alerts, err := LoadRecentAlerts(filepath.Join(t.TempDir(), "any.log"), 0) if err != nil { t.Fatalf("expected no error, got %v", err) } if len(alerts) != 0 { t.Fatalf("expected no alerts for limit=0, got %d", len(alerts)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/alerts_test.go` around lines 37 - 47, Add a new unit test in internal/tui/alerts_test.go that calls LoadRecentAlerts with limit=0 to verify the non-positive limit guard returns no error and an empty slice; name it e.g. TestLoadRecentAlertsZeroLimitReturnsEmpty, run it with t.Parallel(), call LoadRecentAlerts(filepath.Join(t.TempDir(), "any.log"), 0), assert err == nil and len(alerts) == 0, and match the assertion style used in the existing TestLoadRecentAlertsMissingFileIsEmpty.go.mod (1)
14-17: Pre-release dependency versions may introduce instability.Two indirect dependencies use pre-release/pseudo-versions:
charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dccharmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31ddThese are pulled transitively by bubbletea. While this is often unavoidable with rapidly evolving libraries, be aware that pseudo-versions can introduce breaking changes. Consider periodically running
go get -uonce stable releases are available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 14 - 17, The go.mod currently contains pseudo/pre-release versions for indirect modules github.com/charmbracelet/colorprofile and github.com/charmbracelet/x/cellbuf; update these to stable tagged releases (or remove explicit indirect pins and let `go get`/`go mod tidy` resolve to stable releases) once available to avoid instability. Locate the entries referencing `github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc` and `github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd` and replace them with their corresponding semver tags (or run `go get -u github.com/charmbracelet/bubbletea`/the specific modules and then `go mod tidy`) so the module graph points to non-pseudo versions. Ensure you run `go mod tidy` and run tests/build to verify compatibility after the change.internal/tui/model.go (3)
185-189: Consider dynamic column sizing or truncation.The hardcoded widths (
%-30sfor name,%-18sfor reason) will cause misalignment if values exceed these sizes. For a TUI, consider truncating long values or calculating widths dynamically based on actual content.💡 Example truncation helper
func truncate(s string, maxLen int) string { if len(s) <= maxLen { return s } if maxLen <= 3 { return s[:maxLen] } return s[:maxLen-3] + "..." }Usage:
-line := fmt.Sprintf(" %s %-30s %-18s %8s", indicator, result.Name, result.Reason, formatDuration(result.Duration)) +line := fmt.Sprintf(" %s %-30s %-18s %8s", indicator, truncate(result.Name, 30), truncate(result.Reason, 18), formatDuration(result.Duration))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/model.go` around lines 185 - 189, The current fixed-width formatting in the entries loop (inside the method that calls m.statusStyle and uses formatDuration) causes misalignment when result.Name or result.Reason exceed the hardcoded widths; update the loop to compute column widths dynamically (based on the longest visible Name/Reason among entries or terminal width) and/or truncate long strings before formatting (use a truncate helper that adds "..." for overflow) so the fmt.Sprintf line uses calculated widths or fixed placeholders with already-truncated values, keeping m.statusStyle and formatDuration usage unchanged.
136-147: Inconsistent receiver type.This method uses a pointer receiver while all other
Modelmethods use value receivers. While this works (Go auto-dereferences in the call at line 73), mixing receiver types is inconsistent and can cause confusion in Bubble Tea models.Consider refactoring to return the loaded alerts, keeping all methods consistent with value receivers:
♻️ Suggested refactor for consistent value receivers
-func (m *Model) loadAlerts() { - if m.alertsPath == "" { - return - } - alerts, err := LoadRecentAlerts(m.alertsPath, 10) - if err != nil { - m.alertsErr = err - return - } - m.alerts = alerts - m.alertsErr = nil -} +func (m Model) loadAlerts() ([]AlertLine, error) { + if m.alertsPath == "" { + return nil, nil + } + return LoadRecentAlerts(m.alertsPath, 10) +}Then in
Update:case checksMsg: m.results = msg.results - m.loadAlerts() + m.alerts, m.alertsErr = m.loadAlerts() if !m.watch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/model.go` around lines 136 - 147, The method loadAlerts currently has a pointer receiver for type Model while other Model methods use value receivers; change loadAlerts to use a value receiver and refactor it to return the loaded alerts and an error (e.g., func (m Model) loadAlerts() ([]AlertType, error)) so callers (notably Update) can assign m.alerts and m.alertsErr from the returned values; use the existing LoadRecentAlerts(alertsPath, 10) call inside the new function and propagate its error instead of mutating m.alerts/m.alertsErr directly, and update all call sites (e.g., the Update method) to handle the returned alerts and error.
209-214: Consider using constants for alert state comparisons.The state strings
"recovered"and"warn"are used as magic strings. If these states are defined elsewhere (e.g., in the notify package), consider exporting and reusing those constants to prevent typos and improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/model.go` around lines 209 - 214, Replace the magic string comparisons on alert.State in the render logic (the block that sets stateStyle in internal/tui/model.go) with the exported constants from the notify package (e.g., use notify.<STATE_RECOVERED_CONST> and notify.<STATE_WARN_CONST> instead of "recovered" and "warn"); update the import if necessary and adjust comparisons to use those constants to avoid typos and improve maintainability while keeping the existing mapping to m.styles.Pass/m.styles.TimedOut/m.styles.Fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/status_test.go`:
- Around line 26-35: The test is failing because result.stdout from
executeCheckCommand contains raw ANSI escape sequences from the Bubble Tea TUI;
update the test to strip ANSI sequences from result.stdout before performing
assertions (e.g., run resultStd :=
regexp.MustCompile(`\x1b\[[0-9;?]*[A-Za-z]`).ReplaceAllString(result.stdout, "")
or equivalent) and then assert against resultStd for the expected strings
("healthd - 2 checks - 1 ok - 1 fail" and "services"); reference the
executeCheckCommand call and use the cleaned stdout variable in the subsequent
t.Fatalf checks.
---
Nitpick comments:
In `@go.mod`:
- Around line 14-17: The go.mod currently contains pseudo/pre-release versions
for indirect modules github.com/charmbracelet/colorprofile and
github.com/charmbracelet/x/cellbuf; update these to stable tagged releases (or
remove explicit indirect pins and let `go get`/`go mod tidy` resolve to stable
releases) once available to avoid instability. Locate the entries referencing
`github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc` and
`github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd` and
replace them with their corresponding semver tags (or run `go get -u
github.com/charmbracelet/bubbletea`/the specific modules and then `go mod tidy`)
so the module graph points to non-pseudo versions. Ensure you run `go mod tidy`
and run tests/build to verify compatibility after the change.
In `@internal/tui/alerts_test.go`:
- Around line 37-47: Add a new unit test in internal/tui/alerts_test.go that
calls LoadRecentAlerts with limit=0 to verify the non-positive limit guard
returns no error and an empty slice; name it e.g.
TestLoadRecentAlertsZeroLimitReturnsEmpty, run it with t.Parallel(), call
LoadRecentAlerts(filepath.Join(t.TempDir(), "any.log"), 0), assert err == nil
and len(alerts) == 0, and match the assertion style used in the existing
TestLoadRecentAlertsMissingFileIsEmpty.
In `@internal/tui/alerts.go`:
- Around line 31-67: LoadRecentAlerts currently accumulates all parsed AlertLine
entries into parsed before slicing the last limit, which can use excessive
memory for large logs; update the loop in LoadRecentAlerts to use a fixed-size
ring buffer behavior (keep parsed with capacity limit and when at capacity drop
the oldest entry before appending) so only the last `limit` entries are retained
in memory (or alternatively read file in reverse), ensuring scanner.Err()
handling and the same return semantics remain unchanged.
In `@internal/tui/model.go`:
- Around line 185-189: The current fixed-width formatting in the entries loop
(inside the method that calls m.statusStyle and uses formatDuration) causes
misalignment when result.Name or result.Reason exceed the hardcoded widths;
update the loop to compute column widths dynamically (based on the longest
visible Name/Reason among entries or terminal width) and/or truncate long
strings before formatting (use a truncate helper that adds "..." for overflow)
so the fmt.Sprintf line uses calculated widths or fixed placeholders with
already-truncated values, keeping m.statusStyle and formatDuration usage
unchanged.
- Around line 136-147: The method loadAlerts currently has a pointer receiver
for type Model while other Model methods use value receivers; change loadAlerts
to use a value receiver and refactor it to return the loaded alerts and an error
(e.g., func (m Model) loadAlerts() ([]AlertType, error)) so callers (notably
Update) can assign m.alerts and m.alertsErr from the returned values; use the
existing LoadRecentAlerts(alertsPath, 10) call inside the new function and
propagate its error instead of mutating m.alerts/m.alertsErr directly, and
update all call sites (e.g., the Update method) to handle the returned alerts
and error.
- Around line 209-214: Replace the magic string comparisons on alert.State in
the render logic (the block that sets stateStyle in internal/tui/model.go) with
the exported constants from the notify package (e.g., use
notify.<STATE_RECOVERED_CONST> and notify.<STATE_WARN_CONST> instead of
"recovered" and "warn"); update the import if necessary and adjust comparisons
to use those constants to avoid typos and improve maintainability while keeping
the existing mapping to m.styles.Pass/m.styles.TimedOut/m.styles.Fail.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
cmd/root.gocmd/status.gocmd/status_test.gogo.modinternal/tui/alerts.gointernal/tui/alerts_test.gointernal/tui/model.gointernal/tui/model_accessors.gointernal/tui/styles.go
glitch418x
force-pushed
the
feat/status-tui
branch
from
aca6b32 to
b7ff3ea
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/tui/model.go (1)
120-128: Consider context cancellation for long-running checks.
runChecksCmdusescontext.Background()which means running checks cannot be cancelled when the user quits. If checks are long-running, they will continue executing in the background even after the TUI exits.For v1 this is acceptable, but consider passing a cancellable context in future iterations for cleaner shutdown.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/model.go` around lines 120 - 128, runChecksCmd currently uses context.Background(), so long-running checks started by Model.runChecks won't be cancellable when the TUI exits; change it to use a cancellable context from the Model (e.g., a field like m.ctx or a context created via context.WithCancel stored on Model) or make runChecksCmd accept a context parameter and pass that through to runChecks; ensure runChecks is called with that cancellable context and wire Model shutdown/quit logic to call the cancel function so running checks stop on exit (refer to Model.runChecksCmd, Model.runChecks, and checksMsg).internal/tui/alerts.go (1)
31-67: Consider memory usage for large log files.
LoadRecentAlertsreads the entire file into memory before tailing. For very large alert logs, this could consume significant memory. Since this is a TUI displaying only recent alerts (limit of 10 in practice), consider reading the file in reverse or using a ring buffer approach if logs grow large.For the current use case this is acceptable, but worth noting for future scalability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/alerts.go` around lines 31 - 67, LoadRecentAlerts currently appends every parsed entry into the parsed slice (parsed) which can grow to file size; change it to maintain only the last N entries by using a fixed-size ring buffer or circular slice instead of collecting all lines: inside LoadRecentAlerts (and where parsed and scanner are used) allocate a slice with capacity limit and push entries into it; when capacity is exceeded overwrite oldest entries (or use container/ring) so that at the end you can return the collected entries in chronological order limited to `limit` without reading the entire file into memory. Ensure behavior for limit<=0 and file-not-exist remains unchanged and keep using parseAlertLine for parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/tui/alerts.go`:
- Around line 31-67: LoadRecentAlerts currently appends every parsed entry into
the parsed slice (parsed) which can grow to file size; change it to maintain
only the last N entries by using a fixed-size ring buffer or circular slice
instead of collecting all lines: inside LoadRecentAlerts (and where parsed and
scanner are used) allocate a slice with capacity limit and push entries into it;
when capacity is exceeded overwrite oldest entries (or use container/ring) so
that at the end you can return the collected entries in chronological order
limited to `limit` without reading the entire file into memory. Ensure behavior
for limit<=0 and file-not-exist remains unchanged and keep using parseAlertLine
for parsing.
In `@internal/tui/model.go`:
- Around line 120-128: runChecksCmd currently uses context.Background(), so
long-running checks started by Model.runChecks won't be cancellable when the TUI
exits; change it to use a cancellable context from the Model (e.g., a field like
m.ctx or a context created via context.WithCancel stored on Model) or make
runChecksCmd accept a context parameter and pass that through to runChecks;
ensure runChecks is called with that cancellable context and wire Model
shutdown/quit logic to call the cancel function so running checks stop on exit
(refer to Model.runChecksCmd, Model.runChecks, and checksMsg).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
cmd/root.gocmd/status.gocmd/status_test.gogo.modinternal/tui/alerts.gointernal/tui/alerts_test.gointernal/tui/model.gointernal/tui/model_accessors.gointernal/tui/model_test.gointernal/tui/styles.go
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/status_test.go
- internal/tui/model_accessors.go
- cmd/root.go
New 'healthd status' command renders check results grouped by category with colored pass/fail indicators and recent alert history. - One-shot mode: runs checks once, renders, exits - Watch mode (-w): live-updating display on config interval - Supports --only and --group filters - Uses charmbracelet/bubbletea + lipgloss for rendering
glitch418x
force-pushed
the
feat/status-tui
branch
from
b7ff3ea to
95cfae3
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/tui/model.go (1)
73-77: Consider decoupling alert-log reload from every check cycle in watch mode.At Line 73, each
checksMsgtriggersloadAlerts(). With largeralerts.log, this adds repeated I/O on every refresh interval and can impact TUI responsiveness. A separate (slower) alerts refresh cadence or mtime-based reload would scale better.Also applies to: 136-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/model.go` around lines 73 - 77, The checksMsg handler is calling loadAlerts() on every refresh, causing heavy I/O; change the logic in the checksMsg handling (and the similar block around lines 136-147) to avoid unconditional reloads by either: 1) tracking the alerts.log modification time in a field (e.g., lastAlertsModTime) and calling loadAlerts() only when mtime differs, or 2) decoupling alert reload into its own slower ticker/command (e.g., alertsTickCmd) that updates alerts on a separate cadence; update references to loadAlerts(), tickCmd(), and the watch flag accordingly so regular tickCmd-driven refreshes do not repeatedly read the file.internal/tui/alerts.go (1)
45-67: Keep only the lastlimitalerts while scanning.At Line 45, every parsed alert is accumulated and only then tail-sliced. This is avoidable memory growth for large logs.
Proposed refactor
- parsed := make([]AlertLine, 0, limit) + parsed := make([]AlertLine, 0, limit) scanner := bufio.NewScanner(file) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) if line == "" { continue } entry, ok := parseAlertLine(line) if !ok { continue } - parsed = append(parsed, entry) + if len(parsed) < limit { + parsed = append(parsed, entry) + continue + } + copy(parsed, parsed[1:]) + parsed[len(parsed)-1] = entry } @@ - if len(parsed) <= limit { - return parsed, nil - } - return parsed[len(parsed)-limit:], nil + return parsed, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tui/alerts.go` around lines 45 - 67, The current loop in the function that reads alerts accumulates all parsed entries into parsed before returning the last limit items; change the loop so it only retains the last limit entries as it scans: when parseAlertLine(line) returns entry, append it to parsed and if len(parsed) > limit drop the oldest (e.g., parsed = parsed[1:] or implement a small circular buffer using an index) so memory doesn’t grow for large logs; keep the existing scanner.Err() check and final return of parsed (which will already be at most limit). Ensure you update the code paths that reference parsed, and keep parseAlertLine, scanner, limit and file usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 13-15: The go.mod entries for github.com/charmbracelet/bubbletea
and github.com/charmbracelet/lipgloss are marked as // indirect but are actually
directly imported (see internal/tui/model.go, internal/tui/styles.go,
cmd/status.go, internal/tui/model_test.go); run `go mod tidy` (or remove the `//
indirect` annotations and re-run tidy) to update go.mod so those packages are
listed as direct dependencies, ensuring the module metadata reflects actual
imports.
- Line 3: The go.mod currently lists github.com/charmbracelet/bubbletea and
github.com/charmbracelet/lipgloss as indirect, but they are directly imported in
internal/tui/model.go, internal/tui/styles.go and cmd/status.go; update go.mod
to make them direct dependencies by adding them to the main require block (or
run go get github.com/charmbracelet/bubbletea@<version> and go get
github.com/charmbracelet/lipgloss@<version>) so the entries are non-// indirect
and match the versions used in the codebase; ensure go.mod is tidy afterward (go
mod tidy) to remove any stale indirect markers.
In `@internal/tui/alerts.go`:
- Around line 46-61: LoadRecentAlerts currently uses bufio.NewScanner with
default 64KiB token size and can fail with bufio.ErrTooLong for long alert
lines; update it to handle long lines by either switching to bufio.Reader and
looping with ReadString('\n') (trim, skip empty, call parseAlertLine and append)
or by calling scanner.Buffer(make([]byte, 0, 64*1024), maxTokenBytes) before
scanning to raise the max token size and explicitly check for
scanner.ErrTooLong; ensure error handling still returns fmt.Errorf("read alerts
log %q: %w", path, err) and keep parseAlertLine usage unchanged.
---
Nitpick comments:
In `@internal/tui/alerts.go`:
- Around line 45-67: The current loop in the function that reads alerts
accumulates all parsed entries into parsed before returning the last limit
items; change the loop so it only retains the last limit entries as it scans:
when parseAlertLine(line) returns entry, append it to parsed and if len(parsed)
> limit drop the oldest (e.g., parsed = parsed[1:] or implement a small circular
buffer using an index) so memory doesn’t grow for large logs; keep the existing
scanner.Err() check and final return of parsed (which will already be at most
limit). Ensure you update the code paths that reference parsed, and keep
parseAlertLine, scanner, limit and file usage unchanged.
In `@internal/tui/model.go`:
- Around line 73-77: The checksMsg handler is calling loadAlerts() on every
refresh, causing heavy I/O; change the logic in the checksMsg handling (and the
similar block around lines 136-147) to avoid unconditional reloads by either: 1)
tracking the alerts.log modification time in a field (e.g., lastAlertsModTime)
and calling loadAlerts() only when mtime differs, or 2) decoupling alert reload
into its own slower ticker/command (e.g., alertsTickCmd) that updates alerts on
a separate cadence; update references to loadAlerts(), tickCmd(), and the watch
flag accordingly so regular tickCmd-driven refreshes do not repeatedly read the
file.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
cmd/root.gocmd/status.gocmd/status_test.gogo.modinternal/tui/alerts.gointernal/tui/alerts_test.gointernal/tui/model.gointernal/tui/model_accessors.gointernal/tui/model_test.gointernal/tui/styles.go
🚧 Files skipped from review as they are similar to previous changes (6)
- internal/tui/model_test.go
- internal/tui/model_accessors.go
- internal/tui/styles.go
- cmd/status.go
- cmd/status_test.go
- cmd/root.go
| module github.com/uinaf/healthd | ||
|
|
||
| go 1.22 | ||
| go 1.24.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for Go version pins in CI, Docker, docs, and other config files
set -euo pipefail
echo "=== Go version references in codebase ==="
rg -n --hidden -S 'go-version|golang:|FROM golang|go 1\.[0-9]+' --type-list | head -5
echo
echo "=== Checking specific files for Go version constraints ==="
for file in "go.mod" "go.work" "Dockerfile" ".tool-versions" ".go-version" "README.md"; do
if [ -f "$file" ]; then
echo "--- $file ---"
cat "$file" | head -20
fi
done
echo
echo "=== GitHub workflow files ==="
find .github/workflows -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -5 | xargs -I {} sh -c 'echo "--- {} ---"; head -40 {}'Repository: uinaf/healthd
Length of output: 505
🏁 Script executed:
#!/bin/bash
# Check if charmbracelet/bubbletea and charmbracelet/lipgloss are directly imported
echo "=== Searching for direct imports of bubbletea or lipgloss ==="
rg -n 'import\s*\(|import\s+"github\.com/charmbracelet/(bubbletea|lipgloss)' --type goRepository: uinaf/healthd
Length of output: 1400
🏁 Script executed:
#!/bin/bash
# Check go.mod for dependency structure
echo "=== Full go.mod content ==="
cat go.modRepository: uinaf/healthd
Length of output: 1581
🏁 Script executed:
#!/bin/bash
# Check for direct bubbletea imports specifically
echo "=== Direct imports of bubbletea ==="
rg -n '"github\.com/charmbracelet/bubbletea"' --type go
echo
echo "=== All charmbracelet imports in .go files ==="
rg -n 'charmbracelet/(bubbletea|lipgloss)' --type go | head -20Repository: uinaf/healthd
Length of output: 662
🏁 Script executed:
#!/bin/bash
# Find CI/config files that may pin Go versions
echo "=== CI/Config files ==="
find . -type f \( -name "*.yml" -o -name "*.yaml" -o -name "Dockerfile*" -o -name ".go-version" -o -name ".tool-versions" \) 2>/dev/null | grep -v node_modules
echo
echo "=== Go version in CI workflows ==="
for f in $(find .github -name "*.yml" -o -name "*.yaml" 2>/dev/null); do
echo "--- $f ---"
rg -n 'go-version|golang' "$f"
done
echo
echo "=== Dockerfile Go version ==="
if [ -f Dockerfile ]; then
head -10 Dockerfile
fiRepository: uinaf/healthd
Length of output: 871
🏁 Script executed:
#!/bin/bash
# Examine internal/tui files to see bubbletea usage
echo "=== Files in internal/tui ==="
ls -la internal/tui/*.go 2>/dev/null
echo
echo "=== Check model.go for bubbletea imports ==="
grep -n 'bubbletea\|tea\.' internal/tui/model.go 2>/dev/null | head -10Repository: uinaf/healthd
Length of output: 966
🏁 Script executed:
#!/bin/bash
cat .tool-versionsRepository: uinaf/healthd
Length of output: 70
Move bubbletea and lipgloss to direct dependencies in go.mod.
Both packages are directly imported in your code (internal/tui/model.go, internal/tui/styles.go, cmd/status.go) but currently marked as // indirect in go.mod. Move them to the main require block to reflect actual usage and avoid potential issues with dependency pruning or future Go versions.
Tooling alignment is fine—.tool-versions pins Go 1.26.0, which exceeds the go.mod requirement of 1.24.0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 3, The go.mod currently lists
github.com/charmbracelet/bubbletea and github.com/charmbracelet/lipgloss as
indirect, but they are directly imported in internal/tui/model.go,
internal/tui/styles.go and cmd/status.go; update go.mod to make them direct
dependencies by adding them to the main require block (or run go get
github.com/charmbracelet/bubbletea@<version> and go get
github.com/charmbracelet/lipgloss@<version>) so the entries are non-// indirect
and match the versions used in the codebase; ensure go.mod is tidy afterward (go
mod tidy) to remove any stale indirect markers.
Comment on lines
+13
to
+15
| github.com/charmbracelet/bubbletea v1.3.10 // indirect | ||
| github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect | ||
| github.com/charmbracelet/lipgloss v1.1.0 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether bubbletea/lipgloss are directly imported by this module's Go files.
set -euo pipefail
echo "== Direct imports of Bubble Tea / Lipgloss in repository Go files =="
rg -n --type=go -P '"github\.com/charmbracelet/(bubbletea|lipgloss)(/[^"]*)?"' -g '!**/vendor/**'
echo
echo "If matches are found in module source/test files, these deps should typically be direct (not // indirect)."Repository: uinaf/healthd
Length of output: 559
Remove // indirect comments from bubbletea and lipgloss in go.mod.
Both packages are directly imported in module code (internal/tui/model.go, internal/tui/styles.go, cmd/status.go, and internal/tui/model_test.go). Run go mod tidy to correct their dependency labels.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 13 - 15, The go.mod entries for
github.com/charmbracelet/bubbletea and github.com/charmbracelet/lipgloss are
marked as // indirect but are actually directly imported (see
internal/tui/model.go, internal/tui/styles.go, cmd/status.go,
internal/tui/model_test.go); run `go mod tidy` (or remove the `// indirect`
annotations and re-run tidy) to update go.mod so those packages are listed as
direct dependencies, ensuring the module metadata reflects actual imports.
Comment on lines
+46
to
+61
| scanner := bufio.NewScanner(file) | ||
| for scanner.Scan() { | ||
| line := strings.TrimSpace(scanner.Text()) | ||
| if line == "" { | ||
| continue | ||
| } | ||
| entry, ok := parseAlertLine(line) | ||
| if !ok { | ||
| continue | ||
| } | ||
| parsed = append(parsed, entry) | ||
| } | ||
|
|
||
| if err := scanner.Err(); err != nil { | ||
| return nil, fmt.Errorf("read alerts log %q: %w", path, err) | ||
| } |
There was a problem hiding this comment.
Handle long alert lines to avoid scanner overflow.
At Line 46, bufio.Scanner keeps the default token limit (~64 KiB). A long alert reason can trigger bufio.ErrTooLong, causing LoadRecentAlerts to fail.
Proposed fix
scanner := bufio.NewScanner(file)
+ // Allow larger alert lines (e.g., long failure reasons).
+ scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024)
for scanner.Scan() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| scanner := bufio.NewScanner(file) | |
| for scanner.Scan() { | |
| line := strings.TrimSpace(scanner.Text()) | |
| if line == "" { | |
| continue | |
| } | |
| entry, ok := parseAlertLine(line) | |
| if !ok { | |
| continue | |
| } | |
| parsed = append(parsed, entry) | |
| } | |
| if err := scanner.Err(); err != nil { | |
| return nil, fmt.Errorf("read alerts log %q: %w", path, err) | |
| } | |
| scanner := bufio.NewScanner(file) | |
| // Allow larger alert lines (e.g., long failure reasons). | |
| scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) | |
| for scanner.Scan() { | |
| line := strings.TrimSpace(scanner.Text()) | |
| if line == "" { | |
| continue | |
| } | |
| entry, ok := parseAlertLine(line) | |
| if !ok { | |
| continue | |
| } | |
| parsed = append(parsed, entry) | |
| } | |
| if err := scanner.Err(); err != nil { | |
| return nil, fmt.Errorf("read alerts log %q: %w", path, err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/tui/alerts.go` around lines 46 - 61, LoadRecentAlerts currently uses
bufio.NewScanner with default 64KiB token size and can fail with
bufio.ErrTooLong for long alert lines; update it to handle long lines by either
switching to bufio.Reader and looping with ReadString('\n') (trim, skip empty,
call parseAlertLine and append) or by calling scanner.Buffer(make([]byte, 0,
64*1024), maxTokenBytes) before scanning to raise the max token size and
explicitly check for scanner.ErrTooLong; ensure error handling still returns
fmt.Errorf("read alerts log %q: %w", path, err) and keep parseAlertLine usage
unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New
healthd statuscommand with terminal UI for viewing check results.Features
-w)--onlyand--groupfiltersPreview
Summary by CodeRabbit
New Features
Chores
Tests