feat: host grouping with bug fixes and hardening

[amir20]

Summary

Adds host-grouping support (sidebar grouping, collapse state, group log route, agent endpoint group parsing) and applies post-review fixes:

• URL-encode group names in useHostGroupStream and decode them with url.PathUnescape server-side so groups containing spaces, slashes, or other special characters stream correctly
• Reject empty group on /api/host-groups/{group}/logs/stream instead of silently matching every ungrouped host
• Reject empty address in parseEndpoint (e.g. |name|group) so misconfigured agents fail fast
• Hoist the visibleKeys Map in HostGroupLog.vue so it isn't re-allocated every render, restoring memoization in ViewerWithSource

Test plan

• go test ./internal/agent/... ./internal/web/... ./internal/container/...
• pnpm typecheck
• Manually verify a host group with a space in its name streams logs end-to-end
• Visual snapshots pass in CI

🤖 Generated with Claude Code

[claude]

Review: host grouping feature

Solid implementation overall. A few issues worth addressing:

Bug: Inconsistent Name fallback for unavailable agents

• internal/agent/client.go:360-362 — error path sets Name: c.nameOverride, which is "" when no override is given. The host appears with a blank name in the sidebar.
• internal/support/docker/retriable_client_manager.go:713-715 — failed agents (never connected) correctly fall back to addr as the name.

Both code paths should apply the same fallback: use the override if set, otherwise derive a sensible name from the address.

Code smell: duplicated endpoint parsing

parseEndpoint() in client.go and the inline SplitN block in retriable_client_manager.go:711-721 duplicate the same |-splitting logic. Exporting parseEndpoint (or moving it to the container package) would remove the drift risk.

Minor: wrong button title for single-group collapse

HostMenu.vue:70 — the collapse button inside a group header shows :title="$t('label.collapse-all')". This collapses one group, not all of them; the title is misleading.

Minor: no feedback for unknown group route

host-group/[name].vue — navigating to /host-group/does-not-exist renders nothing (the v-if in HostGroupLog hides content) with no 404 or "group not found" message and no page title set.

Good:

• URL encoding/decoding (encodeURIComponent + url.PathUnescape) is correct
• Empty group rejection in streamHostGroupLogs prevents the silent match-all bug
• Empty address rejection in parseEndpoint catches misconfigured agents early
• visibleKeys hoisting in HostGroupLog.vue restores memoization correctly
• watchEffect → watch conversion in HostMenu.vue is a clear improvement
• Test coverage for the new parsing paths is thorough

[claude]

Code Review

• HostGroupLog.vue:252 — $t("label.host-count", ...) and tooltip.merge-all (line 64) are not added to any locale file in this diff; they may render as empty strings or raw keys at runtime.

• HostMenu.vue:69 — the inner collapse button title uses $t("label.collapse-all") but only collapses one host group, not all groups. Misleading tooltip; should be something like "label.collapse-group".

• retriable_client_manager.go:711-720 — endpoint parsing (SplitN + manual extraction) duplicates the logic in parseEndpoint(). A shared helper would eliminate the drift risk (e.g. SplitN silently truncates 4-part strings there while Split + bounds check in parseEndpoint rejects them).

• HostMenu.vue:288 — visibleKeys = new Map<string[], boolean>() hoisting is correct per the PR description; good fix.

• parseEndpoint / ParseConnection both parse |-delimited format but live in separate packages with slightly different validation (one rejects empty address, the other doesn't). Worth a comment noting the intentional split.

• Tests cover the happy paths and the rejection of extra segments — good coverage for parseEndpoint.

🤖 Generated with Claude Code

[claude]

Code Review

• HostGroupLog.vue:259 — label.host-count is used but not added to any locale file in this PR; will render raw key in all locales
• locales/*.yml — label.collapse-group added only to en.yml, missing from all other 15 locale files; non-English users get a raw key
• retriable_client_manager.go:722 — ID: endpoint still stores the full addr|name|group string for failed agents; successful agents get a Docker-derived hostname, so the IDs will never match if an agent recovers
• host.go:668 — ParseConnection silently accepts an empty third segment (addr|name|), producing Group: ""; ParseEndpoint rejects empty address but has the same gap for group — consider consistent trim/reject
• HostMenu.vue:73-76 — collapse button inside <summary> is always visible and always shows a collapse icon even when the group is already collapsed; clicking a collapsed group's button is a no-op with no feedback

[claude]

Review: host grouping with bug fixes and hardening

• internal/container/host.go ParseConnection: no empty-address guard (e.g. |name|group) — unlike ParseEndpoint which rejects parts[0] == "". Silently produces an empty hostname.

• assets/components/HostViewer/HostGroupLog.vue:297 — new Map<string[], boolean>(): array keys are compared by reference in JS, not value, so two distinct string[] arrays with the same content won't match. Fine if ViewerWithSource always reuses the same key references, but worth verifying that assumption holds.

• HostGroupLog.vue:258 uses $t("label.host-count", groupHosts.length) — host-count doesn't appear in any locale file in this diff. Is this key already defined upstream, or is it missing?

• No unit tests for ParseEndpoint edge cases: "addr|name|" (empty trailing group), "|name|group" (empty address — currently accepted by ParseConnection but rejected by ParseEndpoint).

• HostMenu.vue:163 allCollapsed returns false when there are zero groups of either kind, so the "Collapse all" button renders but clicking it is a no-op with no visual feedback. Minor UX rough edge.

Good fixes overall: watchEffect→watch with explicit deps is cleaner, URL encoding/decoding is correct, backend empty-group guard prevents silent match-all, and the visibleKeys hoist restores memoization correctly.

[claude]

Review: host grouping PR

• HostMenu.vue:158-163 — collapseHostGroup() adds to collapsedHostGroups and relies on Vue re-rendering :open on <details>. If the browser fires a toggle event when .open is set programmatically, updateCollapsedHostGroups will run again (double-toggle). Low risk (most browsers don't fire toggle on programmatic .open changes) but worth testing the per-group collapse button specifically.
• host-group/[name].vue:345 — setTitle(route.params.name + " group") skips i18n; other pages use $t for this.
• HostGroupLog.vue:303 — new Map<string[], boolean>() uses string[] as key type; JS Map uses reference equality for object keys so this Map can never have populated entries. If ViewerWithSource expects a non-empty Map, this will silently ignore visible-key state. Fine if an empty Map is the intended stable default.
• ParseConnection (container/host.go) and ParseEndpoint (agent/client.go) are near-identical — minor duplication.

Solid otherwise: URL-encode/decode for groups, empty-group rejection in streamHostGroupLogs, nil-guard on Env in FromProto, and the watchEffect→watch fix are all correct.

🤖 Generated with Claude Code