Skip to content

Commit 13ec351

Browse files
committed
Merge remote-tracking branch 'origin/main' into feature/45-held-packages
# Conflicts: # gearbox-agent/internal/gears/updates/updates_test.go
2 parents aae0a24 + ced440a commit 13ec351

32 files changed

Lines changed: 2534 additions & 337 deletions

CLAUDE.md

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -102,26 +102,9 @@ When a new feature is planned or a bug is reported:
102102
7. **Track Progress** — Keep the project board and issues in sync
103103
8. **Complete** — When user confirms done: merge PR, close issue, move project card to Done
104104

105-
### Branch Naming Convention
105+
### Branching, PR, and label conventions
106106

107-
- **Features:** `feature/short-description` (e.g., `feature/dashboard-export`)
108-
- **Bug fixes:** `fix/short-description` (e.g., `fix/websocket-reconnect`)
109-
- **Always branch from `main`**
110-
111-
### PR Convention
112-
113-
- All PRs target `main`
114-
- PR body must include `Closes #<issue-number>` to auto-close the issue on merge
115-
- Use the standard PR template format (Summary, Test Plan)
116-
117-
### Issue Labels
118-
119-
Use these labels consistently:
120-
121-
- `enhancement` — New features
122-
- `bug` — Bug fixes
123-
- `documentation` — Docs-only changes
124-
- `refactor` — Code improvements without behavior change
107+
Generic branching/PR/label rules live in `~/.claude/CLAUDE.md`. All of them apply here unmodified — `feature/...` and `fix/...` branches from `main`, `Closes #N` in PR bodies, the four standard labels.
125108

126109
### Project Board
127110

@@ -151,14 +134,6 @@ TASKS.md is a **scratch pad only** — not a tracking system. The GitHub Project
151134
- `/dowork` - Read TASKS.md, create issues from it, and start working (ask questions as needed)
152135
- `/doallwork` - Read TASKS.md, create issues from it, and work autonomously
153136

154-
## User Preferences
155-
156-
### Code Block Formatting
157-
158-
- **Always use fenced code blocks** (triple backticks) for commands
159-
- Fenced blocks provide a copy button in the IDE
160-
- Never use inline code for commands the user should run
161-
162137
## UI Conventions
163138

164139
### Modal dialogs — never use native `confirm()`, `alert()`, or `prompt()`
@@ -200,6 +175,28 @@ await showAlertDialog({
200175

201176
Existing reference usages: [user-pages/admin-user-detail.js](static/js/user-pages/admin-user-detail.js), [user-pages/profile-management.js](static/js/user-pages/profile-management.js), [haproxy_config/editor.js](static/js/haproxy_config/editor.js).
202177

178+
### Toggle switches — never use a bare `<input type="checkbox">` in templates
179+
180+
For every boolean input in a `.templ` file — feature opt-ins, settings, "enable this gear", "show all", per-row enable/disable — use the shared slider component in [internal/framework/ui/toggle.templ](gearbox/internal/framework/ui/toggle.templ):
181+
182+
```templ
183+
import "github.com/sarg3nt/gearbox/internal/framework/ui"
184+
185+
// Single toggle (no inline label — pair with your own <label for=...>)
186+
@ui.Toggle("welcome-gear-home", "gears", "home", false, false)
187+
// args: id, name, value (submitted when checked), checked, disabled
188+
189+
// Toggle + label + description, stacked horizontally
190+
@ui.ToggleWithLabel("notify-email", "notify_email", "1", "Email notifications", "Send a digest each morning", true, false)
191+
```
192+
193+
**Rules of thumb:**
194+
195+
- The underlying input is `sr-only` but real — it submits with the form and respects `checked` / `disabled`. No JS required for plain forms.
196+
- Pass `value` when multiple toggles share a `name` (e.g., a multi-select checkbox group posting as `name="gears"`). Leave empty when a single boolean field submits as the default `"on"`.
197+
- For AJAX toggles that POST on change (no enclosing form), the per-row `gear-toggle` `<button role="switch">` pattern in [gears.templ](gearbox/internal/framework/templates/pages/gears.templ) is the established alternative — but for **anything inside a `<form>`**, use `@ui.Toggle`.
198+
- Never inline `peer-checked:after:...` Tailwind salads in a new template — that's a sign you should be calling `@ui.Toggle`. Existing inline copies in `overview.templ` and `admin_user_permissions.templ` are tech debt; migrate them when you're already editing those files.
199+
203200
## Key Constraints
204201

205202
### NEVER
@@ -219,13 +216,9 @@ Existing reference usages: [user-pages/admin-user-detail.js](static/js/user-page
219216
- Inform user before running `make deploy` for agent
220217
- Validate HAProxy config with `haproxy -c` before reload (if applicable)
221218

222-
### Markdown Linting
223-
224-
Run `npx markdownlint-cli '**/*.md' --config .markdownlint.json` to validate. Key rules: blank lines around lists/code blocks, specify language for code blocks, proper headings, single newline at EOF.
225-
226-
### Creating New Documentation
219+
### Markdown and docs
227220

228-
Store in `docs/` directory using kebab-case naming. Include TOC after main heading. Reference in README.md.
221+
See `~/.claude/CLAUDE.md` for markdown style and the `docs/` + kebab-case rule. This repo also requires a TOC after the main heading in new docs.
229222

230223
### Creating Reports and Scan Results
231224

DESIGN.md

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,53 @@ Each gear is self-contained: it defines its own routes, handlers, templates, and
7171

7272
Gears progress through a state machine: `disabled``alpha``beta``production`. Alpha and beta gears must be explicitly enabled by the user. Production gears are enabled by default. The `disabled` state excludes the gear from the build entirely.
7373

74-
### Dashboard Gears (7)
75-
76-
| Gear | Purpose |
77-
|--------------|----------------------------------------------|
78-
| HAProxy | HAProxy overview, status grid, and backend/frontend/server monitoring |
79-
| Metrics | Historical CPU, memory, disk, network charts |
80-
| Services | Systemd service monitoring and control |
81-
| Certificates | TLS certificate expiration tracking |
82-
| Logs | Real-time log viewing and search |
83-
| Traffic | Traffic analysis and GeoIP visualization |
84-
| Alerts | Alert rules, notifications, and history |
74+
### Gear Scopes
75+
76+
Each gear declares a `Scope` controlling where its rows live in the database
77+
and how the sidebar treats it (see [`internal/framework/gear/interface.go`](gearbox/internal/framework/gear/interface.go)):
78+
79+
- **`ScopeBox`** *(default)* — one row per (box_id, gear_name) in the gears
80+
table. The gear is shown in the sidebar only when an active box context
81+
is set, because its UI is meaningless without one. Examples: HAProxy,
82+
Metrics, Logs, Services, Certificates, Traffic, Alerts, OS Updates.
83+
- **`ScopeSystem`** — a single install-wide row keyed by the
84+
`SystemServerID` sentinel. The gear is always visible in the sidebar and
85+
ignores box context. Example: the Home dashboard.
86+
- **`ScopeBoxAgnostic`** — install-wide like `ScopeSystem` but
87+
semantically the gear lists or aggregates *across* boxes rather than
88+
ignoring them. Always visible; box-specific gears are hidden when no box
89+
is active because this gear is the place to pick one. Example: the Bx
90+
fleet view.
91+
92+
### Multi-box UX
93+
94+
A single Gearbox dashboard connects to many agents. The user picks which box
95+
they are "in" via two affordances backed by the same `?box_id=<id>` query
96+
convention:
97+
98+
1. **Bx fleet view** at `/bx` — a `ScopeBoxAgnostic` gear that lists every
99+
configured box with live status dots and click-through to that box.
100+
2. **Persistent box-switcher chip** in the chrome — opens a Cockpit-style
101+
command palette (search + arrow-keys; `g b` shortcut) for jumping
102+
between boxes mid-task without losing place.
103+
104+
When no box is selected, box-scoped gears are hidden from the sidebar; the
105+
user sees only `Bx`, `Home`, and `Settings`. Selecting a box hydrates the
106+
sidebar with that box's enabled gears.
107+
108+
### Dashboard Gears (8)
109+
110+
| Gear | Scope | Purpose |
111+
|--------------|--------------|-----------------------------------------------|
112+
| Bx | box-agnostic | Fleet overview: list + status + switcher home |
113+
| Home | system | App dashboard with launcher tiles and widgets |
114+
| HAProxy | box | HAProxy overview, status grid, and backend/frontend/server monitoring |
115+
| Metrics | box | Historical CPU, memory, disk, network charts |
116+
| Services | box | Systemd service monitoring and control |
117+
| Certificates | box | TLS certificate expiration tracking |
118+
| Logs | box | Real-time log viewing and search |
119+
| Traffic | box | Traffic analysis and GeoIP visualization |
120+
| Alerts | box | Alert rules, notifications, and history |
85121

86122
### Agent Gears (7)
87123

gearbox-agent/deploy/gearbox-agent.service

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ Environment=HOME=/root
1717
Environment=HAPROXY_AGENT_LISTEN=0.0.0.0:8405
1818
Environment=HAPROXY_AGENT_DATA_DIR=/var/lib/gearbox-agent
1919
Environment=HAPROXY_AGENT_LOG_LEVEL=info
20-
# pipx derives its log dir from XDG_STATE_HOME ($XDG_STATE_HOME/pipx/log).
21-
# Redirect to /tmp so ProtectHome=read-only doesn't cause pipx to crash before
22-
# listing packages. PrivateTmp=true ensures cleanup on service stop.
23-
Environment=XDG_STATE_HOME=/tmp/xdg-state
2420

2521
# Optional: Load additional environment from file
2622
EnvironmentFile=-/etc/default/gearbox-agent
@@ -34,8 +30,19 @@ RestartSec=5
3430
# access to /etc (HAProxy config, certificates) and /var (package management,
3531
# data dir). ProtectHome=read-only is set since the agent runs as root and only
3632
# needs read access to home directories.
33+
#
34+
# pipx upgrades write new package files into /root/.local/share/pipx/venvs/<pkg>/,
35+
# pipx logs to /root/.local/state/pipx/log/, pipx CLI shims live at /root/.local/bin/,
36+
# and pip caches downloads under /root/.cache/pip/ — all inside the otherwise
37+
# read-only /root. We grant the broader XDG data/cache parents (/root/.local and
38+
# /root/.cache) rather than narrower per-tool subdirs because pipx and pip create
39+
# their state/log/cache subdirectories on demand at first use, and the mkdir of
40+
# e.g. /root/.local/state/pipx/ requires the /root/.local/state/ parent to be
41+
# writable. Nothing security-sensitive lives under either XDG dir on this host.
42+
# The "-" prefix tolerates the paths not existing yet.
3743
NoNewPrivileges=false
3844
ProtectHome=read-only
45+
ReadWritePaths=-/root/.local -/root/.cache
3946
PrivateTmp=true
4047
ProtectKernelTunables=true
4148

gearbox-agent/internal/gears/updates/updates.go

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,23 @@ func (c *UpdatesCollector) runPipxCommand(args ...string) ([]byte, error) {
222222
}
223223

224224
// runPipxCommandWithOutput runs a pipx command and wraps failures with the
225-
// command's output, just like runCommandWithOutput does for regular commands.
225+
// command's output. Pipx's failure messages don't follow apt's "E:" convention,
226+
// so we use a pipx-tuned extractor that surfaces multiple trailing lines rather
227+
// than just the final "<long-path>/python -m pip install --upgrade pkg -q' failed"
228+
// summary line — that line on its own is missing the actual pip diagnostic.
229+
//
230+
// On failure the full output is also logged via slog at Warn level so operators
231+
// can dig into the agent log for full pip stdout/stderr; the returned error is
232+
// shorter and dashboard-friendly.
226233
func (c *UpdatesCollector) runPipxCommandWithOutput(args ...string) ([]byte, error) {
227234
output, err := c.runPipxCommand(args...)
228235
if err != nil {
229-
errDetail := extractErrorLines(output)
236+
slog.Warn("pipx command failed",
237+
"args", args,
238+
"err", err,
239+
"output", strings.TrimSpace(string(output)),
240+
)
241+
errDetail := extractPipxErrorDetail(output)
230242
if errDetail != "" {
231243
return output, errors.New(errDetail)
232244
}
@@ -235,6 +247,50 @@ func (c *UpdatesCollector) runPipxCommandWithOutput(args ...string) ([]byte, err
235247
return output, nil
236248
}
237249

250+
// extractPipxErrorDetail collects the most useful diagnostic lines from pipx
251+
// output for surfacing in a wrapped error. Unlike apt, pipx's failure summary
252+
// is typically the final line ("'<python> -m pip install --upgrade pkg -q' failed")
253+
// while the cause (network error, dependency conflict, missing module, etc.)
254+
// is on lines above it. We therefore return up to the last few non-empty lines
255+
// joined with "; ", capped at a reasonable display length.
256+
//
257+
// No prefix-based filtering: pip diagnostics legitimately use "Failed to …"
258+
// prefixes too (e.g. "Failed to build wheels for cryptography"), and those
259+
// are exactly the actionable lines we want to surface.
260+
func extractPipxErrorDetail(output []byte) string {
261+
if len(output) == 0 {
262+
return ""
263+
}
264+
265+
text := strings.TrimSpace(string(output))
266+
if text == "" {
267+
return ""
268+
}
269+
270+
const maxLines = 5
271+
const maxTotalLen = 500
272+
273+
rawLines := strings.Split(text, "\n")
274+
var picked []string
275+
for i := len(rawLines) - 1; i >= 0 && len(picked) < maxLines; i-- {
276+
trimmed := strings.TrimSpace(rawLines[i])
277+
if trimmed == "" {
278+
continue
279+
}
280+
picked = append([]string{trimmed}, picked...)
281+
}
282+
283+
if len(picked) == 0 {
284+
return ""
285+
}
286+
287+
joined := strings.Join(picked, "; ")
288+
if len(joined) > maxTotalLen {
289+
joined = joined[:maxTotalLen] + "..."
290+
}
291+
return joined
292+
}
293+
238294
// CheckUpdates retrieves the current update status.
239295
func (c *UpdatesCollector) CheckUpdates() (*UpdateInfo, error) {
240296
return c.PM().CheckUpdates()

gearbox-agent/internal/gears/updates/updates_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,3 +526,82 @@ func TestApplyHeldMarks(t *testing.T) {
526526
}
527527
})
528528
}
529+
530+
// TestExtractPipxErrorDetail verifies that pipx error extraction surfaces the
531+
// last few meaningful lines of pipx output rather than just the final
532+
// "'...python -m pip install --upgrade pkg -q' failed" summary — which on its
533+
// own loses the diagnostic that explains *why* the upgrade failed.
534+
func TestExtractPipxErrorDetail(t *testing.T) {
535+
tests := []struct {
536+
name string
537+
output string
538+
wantSubs []string // every substring must appear in the result
539+
notWant []string // none of these may appear
540+
empty bool // expect empty string
541+
}{
542+
{
543+
name: "empty input",
544+
output: "",
545+
empty: true,
546+
},
547+
{
548+
name: "whitespace only",
549+
output: " \n\t\n \n",
550+
empty: true,
551+
},
552+
{
553+
name: "real-world pipx failure surfaces cause and summary",
554+
output: "Upgrading certbot...\n" +
555+
"ERROR: Could not find a version that satisfies the requirement certbot==9.99\n" +
556+
"ERROR: No matching distribution found for certbot==9.99\n" +
557+
"'/root/.local/share/pipx/venvs/certbot/bin/python -m pip install --upgrade certbot -q' failed",
558+
wantSubs: []string{
559+
"No matching distribution found",
560+
"--upgrade certbot -q' failed",
561+
},
562+
},
563+
{
564+
name: "surfaces Failed to lines from pip (e.g. build-wheel failures)",
565+
output: "Collecting cryptography\n" +
566+
"Failed to build wheels for cryptography\n" +
567+
"'/root/.local/share/pipx/venvs/foo/bin/python -m pip install --upgrade foo -q' failed",
568+
wantSubs: []string{
569+
"Failed to build wheels for cryptography",
570+
"--upgrade foo -q' failed",
571+
},
572+
},
573+
{
574+
name: "caps at 5 lines",
575+
output: "l1\nl2\nl3\nl4\nl5\nl6\nl7",
576+
wantSubs: []string{"l3", "l4", "l5", "l6", "l7"},
577+
notWant: []string{"l1", "l2"},
578+
},
579+
{
580+
name: "truncates very long combined output",
581+
output: strings.Repeat("a", 600),
582+
wantSubs: []string{"..."},
583+
},
584+
}
585+
586+
for _, tc := range tests {
587+
t.Run(tc.name, func(t *testing.T) {
588+
got := extractPipxErrorDetail([]byte(tc.output))
589+
if tc.empty {
590+
if got != "" {
591+
t.Errorf("expected empty, got %q", got)
592+
}
593+
return
594+
}
595+
for _, sub := range tc.wantSubs {
596+
if !strings.Contains(got, sub) {
597+
t.Errorf("missing substring %q in result %q", sub, got)
598+
}
599+
}
600+
for _, sub := range tc.notWant {
601+
if strings.Contains(got, sub) {
602+
t.Errorf("unexpected substring %q in result %q", sub, got)
603+
}
604+
}
605+
})
606+
}
607+
}

0 commit comments

Comments
 (0)