Skip to content

Commit c10d28c

Browse files
committed
Improve process safety, metrics, and UI fixes
Several improvements and bugfixes across app, metrics, config and UI: - Prevent sending signals to PID <= 1 on Unix and Windows to avoid killing init/critical processes. - Add ProcLimit config/CLI flag (default 50) and thread it through collectSnapshot so process collection can be limited. - Refactor metric panel rendering into buildMetricsSection to remove duplicated layout logic used by View() and computeUsedLines(). - Make GPU collection concurrent with other collectors and compute energy impact after collection; export ComputeEnergyImpact and update backends to use it. - Handle counter wrap for network and disk deltas via safeDeltaRate/safeDiskDeltaRate and add tests for counter wrap behavior. - Improve battery parsing to infer charging state from power source when status is absent. - Export snapshots to the user's home directory with more restrictive file permissions (0600) and stable filename. - Improve UI: preserve UTF-8 when truncating strings, and warn to stderr and fall back to dark theme when an unknown theme is selected. - Add Ctrl-C handling to search to cancel collection and quit. These changes improve robustness (prevent dangerous signals and counter-wrap artifacts), configurability (process limit), and maintainability (layout refactor).
1 parent 1433aae commit c10d28c

15 files changed

Lines changed: 191 additions & 116 deletions

File tree

internal/app/kill_unix.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22

33
package app
44

5-
import "syscall"
5+
import (
6+
"fmt"
7+
"syscall"
8+
)
69

710
// killSignal represents a signal to send to a process.
811
type killSignal int
@@ -13,6 +16,10 @@ const (
1316
)
1417

1518
// killProcess sends sig to the given PID.
19+
// Rejects PID <= 1 to prevent killing init or the entire process group.
1620
func killProcess(pid int, sig killSignal) error {
21+
if pid <= 1 {
22+
return fmt.Errorf("refusing to signal PID %d", pid)
23+
}
1724
return syscall.Kill(pid, syscall.Signal(sig))
1825
}

internal/app/kill_windows.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ const (
1616
)
1717

1818
// killProcess terminates the given PID on Windows via taskkill.
19+
// Rejects PID <= 1 to prevent killing critical system processes.
1920
func killProcess(pid int, sig killSignal) error {
21+
if pid <= 1 {
22+
return fmt.Errorf("refusing to signal PID %d", pid)
23+
}
2024
var cmd *exec.Cmd
2125
if sig == signalKill {
2226
cmd = exec.Command("taskkill", "/F", "/PID", fmt.Sprint(pid))

internal/app/model.go

Lines changed: 35 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
101101
ctx, cancel := context.WithTimeout(context.Background(), m.collectionTimeout())
102102
m.collectCancel = cancel
103103
m.collecting = true
104-
cmds = append(cmds, collectSnapshot(ctx, m.sortBy, m.snap, m.processSampleEvery(), metrics.CollectOptions{
104+
cmds = append(cmds, collectSnapshot(ctx, m.sortBy, m.snap, m.processSampleEvery(), m.cfg.ProcLimit, metrics.CollectOptions{
105105
SkipGPU: m.cfg.NoGPU,
106106
SkipTemp: m.cfg.NoTemp,
107107
}))
@@ -217,22 +217,7 @@ func (m Model) View() string {
217217
}
218218

219219
// Render panels at appropriate widths
220-
cpuPanel := ui.RenderCPU(m.snap.CPU, colL, m.cpuHistory)
221-
gpuPanel := ui.RenderGPU(m.snap.GPU, colR, m.gpuHistory)
222-
tempPanel := ui.RenderTemperature(m.snap.Temperature, colR)
223-
netPanel := ui.RenderNetwork(m.netDelta, colL)
224-
diskPanel := ui.RenderDisk(m.diskDelta, m.snap.Disk, colR)
225-
226-
// Memory panel width depends on whether GPU is present (left vs right column)
227-
var memPanel string
228-
if twoCol && gpuPanel != "" {
229-
memPanel = ui.RenderMemory(m.snap.Memory, m.snap.Load, colL, m.memHistory)
230-
} else if twoCol {
231-
// No GPU: memory goes to the right of CPU
232-
memPanel = ui.RenderMemory(m.snap.Memory, m.snap.Load, colR, m.memHistory)
233-
} else {
234-
memPanel = ui.RenderMemory(m.snap.Memory, m.snap.Load, colL, m.memHistory)
235-
}
220+
metricsSection := m.buildMetricsSection(colL, colR, twoCol)
236221

237222
// Filter processes and resolve PID-based selection.
238223
procs := m.filteredProcesses()
@@ -248,57 +233,6 @@ func (m Model) View() string {
248233
TotalProcs: len(m.snap.Processes),
249234
}
250235

251-
// Build the metric panels section
252-
var metricRows []string
253-
254-
if twoCol {
255-
// Two-column layout: pair panels side by side with matched heights
256-
// Row 1: CPU | GPU (or Memory if no GPU)
257-
if gpuPanel != "" {
258-
metricRows = append(metricRows, joinPanelRow(cpuPanel, gpuPanel, colL, colR))
259-
} else {
260-
metricRows = append(metricRows, joinPanelRow(cpuPanel, memPanel, colL, colR))
261-
}
262-
263-
// Row 2: Memory | Temperature (only if GPU existed in row 1)
264-
if gpuPanel != "" {
265-
if tempPanel != "" {
266-
metricRows = append(metricRows, joinPanelRow(memPanel, tempPanel, colL, colR))
267-
} else {
268-
metricRows = append(metricRows, memPanel)
269-
}
270-
} else if tempPanel != "" {
271-
metricRows = append(metricRows, tempPanel)
272-
}
273-
274-
// Row 3: Network | Disk
275-
if netPanel != "" && diskPanel != "" {
276-
metricRows = append(metricRows, joinPanelRow(netPanel, diskPanel, colL, colR))
277-
} else if netPanel != "" {
278-
metricRows = append(metricRows, netPanel)
279-
} else if diskPanel != "" {
280-
metricRows = append(metricRows, diskPanel)
281-
}
282-
} else {
283-
// Single-column stacked layout
284-
metricRows = append(metricRows, cpuPanel)
285-
if gpuPanel != "" {
286-
metricRows = append(metricRows, gpuPanel)
287-
}
288-
metricRows = append(metricRows, memPanel)
289-
if tempPanel != "" {
290-
metricRows = append(metricRows, tempPanel)
291-
}
292-
if netPanel != "" {
293-
metricRows = append(metricRows, netPanel)
294-
}
295-
if diskPanel != "" {
296-
metricRows = append(metricRows, diskPanel)
297-
}
298-
}
299-
300-
metricsSection := lipgloss.JoinVertical(lipgloss.Left, metricRows...)
301-
302236
// Count lines used by fixed panels to size the process panel.
303237
usedLines := strings.Count(header, "\n") + 1
304238
usedLines += strings.Count(metricsSection, "\n") + 1
@@ -507,9 +441,6 @@ func (m Model) computeUsedLines() int {
507441
w = 80
508442
}
509443

510-
// Header is always 1 line
511-
usedLines := 1
512-
513444
twoCol := w >= 110
514445
var colL, colR int
515446
if twoCol {
@@ -520,6 +451,20 @@ func (m Model) computeUsedLines() int {
520451
colR = w
521452
}
522453

454+
metricsSection := m.buildMetricsSection(colL, colR, twoCol)
455+
456+
// Header is always 1 line
457+
usedLines := 1
458+
usedLines += strings.Count(metricsSection, "\n") + 1
459+
usedLines += 1 // help bar
460+
461+
return usedLines
462+
}
463+
464+
// buildMetricsSection renders all metric panels and joins them into a single string.
465+
// This is the single source of truth for panel layout, used by both View() and
466+
// computeUsedLines() to avoid duplication.
467+
func (m Model) buildMetricsSection(colL, colR int, twoCol bool) string {
523468
cpuPanel := ui.RenderCPU(m.snap.CPU, colL, m.cpuHistory)
524469
gpuPanel := ui.RenderGPU(m.snap.GPU, colR, m.gpuHistory)
525470
tempPanel := ui.RenderTemperature(m.snap.Temperature, colR)
@@ -536,6 +481,7 @@ func (m Model) computeUsedLines() int {
536481
}
537482

538483
var metricRows []string
484+
539485
if twoCol {
540486
if gpuPanel != "" {
541487
metricRows = append(metricRows, joinPanelRow(cpuPanel, gpuPanel, colL, colR))
@@ -575,11 +521,7 @@ func (m Model) computeUsedLines() int {
575521
}
576522
}
577523

578-
metricsSection := lipgloss.JoinVertical(lipgloss.Left, metricRows...)
579-
usedLines += strings.Count(metricsSection, "\n") + 1
580-
usedLines += 1 // help bar
581-
582-
return usedLines
524+
return lipgloss.JoinVertical(lipgloss.Left, metricRows...)
583525
}
584526

585527
// computeProcDataY returns the Y line where process data rows begin on screen.
@@ -591,6 +533,13 @@ func (m Model) computeProcDataY() int {
591533

592534
func (m Model) handleSearchKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
593535
switch msg.Type {
536+
case tea.KeyCtrlC:
537+
m.quitting = true
538+
if m.collectCancel != nil {
539+
m.collectCancel()
540+
m.collectCancel = nil
541+
}
542+
return m, tea.Quit
594543
case tea.KeyEscape:
595544
m.searching = false
596545
m.searchQuery = ""
@@ -724,9 +673,9 @@ func tick(d time.Duration) tea.Cmd {
724673
})
725674
}
726675

727-
func collectSnapshot(ctx context.Context, sortBy metrics.SortField, previous metrics.Snapshot, processSampleEvery time.Duration, opts metrics.CollectOptions) tea.Cmd {
676+
func collectSnapshot(ctx context.Context, sortBy metrics.SortField, previous metrics.Snapshot, processSampleEvery time.Duration, procLimit int, opts metrics.CollectOptions) tea.Cmd {
728677
return func() tea.Msg {
729-
snap := metrics.Collect(ctx, 200*time.Millisecond, sortBy, 50, processSampleEvery, previous, opts)
678+
snap := metrics.Collect(ctx, 200*time.Millisecond, sortBy, procLimit, processSampleEvery, previous, opts)
730679
return snapshotMsg(snap)
731680
}
732681
}
@@ -766,12 +715,18 @@ func (m Model) killSelectedProcess(sig killSignal) string {
766715
}
767716

768717
func (m Model) exportSnapshot() string {
769-
filename := fmt.Sprintf("hideTop_%s.json", m.snap.CollectedAt.Format("20060102_150405"))
718+
basename := fmt.Sprintf("hideTop_%s.json", m.snap.CollectedAt.Format("20060102_150405"))
719+
// Write to user's home directory for a predictable location.
720+
home, err := os.UserHomeDir()
721+
if err != nil {
722+
home = "."
723+
}
724+
filename := home + string(os.PathSeparator) + basename
770725
data, err := json.MarshalIndent(m.snap, "", " ")
771726
if err != nil {
772727
return fmt.Sprintf("export error: %v", err)
773728
}
774-
if err := os.WriteFile(filename, data, 0o644); err != nil {
729+
if err := os.WriteFile(filename, data, 0o600); err != nil {
775730
return fmt.Sprintf("export error: %v", err)
776731
}
777732
return fmt.Sprintf("exported to %s", filename)

internal/config/config.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ type Config struct {
1616
NoGPU bool
1717
NoTemp bool
1818
FilterUsers []string
19+
ProcLimit int
1920
}
2021

2122
// DefaultFilterUsers is used when no custom filter is configured.
@@ -29,6 +30,7 @@ type fileConfig struct {
2930
NoTemp bool `json:"no_temp"`
3031
Debug bool `json:"debug"`
3132
FilterUsers []string `json:"filter_users"`
33+
ProcLimit int `json:"proc_limit"`
3234
}
3335

3436
func Parse() Config {
@@ -40,6 +42,7 @@ func Parse() Config {
4042
theme := flag.String("theme", "", "color theme (dark, light, dracula, nord, monokai)")
4143
noGPU := flag.Bool("no-gpu", false, "disable GPU metrics")
4244
noTemp := flag.Bool("no-temp", false, "disable temperature metrics")
45+
procLimit := flag.Int("proc-limit", 0, "max number of processes to display (0 = 50)")
4346
flag.Parse()
4447

4548
cfg := Config{
@@ -49,6 +52,7 @@ func Parse() Config {
4952
Theme: *theme,
5053
NoGPU: *noGPU,
5154
NoTemp: *noTemp,
55+
ProcLimit: *procLimit,
5256
}
5357

5458
// Load config file (flags take precedence)
@@ -85,6 +89,14 @@ func Parse() Config {
8589
cfg.FilterUsers = DefaultFilterUsers
8690
}
8791

92+
// Apply proc_limit from config file if not set via CLI
93+
if fc != nil && cfg.ProcLimit == 0 && fc.ProcLimit > 0 {
94+
cfg.ProcLimit = fc.ProcLimit
95+
}
96+
if cfg.ProcLimit <= 0 {
97+
cfg.ProcLimit = 50
98+
}
99+
88100
return cfg
89101
}
90102

internal/metrics/battery.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,7 @@ func collectBatteryDarwin(_ context.Context) (BatteryStats, error) {
7676

7777
s := string(out)
7878
stats := BatteryStats{}
79-
80-
// Parse: "Now drawing from 'Battery Power'" or "'AC Power'"
81-
if strings.Contains(s, "'AC Power'") {
82-
stats.Charging = true
83-
}
79+
onAC := strings.Contains(s, "'AC Power'")
8480

8581
// Parse percentage: "InternalBattery-0 (id=...) 85%; charging; ..."
8682
for _, line := range strings.Split(s, "\n") {
@@ -116,6 +112,9 @@ func collectBatteryDarwin(_ context.Context) (BatteryStats, error) {
116112
case strings.Contains(lower, "charged"):
117113
stats.Status = "Full"
118114
stats.Charging = false
115+
default:
116+
// No explicit status found; infer from power source
117+
stats.Charging = onAC
119118
}
120119
break
121120
}

internal/metrics/collector.go

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,15 @@ func Collect(
3737
)
3838

3939
processesDue := shouldCollectProcesses(now, processSampleEvery, sortBy, previous)
40-
workers := 5 // cpu, memory, load, network, disk, battery — adjusted below
40+
41+
// Count workers: cpu + memory + load + network + disk + battery = 6
42+
workers := 6
4143
if !opts.SkipTemp {
4244
workers++
4345
}
44-
workers++ // battery
46+
if !opts.SkipGPU {
47+
workers++
48+
}
4549
if processesDue {
4650
workers++
4751
} else {
@@ -180,17 +184,27 @@ func Collect(
180184
}()
181185
}
182186

187+
if !opts.SkipGPU {
188+
go func() {
189+
defer wg.Done()
190+
g := gpu.Collect(ctx, 0) // cpuTotal not needed for raw GPU metrics
191+
mu.Lock()
192+
defer mu.Unlock()
193+
if g.Available {
194+
snap.GPU = &g
195+
} else if previous.GPU != nil && previous.GPU.Available {
196+
snap.GPU = previous.GPU
197+
snap.Status.GPU = staleStatus(errors.New("collector unavailable"))
198+
}
199+
}()
200+
}
201+
183202
wg.Wait()
184203

185-
// GPU collection runs after CPU so it can use cpuTotal for energy impact.
186-
if !opts.SkipGPU {
187-
g := gpu.Collect(ctx, snap.CPU.Total)
188-
if g.Available {
189-
snap.GPU = &g
190-
} else if previous.GPU != nil && previous.GPU.Available {
191-
snap.GPU = previous.GPU
192-
snap.Status.GPU = staleStatus(errors.New("collector unavailable"))
193-
}
204+
// Compute energy impact after all metrics are collected, since it
205+
// depends on both CPU and GPU utilization.
206+
if snap.GPU != nil && snap.GPU.Available {
207+
snap.GPU.Energy = gpu.ComputeEnergyImpact(snap.CPU.Total, snap.GPU.Utilization, true, snap.GPU.Thermal)
194208
}
195209

196210
return snap

0 commit comments

Comments
 (0)