From 85b3b9a77ae40e4472380fb4a1e972de0fd7bb30 Mon Sep 17 00:00:00 2001 From: Tejender Upadhyay Date: Wed, 13 May 2026 14:33:52 +0200 Subject: [PATCH 1/2] feat(tools): boot-diagnosis pack v1 + dmesg fix + filter args MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the headline gaps from a live boot-triage exercise against the fleet. The user reported that "why is boot slow?" was unanswerable because read_dmesg was broken, the journal couldn't reach across boots, and there was no systemd-analyze / systemctl surface at all. read_dmesg / read_journal - Drop the broken `--read-clear=false` argv from read_dmesg (the flag has no boolean form; this was always erroring out). - read_journal gains `boot int` (mapped to journalctl -b N, validated -10..0) so "show me the previous boot" is a one-field call instead of arithmetic on uptime. - read_journal gains `match string` (mapped to journalctl --grep, validated as printable-ASCII, length-capped, must compile in Go). systemd-analyze suite (new boot.go) - boot_time parses `systemd-analyze time` into typed seconds per phase (firmware/loader/kernel/initrd/userspace/total) plus the target-reached line. Phases that don't apply on a given host are reported as 0 rather than failing the call. - boot_blame parses `systemd-analyze blame` and sorts descending; supports `top:N` (default 50, max 500). - boot_critical_chain parses the unit tree, returning a flat list of {unit, active_at_seconds, startup_seconds} alongside the raw text. - Composite durations like "1min 23.456s" parse correctly; the duration regex is shared via a `durRE` constant. systemctl surface (new systemd.go) - list_systemd_units calls `systemctl list-units --output=json` with validated state/type filters and a configurable limit (default 500, cap 2000). - unit_status combines `systemctl show` (parsed as key=value, embedded '=' values preserved) with a tail of the unit's journal. - list_timers parses `systemctl list-timers --output=json`. The wire format emits next/left/last/passed as epoch microseconds (numbers, not strings); fields are typed int64 and renamed _micros for clarity. Filter ergonomics (Sig-9) - list_processes: top:N (alias for limit, smaller wins), name_regex (comm match), state (R/S/D/Z/T/I). - list_mounts: fstype, mount_point_regex. - list_block_devices: name_regex (top-level), fields []string (project device entries to a subset of keys, applied recursively to children). Tools registry / catalogue - AllToolNames grows 19 → 25 so fleet peers publish the new surface. - README catalogue table extended; the six DBus-using tools are flagged with a snap-confinement caveat: inside the strictly-confined snap they return isError with "Failed to connect to bus: Permission denied" because the current plug list (system-observe etc.) does not grant DBus to systemd's system bus. They work normally when the daemon is run as a plain binary outside the snap. Adding a narrow DBus interface is deferred per the prior plug-strictness call. Tests - New boot_test.go covers parseDurationSecs (composite + unknown suffix), parseBootTime (typical, no-firmware-VM, with-initrd), parseBlame (sort + ms/s/min mix), parseCriticalChain (depth + startup time + zero-startup leaf). - New systemd_test.go covers listUnitsArgs (filters + validation) and parseSystemctlShow (value containing '='). - Extended logs_test.go for the new boot/match validations. Verification: go build / go vet / gofmt / go test -race ./... clean. Smoke-tested every new tool end-to-end via MCP tools/call against a local binary and against the live fleetmind-a/b LXD VMs. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 13 +- internal/tools/block.go | 97 +++++++++++- internal/tools/boot.go | 265 +++++++++++++++++++++++++++++++++ internal/tools/boot_test.go | 121 +++++++++++++++ internal/tools/logs.go | 29 +++- internal/tools/logs_test.go | 40 +++++ internal/tools/mount.go | 28 +++- internal/tools/process.go | 65 +++++++- internal/tools/systemd.go | 218 +++++++++++++++++++++++++++ internal/tools/systemd_test.go | 62 ++++++++ internal/tools/tools.go | 8 + 11 files changed, 933 insertions(+), 13 deletions(-) create mode 100644 internal/tools/boot.go create mode 100644 internal/tools/boot_test.go create mode 100644 internal/tools/systemd.go create mode 100644 internal/tools/systemd_test.go diff --git a/README.md b/README.md index 7012e4d..1ad7b67 100644 --- a/README.md +++ b/README.md @@ -25,9 +25,20 @@ The server speaks the [Streamable HTTP MCP transport][mcp-spec] on | `list_pci_devices`, `list_usb_devices` | Device IDs and bound kernel drivers | | `kernel_info`, `list_kernel_modules` | Kernel build, cmdline, loaded modules | | `list_dmi`, `list_sensors` | SMBIOS/DMI strings; hwmon temperatures, voltages, fans | -| `read_journal`, `read_dmesg` | Recent journald and kernel ring-buffer entries | +| `read_journal`, `read_dmesg` | Recent journald and kernel ring-buffer entries. `read_journal` supports `boot:-N` for previous-boot reads and `match` for server-side regex grep. | +| `boot_time`, `boot_blame`, `boot_critical_chain` *(†)* | Parsed `systemd-analyze` outputs: per-phase boot timings, per-unit init time, and the critical chain that gated the default target | +| `list_systemd_units`, `unit_status`, `list_timers` *(†)* | systemd unit inventory (with `state`/`type` filters), per-unit detail + journal tail, and all scheduled timers | | `list_fleet` | Every MCP server the local node sees in its fleet (fleet mode) | +*(†) Snap-confinement note:* the `systemd-analyze` and `systemctl` family +require D-Bus access to the systemd system bus, which the current strict +plug list (`system-observe` etc.) does not grant. These six tools therefore +return `isError: true` with `Failed to connect to bus: Permission denied` +inside the snap; they work normally when the daemon is run as a plain binary +outside the snap (the README's "Building locally" recipe). Adding a D-Bus +plug to expose them in the snap is tracked as a follow-up — see the +deferred-items section of the plan in `.claude/plans/`. + ## Building locally Requires Go ≥ 1.25. diff --git a/internal/tools/block.go b/internal/tools/block.go index 2e797f9..bd9b2c1 100644 --- a/internal/tools/block.go +++ b/internal/tools/block.go @@ -4,11 +4,15 @@ import ( "context" "encoding/json" "fmt" + "regexp" "github.com/modelcontextprotocol/go-sdk/mcp" ) -type listBlockIn struct{} +type listBlockIn struct { + NameRegex string `json:"name_regex,omitempty" jsonschema:"keep only top-level devices whose name matches this regex (max 200 chars). Children are kept unfiltered."` + Fields []string `json:"fields,omitempty" jsonschema:"project each device entry to only these keys (max 32 entries). Empty = return every key."` +} type listBlockOut struct { // Devices is the parsed lsblk JSON object. lsblk -J emits @@ -20,8 +24,22 @@ type listBlockOut struct { func registerBlock(s *mcp.Server, d Deps) { mcp.AddTool(s, &mcp.Tool{ Name: "list_block_devices", - Description: "Block-device tree as reported by `lsblk -J -O` (disks, partitions, LVM, RAID, holders).", - }, func(ctx context.Context, _ *mcp.CallToolRequest, _ listBlockIn) (*mcp.CallToolResult, listBlockOut, error) { + Description: "Block-device tree as reported by `lsblk -J -O` (disks, partitions, LVM, RAID, holders). Optional: name_regex filters top-level devices; fields projects each entry to a subset of keys.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in listBlockIn) (*mcp.CallToolResult, listBlockOut, error) { + var nameRE *regexp.Regexp + if in.NameRegex != "" { + if len(in.NameRegex) > 200 { + return nil, listBlockOut{}, fmt.Errorf("name_regex too long (%d > 200)", len(in.NameRegex)) + } + re, err := regexp.Compile(in.NameRegex) + if err != nil { + return nil, listBlockOut{}, fmt.Errorf("name_regex does not compile: %w", err) + } + nameRE = re + } + if len(in.Fields) > 32 { + return nil, listBlockOut{}, fmt.Errorf("fields too long (%d > 32)", len(in.Fields)) + } stdout, _, err := d.Exec.Run(ctx, "lsblk", "-J", "-O") if err != nil { return nil, listBlockOut{}, fmt.Errorf("lsblk: %w", err) @@ -30,6 +48,79 @@ func registerBlock(s *mcp.Server, d Deps) { if err := json.Unmarshal(stdout, &parsed); err != nil { return nil, listBlockOut{}, fmt.Errorf("parse lsblk output: %w", err) } + if nameRE != nil || len(in.Fields) > 0 { + parsed = filterBlockDevices(parsed, nameRE, in.Fields) + } return textResult("lsblk: %d bytes of JSON", len(stdout)), listBlockOut{Devices: parsed}, nil }) } + +// filterBlockDevices applies name_regex (top level only) and fields projection +// (applied recursively to every device dict) to the parsed lsblk output. +func filterBlockDevices(in map[string]any, nameRE *regexp.Regexp, fields []string) map[string]any { + out := map[string]any{} + devs, ok := in["blockdevices"].([]any) + if !ok { + return in + } + var keep []any + for _, raw := range devs { + m, ok := raw.(map[string]any) + if !ok { + continue + } + if nameRE != nil { + name, _ := m["name"].(string) + if !nameRE.MatchString(name) { + continue + } + } + keep = append(keep, projectDevice(m, fields)) + } + out["blockdevices"] = keep + return out +} + +// projectDevice returns a copy of m with only the keys in fields (if any), +// recursing into the "children" array. "name" is always preserved so children +// remain identifiable even with a narrow projection. +func projectDevice(m map[string]any, fields []string) map[string]any { + if len(fields) == 0 { + // Still recurse so projection applied at higher level propagates. + if kids, ok := m["children"].([]any); ok { + out := make(map[string]any, len(m)) + for k, v := range m { + out[k] = v + } + projected := make([]any, 0, len(kids)) + for _, kid := range kids { + if km, ok := kid.(map[string]any); ok { + projected = append(projected, projectDevice(km, fields)) + } + } + out["children"] = projected + return out + } + return m + } + wanted := map[string]struct{}{"name": {}} + for _, f := range fields { + wanted[f] = struct{}{} + } + out := map[string]any{} + for k, v := range m { + if _, ok := wanted[k]; ok { + out[k] = v + } + } + if kids, ok := m["children"].([]any); ok { + projected := make([]any, 0, len(kids)) + for _, kid := range kids { + if km, ok := kid.(map[string]any); ok { + projected = append(projected, projectDevice(km, fields)) + } + } + out["children"] = projected + } + return out +} diff --git a/internal/tools/boot.go b/internal/tools/boot.go new file mode 100644 index 0000000..5fd22e7 --- /dev/null +++ b/internal/tools/boot.go @@ -0,0 +1,265 @@ +package tools + +import ( + "context" + "fmt" + "regexp" + "sort" + "strconv" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// -------- boot_time ---------------------------------------------------------- + +type bootTimeIn struct{} + +type bootTimeOut struct { + FirmwareSec float64 `json:"firmware_seconds,omitempty"` + LoaderSec float64 `json:"loader_seconds,omitempty"` + KernelSec float64 `json:"kernel_seconds,omitempty"` + InitrdSec float64 `json:"initrd_seconds,omitempty"` + UserspaceSec float64 `json:"userspace_seconds,omitempty"` + TotalSec float64 `json:"total_seconds"` + TargetReached string `json:"target_reached,omitempty"` + TargetReachedSec float64 `json:"target_reached_seconds,omitempty"` + Raw string `json:"raw"` +} + +// -------- boot_blame --------------------------------------------------------- + +type bootBlameIn struct { + Top int `json:"top,omitempty" jsonschema:"return only the top N slowest units (default 50, max 500)"` +} + +type blameEntry struct { + Unit string `json:"unit"` + InitSeconds float64 `json:"init_seconds"` +} + +type bootBlameOut struct { + Count int `json:"count"` + Entries []blameEntry `json:"entries"` +} + +// -------- boot_critical_chain ----------------------------------------------- + +type bootCriticalChainIn struct{} + +type chainEntry struct { + Unit string `json:"unit"` + ActiveAtSeconds float64 `json:"active_at_seconds,omitempty"` + StartupSeconds float64 `json:"startup_seconds,omitempty"` +} + +type bootCriticalChainOut struct { + Units []chainEntry `json:"units"` + Raw string `json:"raw"` +} + +func registerBoot(s *mcp.Server, d Deps) { + mcp.AddTool(s, &mcp.Tool{ + Name: "boot_time", + Description: "Parsed `systemd-analyze time` output: per-phase boot timings " + + "(firmware, loader, kernel, initrd, userspace) and the total. " + + "Phases that don't apply on this host are reported as 0 (e.g. firmware on a VM).", + }, func(ctx context.Context, _ *mcp.CallToolRequest, _ bootTimeIn) (*mcp.CallToolResult, bootTimeOut, error) { + stdout, _, err := d.Exec.Run(ctx, "systemd-analyze", "time") + if err != nil { + return nil, bootTimeOut{}, fmt.Errorf("systemd-analyze time: %w", err) + } + out := parseBootTime(string(stdout)) + return textResult("boot total %.2fs (kernel %.2fs · userspace %.2fs)", + out.TotalSec, out.KernelSec, out.UserspaceSec), out, nil + }) + + mcp.AddTool(s, &mcp.Tool{ + Name: "boot_blame", + Description: "Parsed `systemd-analyze blame` output: per-unit initialization time, " + + "sorted descending. Use `top` to cap the response (default 50).", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in bootBlameIn) (*mcp.CallToolResult, bootBlameOut, error) { + top := in.Top + if top <= 0 { + top = 50 + } + if top > 500 { + return nil, bootBlameOut{}, fmt.Errorf("top must be <= 500, got %d", top) + } + stdout, _, err := d.Exec.Run(ctx, "systemd-analyze", "blame") + if err != nil { + return nil, bootBlameOut{}, fmt.Errorf("systemd-analyze blame: %w", err) + } + entries := parseBlame(string(stdout)) + if len(entries) > top { + entries = entries[:top] + } + return textResult("blame: %d unit(s) returned", len(entries)), + bootBlameOut{Count: len(entries), Entries: entries}, nil + }) + + mcp.AddTool(s, &mcp.Tool{ + Name: "boot_critical_chain", + Description: "Parsed `systemd-analyze critical-chain`: the serialized chain of units " + + "that gated the default target (typically multi-user.target). Each entry carries " + + "the cumulative active-at time and the unit's own startup time when systemd reports it.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, _ bootCriticalChainIn) (*mcp.CallToolResult, bootCriticalChainOut, error) { + stdout, _, err := d.Exec.Run(ctx, "systemd-analyze", "critical-chain", "--no-pager") + if err != nil { + return nil, bootCriticalChainOut{}, fmt.Errorf("systemd-analyze critical-chain: %w", err) + } + raw := string(stdout) + units := parseCriticalChain(raw) + return textResult("critical-chain: %d unit(s)", len(units)), + bootCriticalChainOut{Units: units, Raw: raw}, nil + }) +} + +// ---------- parsers (kept pure so they can be unit-tested) ------------------- + +// durRE matches one or more composite tokens (e.g. "5.219s", "1min 23.456s") +// without consuming trailing whitespace. +const durRE = `[0-9]+(?:\.[0-9]+)?(?:y|month|w|d|h|min|ms|s)(?:\s+[0-9]+(?:\.[0-9]+)?(?:y|month|w|d|h|min|ms|s))*` + +var ( + // "5.219s (firmware)" / "234ms (loader)" / "1min 23.456s (userspace)". + segmentRE = regexp.MustCompile(`(` + durRE + `)\s+\(([a-z]+)\)`) + // "= 19.935s" (total, after the chain of segments). + totalRE = regexp.MustCompile(`=\s+(` + durRE + `)`) + // "multi-user.target reached after 12.123s in userspace." + targetRE = regexp.MustCompile(`(\S+\.target)\s+reached after\s+(` + durRE + `)\s+in (?:userspace|initrd)`) + // Units inside critical-chain tree lines: " @ [+]". + chainUnitRE = regexp.MustCompile(`([A-Za-z0-9@:_.\-]+\.(?:target|service|socket|timer|mount|swap|path|slice|scope|device))(?:\s+@(` + durRE + `))?(?:\s+\+(` + durRE + `))?`) +) + +func parseBootTime(s string) bootTimeOut { + out := bootTimeOut{Raw: s} + for _, m := range segmentRE.FindAllStringSubmatch(s, -1) { + secs, ok := parseDurationSecs(m[1]) + if !ok { + continue + } + switch m[2] { + case "firmware": + out.FirmwareSec = secs + case "loader": + out.LoaderSec = secs + case "kernel": + out.KernelSec = secs + case "initrd": + out.InitrdSec = secs + case "userspace": + out.UserspaceSec = secs + } + } + if m := totalRE.FindStringSubmatch(s); m != nil { + if secs, ok := parseDurationSecs(m[1]); ok { + out.TotalSec = secs + } + } + if m := targetRE.FindStringSubmatch(s); m != nil { + out.TargetReached = m[1] + if secs, ok := parseDurationSecs(m[2]); ok { + out.TargetReachedSec = secs + } + } + return out +} + +func parseBlame(s string) []blameEntry { + var out []blameEntry + for _, line := range strings.Split(s, "\n") { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + continue + } + fields := strings.Fields(trimmed) + if len(fields) < 2 { + continue + } + unit := fields[len(fields)-1] + durStr := strings.Join(fields[:len(fields)-1], " ") + secs, ok := parseDurationSecs(durStr) + if !ok { + continue + } + out = append(out, blameEntry{Unit: unit, InitSeconds: secs}) + } + sort.SliceStable(out, func(i, j int) bool { + return out[i].InitSeconds > out[j].InitSeconds + }) + return out +} + +func parseCriticalChain(s string) []chainEntry { + var out []chainEntry + for _, line := range strings.Split(s, "\n") { + m := chainUnitRE.FindStringSubmatch(line) + if m == nil { + continue + } + e := chainEntry{Unit: m[1]} + if m[2] != "" { + if v, ok := parseDurationSecs(m[2]); ok { + e.ActiveAtSeconds = v + } + } + if m[3] != "" { + if v, ok := parseDurationSecs(m[3]); ok { + e.StartupSeconds = v + } + } + out = append(out, e) + } + return out +} + +// parseDurationSecs accepts the composite duration forms systemd-analyze emits: +// "234ms", "1.234s", "1min 23.456s", "2h 3min 4s". Returns total seconds. +// Unknown suffixes (e.g. "y", "month") are tolerated but contribute 0 — boot +// timings should never reach those scales, and if they do the user can read +// the Raw field directly. +func parseDurationSecs(s string) (float64, bool) { + s = strings.TrimSpace(s) + if s == "" { + return 0, false + } + var total float64 + seen := false + for _, f := range strings.Fields(s) { + mult, num, ok := splitDuration(f) + if !ok { + return 0, false + } + total += num * mult + seen = true + } + return total, seen +} + +func splitDuration(f string) (mult, num float64, ok bool) { + // Order matters: "ms" must be checked before "s", "min" before "n", etc. + suffixes := []struct { + s string + mult float64 + }{ + {"ms", 0.001}, + {"min", 60}, + {"month", 30 * 24 * 3600}, + {"s", 1}, + {"h", 3600}, + {"d", 24 * 3600}, + {"w", 7 * 24 * 3600}, + {"y", 365 * 24 * 3600}, + } + for _, suf := range suffixes { + if strings.HasSuffix(f, suf.s) { + v, err := strconv.ParseFloat(strings.TrimSuffix(f, suf.s), 64) + if err != nil { + return 0, 0, false + } + return suf.mult, v, true + } + } + return 0, 0, false +} diff --git a/internal/tools/boot_test.go b/internal/tools/boot_test.go new file mode 100644 index 0000000..6478bea --- /dev/null +++ b/internal/tools/boot_test.go @@ -0,0 +1,121 @@ +package tools + +import ( + "math" + "testing" +) + +func almost(a, b float64) bool { return math.Abs(a-b) < 1e-6 } + +func TestParseDurationSecs(t *testing.T) { + cases := []struct { + in string + want float64 + ok bool + }{ + {"234ms", 0.234, true}, + {"1.234s", 1.234, true}, + {"1min 23.456s", 83.456, true}, + {"2h 3min 4s", 2*3600 + 3*60 + 4, true}, + {"", 0, false}, + {"garbage", 0, false}, + {"5xy", 0, false}, + } + for _, c := range cases { + got, ok := parseDurationSecs(c.in) + if ok != c.ok || !almost(got, c.want) { + t.Errorf("parseDurationSecs(%q) = (%v, %v); want (%v, %v)", c.in, got, ok, c.want, c.ok) + } + } +} + +func TestParseBootTime(t *testing.T) { + t.Run("typical bare-metal", func(t *testing.T) { + input := "Startup finished in 5.219s (firmware) + 234ms (loader) + 2.137s (kernel) + 12.345s (userspace) = 19.935s\n" + + "multi-user.target reached after 12.123s in userspace.\n" + out := parseBootTime(input) + if !almost(out.FirmwareSec, 5.219) { + t.Errorf("firmware = %v", out.FirmwareSec) + } + if !almost(out.LoaderSec, 0.234) { + t.Errorf("loader = %v", out.LoaderSec) + } + if !almost(out.KernelSec, 2.137) { + t.Errorf("kernel = %v", out.KernelSec) + } + if !almost(out.UserspaceSec, 12.345) { + t.Errorf("userspace = %v", out.UserspaceSec) + } + if !almost(out.TotalSec, 19.935) { + t.Errorf("total = %v", out.TotalSec) + } + if out.TargetReached != "multi-user.target" { + t.Errorf("target = %q", out.TargetReached) + } + if !almost(out.TargetReachedSec, 12.123) { + t.Errorf("target_reached = %v", out.TargetReachedSec) + } + }) + t.Run("vm without firmware/loader", func(t *testing.T) { + input := "Startup finished in 1.876s (kernel) + 4.234s (userspace) = 6.110s\n" + out := parseBootTime(input) + if out.FirmwareSec != 0 || out.LoaderSec != 0 { + t.Errorf("expected zero firmware/loader, got %v/%v", out.FirmwareSec, out.LoaderSec) + } + if !almost(out.TotalSec, 6.110) { + t.Errorf("total = %v", out.TotalSec) + } + }) + t.Run("with initrd", func(t *testing.T) { + input := "Startup finished in 1s (kernel) + 2s (initrd) + 3s (userspace) = 6s\n" + out := parseBootTime(input) + if !almost(out.InitrdSec, 2) { + t.Errorf("initrd = %v", out.InitrdSec) + } + }) +} + +func TestParseBlame(t *testing.T) { + input := "12.345s NetworkManager-wait-online.service\n" + + " 4.567s systemd-networkd-wait-online.service\n" + + " 234ms snapd.service\n" + + "1min 23.456s some-slow.service\n" + + "\n" + got := parseBlame(input) + if len(got) != 4 { + t.Fatalf("got %d entries, want 4", len(got)) + } + // Must be sorted descending. + if got[0].Unit != "some-slow.service" || !almost(got[0].InitSeconds, 83.456) { + t.Errorf("entry[0] = %+v", got[0]) + } + if got[1].Unit != "NetworkManager-wait-online.service" || !almost(got[1].InitSeconds, 12.345) { + t.Errorf("entry[1] = %+v", got[1]) + } + if got[3].Unit != "snapd.service" || !almost(got[3].InitSeconds, 0.234) { + t.Errorf("entry[3] = %+v", got[3]) + } +} + +func TestParseCriticalChain(t *testing.T) { + input := "The time when unit became active or started is printed after the \"@\" character.\n" + + "The time the unit took to start is printed after the \"+\" character.\n" + + "\n" + + "multi-user.target @23.476s\n" + + "└─NetworkManager.service @17.234s +6.234s\n" + + " └─dbus.service @17.123s\n" + + " └─basic.target @17.012s\n" + got := parseCriticalChain(input) + if len(got) != 4 { + t.Fatalf("got %d units, want 4: %+v", len(got), got) + } + if got[0].Unit != "multi-user.target" || !almost(got[0].ActiveAtSeconds, 23.476) { + t.Errorf("got[0] = %+v", got[0]) + } + if got[1].Unit != "NetworkManager.service" || !almost(got[1].ActiveAtSeconds, 17.234) || !almost(got[1].StartupSeconds, 6.234) { + t.Errorf("got[1] = %+v", got[1]) + } + if got[3].StartupSeconds != 0 { + t.Errorf("got[3] should have no startup seconds: %+v", got[3]) + } +} diff --git a/internal/tools/logs.go b/internal/tools/logs.go index 8530c1c..c70e4c5 100644 --- a/internal/tools/logs.go +++ b/internal/tools/logs.go @@ -19,6 +19,8 @@ type readJournalIn struct { Lines int `json:"lines,omitempty" jsonschema:"how many recent lines to return (default 100, max 5000)"` Priority string `json:"priority,omitempty" jsonschema:"minimum priority: emerg|alert|crit|err|warning|notice|info|debug"` Since string `json:"since,omitempty" jsonschema:"timestamp accepted by --since (e.g. '1h ago', '2026-05-13 09:00')"` + Boot int `json:"boot,omitempty" jsonschema:"boot offset for journalctl -b (0 = current boot, -1 = previous boot, …). Valid range -10..0."` + Match string `json:"match,omitempty" jsonschema:"PCRE regex passed to journalctl --grep (max 200 chars)"` } type readJournalOut struct { @@ -62,7 +64,10 @@ func registerLogs(s *mcp.Server, d Deps) { if lines > 5000 { return nil, readDmesgOut{}, errors.New("lines must be <= 5000") } - stdout, _, err := d.Exec.Run(ctx, "dmesg", "--time-format=iso", "--color=never", "--read-clear=false") + // dmesg is read-only-no-clear by default. We deliberately do NOT pass + // --read-clear (which is the consume-and-clear flag, has no boolean + // argument, and would also require an extra capability). + stdout, _, err := d.Exec.Run(ctx, "dmesg", "--time-format=iso", "--color=never") if err != nil { return nil, readDmesgOut{}, fmt.Errorf("dmesg: %w", err) } @@ -104,9 +109,31 @@ func journalArgs(in readJournalIn) ([]string, error) { } args = append(args, "--since", in.Since) } + if in.Boot != 0 { + if in.Boot < -10 || in.Boot > 0 { + return nil, fmt.Errorf("boot must be in [-10, 0], got %d", in.Boot) + } + args = append(args, "-b", strconv.Itoa(in.Boot)) + } + if in.Match != "" { + if len(in.Match) > 200 { + return nil, fmt.Errorf("match regex too long (%d > 200)", len(in.Match)) + } + if !matchRE.MatchString(in.Match) { + return nil, fmt.Errorf("invalid match regex %q (control chars or non-printable bytes rejected)", in.Match) + } + if _, err := regexp.Compile(in.Match); err != nil { + return nil, fmt.Errorf("match regex does not compile: %w", err) + } + args = append(args, "--grep", in.Match) + } return args, nil } +// matchRE is a safety screen for --grep input: printable ASCII only. We still +// compile the regex in Go to reject patterns journalctl would also refuse. +var matchRE = regexp.MustCompile(`^[\x20-\x7E]+$`) + // sinceRE accepts ASCII timestamps and the common human-readable forms // journalctl supports. Anything more exotic is rejected to keep argv tight. var sinceRE = regexp.MustCompile(`^[A-Za-z0-9:\-+ ]{1,40}$`) diff --git a/internal/tools/logs_test.go b/internal/tools/logs_test.go index 21b34c8..a63b457 100644 --- a/internal/tools/logs_test.go +++ b/internal/tools/logs_test.go @@ -44,6 +44,46 @@ func TestJournalArgs(t *testing.T) { t.Fatal("expected error for excessive lines") } }) + t.Run("accepts boot offset", func(t *testing.T) { + args, err := journalArgs(readJournalIn{Boot: -1}) + if err != nil { + t.Fatalf("err = %v", err) + } + joined := strings.Join(args, " ") + if !strings.Contains(joined, "-b -1") { + t.Errorf("missing -b -1: %q", joined) + } + }) + t.Run("rejects out-of-range boot offset", func(t *testing.T) { + if _, err := journalArgs(readJournalIn{Boot: 1}); err == nil { + t.Fatal("expected error for boot=1") + } + if _, err := journalArgs(readJournalIn{Boot: -11}); err == nil { + t.Fatal("expected error for boot=-11") + } + }) + t.Run("accepts match regex", func(t *testing.T) { + args, err := journalArgs(readJournalIn{Match: "failed|error"}) + if err != nil { + t.Fatalf("err = %v", err) + } + joined := strings.Join(args, " ") + if !strings.Contains(joined, "--grep failed|error") { + t.Errorf("missing --grep: %q", joined) + } + }) + t.Run("rejects bad match regex", func(t *testing.T) { + if _, err := journalArgs(readJournalIn{Match: "abc\x00def"}); err == nil { + t.Fatal("expected error for non-printable bytes") + } + if _, err := journalArgs(readJournalIn{Match: "[unclosed"}); err == nil { + t.Fatal("expected error for uncompilable regex") + } + big := strings.Repeat("a", 201) + if _, err := journalArgs(readJournalIn{Match: big}); err == nil { + t.Fatal("expected error for too-long regex") + } + }) } func TestTailLines(t *testing.T) { diff --git a/internal/tools/mount.go b/internal/tools/mount.go index 5f79cf1..6ca0971 100644 --- a/internal/tools/mount.go +++ b/internal/tools/mount.go @@ -2,12 +2,17 @@ package tools import ( "context" + "fmt" + "regexp" "syscall" "github.com/modelcontextprotocol/go-sdk/mcp" ) -type listMountsIn struct{} +type listMountsIn struct { + Fstype string `json:"fstype,omitempty" jsonschema:"keep only mounts of this fstype (e.g. ext4, btrfs, squashfs)"` + MountPointRegex string `json:"mount_point_regex,omitempty" jsonschema:"keep only mounts whose mount point matches this regex (max 200 chars)"` +} type mountOut struct { MountPoint string `json:"mount_point"` @@ -29,14 +34,31 @@ type listMountsOut struct { func registerMount(s *mcp.Server, d Deps) { mcp.AddTool(s, &mcp.Tool{ Name: "list_mounts", - Description: "All mount points from /proc/self/mountinfo, enriched with statfs sizes for normal filesystems.", - }, func(_ context.Context, _ *mcp.CallToolRequest, _ listMountsIn) (*mcp.CallToolResult, listMountsOut, error) { + Description: "All mount points from /proc/self/mountinfo, enriched with statfs sizes for normal filesystems. Optional filters: fstype, mount_point_regex.", + }, func(_ context.Context, _ *mcp.CallToolRequest, in listMountsIn) (*mcp.CallToolResult, listMountsOut, error) { + var mpRE *regexp.Regexp + if in.MountPointRegex != "" { + if len(in.MountPointRegex) > 200 { + return nil, listMountsOut{}, fmt.Errorf("mount_point_regex too long (%d > 200)", len(in.MountPointRegex)) + } + re, err := regexp.Compile(in.MountPointRegex) + if err != nil { + return nil, listMountsOut{}, fmt.Errorf("mount_point_regex does not compile: %w", err) + } + mpRE = re + } entries, err := d.ProcFS.Mounts() if err != nil { return nil, listMountsOut{}, err } out := listMountsOut{Mounts: make([]mountOut, 0, len(entries))} for _, e := range entries { + if in.Fstype != "" && e.Type != in.Fstype { + continue + } + if mpRE != nil && !mpRE.MatchString(e.MountPoint) { + continue + } m := mountOut{ MountPoint: e.MountPoint, Source: e.Source, Type: e.Type, Options: e.Options, diff --git a/internal/tools/process.go b/internal/tools/process.go index 4cf7c65..2f2875b 100644 --- a/internal/tools/process.go +++ b/internal/tools/process.go @@ -4,7 +4,9 @@ import ( "context" "errors" "fmt" + "regexp" "sort" + "strings" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -13,8 +15,11 @@ import ( type listProcessesIn struct { // Limit caps the number of processes returned (after sorting by RSS desc). - // 0 means no limit. - Limit int `json:"limit,omitempty" jsonschema:"maximum number of entries to return; 0 = unlimited"` + // 0 means no limit. `top` is an alias kept for ergonomics. + Limit int `json:"limit,omitempty" jsonschema:"maximum number of entries to return; 0 = unlimited"` + Top int `json:"top,omitempty" jsonschema:"alias for limit; if both set, the smaller wins"` + NameRegex string `json:"name_regex,omitempty" jsonschema:"keep only processes whose comm matches this regex (max 200 chars)"` + State string `json:"state,omitempty" jsonschema:"keep only processes in this /proc state: R|S|D|Z|T|I"` } type processOut struct { @@ -42,15 +47,46 @@ type getProcessIn struct { func registerProcess(s *mcp.Server, d Deps) { mcp.AddTool(s, &mcp.Tool{ Name: "list_processes", - Description: "Snapshot of all running processes sorted by resident set size (descending).", + Description: "Snapshot of all running processes sorted by resident set size (descending). Optional filters: name_regex (matches comm), state (R/S/D/Z/T/I), top/limit.", }, func(_ context.Context, _ *mcp.CallToolRequest, in listProcessesIn) (*mcp.CallToolResult, listProcessesOut, error) { + var nameRE *regexp.Regexp + if in.NameRegex != "" { + if len(in.NameRegex) > 200 { + return nil, listProcessesOut{}, fmt.Errorf("name_regex too long (%d > 200)", len(in.NameRegex)) + } + re, err := regexp.Compile(in.NameRegex) + if err != nil { + return nil, listProcessesOut{}, fmt.Errorf("name_regex does not compile: %w", err) + } + nameRE = re + } + if in.State != "" { + if len(in.State) != 1 || !strings.Contains("RSDZTI", strings.ToUpper(in.State)) { + return nil, listProcessesOut{}, fmt.Errorf("invalid state %q (expected one of R/S/D/Z/T/I)", in.State) + } + } ps, err := d.ProcFS.ListProcesses() if err != nil { return nil, listProcessesOut{}, err } sort.Slice(ps, func(i, j int) bool { return ps[i].VMRSS > ps[j].VMRSS }) - if in.Limit > 0 && len(ps) > in.Limit { - ps = ps[:in.Limit] + if nameRE != nil || in.State != "" { + wantState := strings.ToUpper(in.State) + filtered := ps[:0] + for _, p := range ps { + if nameRE != nil && !nameRE.MatchString(p.Comm) { + continue + } + if wantState != "" && !strings.EqualFold(p.State, wantState) { + continue + } + filtered = append(filtered, p) + } + ps = filtered + } + cap := effectiveLimit(in.Limit, in.Top) + if cap > 0 && len(ps) > cap { + ps = ps[:cap] } out := listProcessesOut{Count: len(ps), Processes: make([]processOut, 0, len(ps))} for _, p := range ps { @@ -75,6 +111,25 @@ func registerProcess(s *mcp.Server, d Deps) { }) } +// effectiveLimit returns the smaller of the two positive values, or whichever +// is positive when only one is set. Zero/negative inputs mean "no cap" for +// that side; if both are zero, the result is 0 (unlimited). +func effectiveLimit(limit, top int) int { + switch { + case limit > 0 && top > 0: + if top < limit { + return top + } + return limit + case limit > 0: + return limit + case top > 0: + return top + default: + return 0 + } +} + func toProcessOut(p procfs.ProcessSummary) processOut { return processOut{ PID: p.PID, PPID: p.PPID, Comm: p.Comm, State: p.State, diff --git a/internal/tools/systemd.go b/internal/tools/systemd.go new file mode 100644 index 0000000..807a8eb --- /dev/null +++ b/internal/tools/systemd.go @@ -0,0 +1,218 @@ +package tools + +import ( + "context" + "encoding/json" + "fmt" + "strconv" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// -------- list_systemd_units ------------------------------------------------ + +type listUnitsIn struct { + State string `json:"state,omitempty" jsonschema:"filter by active state: active|inactive|failed|activating|deactivating|reloading"` + Type string `json:"type,omitempty" jsonschema:"filter by unit type: service|timer|socket|mount|target|path|swap|slice|scope|device"` + Limit int `json:"limit,omitempty" jsonschema:"max number of units to return (default 500, max 2000)"` +} + +type systemdUnit struct { + Unit string `json:"unit"` + Load string `json:"load"` + Active string `json:"active"` + Sub string `json:"sub"` + Description string `json:"description"` +} + +type listUnitsOut struct { + Count int `json:"count"` + Units []systemdUnit `json:"units"` +} + +// -------- unit_status ------------------------------------------------------- + +type unitStatusIn struct { + Unit string `json:"unit" jsonschema:"unit name, e.g. NetworkManager.service"` + JournalLines int `json:"journal_lines,omitempty" jsonschema:"how many recent journal entries to include for this unit (default 30, max 500)"` +} + +type unitStatusOut struct { + Unit string `json:"unit"` + Properties map[string]string `json:"properties"` + Journal string `json:"journal,omitempty"` +} + +// -------- list_timers ------------------------------------------------------- + +type listTimersIn struct{} + +// systemdTimer mirrors `systemctl list-timers --output=json` entries. The +// next/left/last/passed fields are emitted as numbers (microseconds since the +// Unix epoch; 0 means "never"), not strings — verified against systemd v255. +type systemdTimer struct { + NextMicros int64 `json:"next_micros"` + LeftMicros int64 `json:"left_micros"` + LastMicros int64 `json:"last_micros"` + PassedMicros int64 `json:"passed_micros"` + Unit string `json:"unit"` + Activates string `json:"activates"` +} + +// rawSystemdTimer is the on-the-wire shape. Mapped into systemdTimer before +// returning so the public field names are explicit about the microsecond unit. +type rawSystemdTimer struct { + Next int64 `json:"next"` + Left int64 `json:"left"` + Last int64 `json:"last"` + Passed int64 `json:"passed"` + Unit string `json:"unit"` + Activates string `json:"activates"` +} + +type listTimersOut struct { + Count int `json:"count"` + Timers []systemdTimer `json:"timers"` +} + +// --------------------------------------------------------------------------- + +func registerSystemd(s *mcp.Server, d Deps) { + mcp.AddTool(s, &mcp.Tool{ + Name: "list_systemd_units", + Description: "All loaded systemd units (`systemctl list-units --output=json`) with " + + "optional state/type filters. Default cap is 500 entries — narrow with `state` " + + "(e.g. \"failed\") or `type` (e.g. \"service\", \"timer\") for focused triage.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in listUnitsIn) (*mcp.CallToolResult, listUnitsOut, error) { + args, err := listUnitsArgs(in) + if err != nil { + return nil, listUnitsOut{}, err + } + stdout, _, err := d.Exec.Run(ctx, "systemctl", args...) + if err != nil { + return nil, listUnitsOut{}, fmt.Errorf("systemctl list-units: %w", err) + } + var parsed []systemdUnit + if err := json.Unmarshal(stdout, &parsed); err != nil { + return nil, listUnitsOut{}, fmt.Errorf("parse systemctl JSON: %w", err) + } + limit := in.Limit + if limit <= 0 { + limit = 500 + } + if limit > 2000 { + return nil, listUnitsOut{}, fmt.Errorf("limit must be <= 2000, got %d", limit) + } + if len(parsed) > limit { + parsed = parsed[:limit] + } + return textResult("%d unit(s) matched", len(parsed)), + listUnitsOut{Count: len(parsed), Units: parsed}, nil + }) + + mcp.AddTool(s, &mcp.Tool{ + Name: "unit_status", + Description: "Detail view of a single systemd unit: every property `systemctl show` " + + "reports, plus a tail of the unit's recent journal. Requires log-observe for journal.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in unitStatusIn) (*mcp.CallToolResult, unitStatusOut, error) { + if !unitRE.MatchString(in.Unit) { + return nil, unitStatusOut{}, fmt.Errorf("invalid unit name %q", in.Unit) + } + jLines := in.JournalLines + if jLines <= 0 { + jLines = 30 + } + if jLines > 500 { + return nil, unitStatusOut{}, fmt.Errorf("journal_lines must be <= 500, got %d", jLines) + } + showOut, _, err := d.Exec.Run(ctx, "systemctl", "show", "--no-pager", in.Unit) + if err != nil { + return nil, unitStatusOut{}, fmt.Errorf("systemctl show %s: %w", in.Unit, err) + } + props := parseSystemctlShow(string(showOut)) + // Journal is best-effort — log-observe may not be connected. + jOut, _, jErr := d.Exec.Run(ctx, "journalctl", "--no-pager", "-o", "short-iso", + "-n", strconv.Itoa(jLines), "--unit", in.Unit) + journal := "" + if jErr == nil { + journal = string(jOut) + } + summary := props["ActiveState"] + if sub := props["SubState"]; sub != "" { + summary = summary + "/" + sub + } + return textResult("%s: %s", in.Unit, summary), + unitStatusOut{Unit: in.Unit, Properties: props, Journal: journal}, nil + }) + + mcp.AddTool(s, &mcp.Tool{ + Name: "list_timers", + Description: "All systemd timers (`systemctl list-timers --output=json`): next/last " + + "fire times and the unit each one activates.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, _ listTimersIn) (*mcp.CallToolResult, listTimersOut, error) { + stdout, _, err := d.Exec.Run(ctx, "systemctl", "list-timers", "--no-pager", "--all", "--output=json") + if err != nil { + return nil, listTimersOut{}, fmt.Errorf("systemctl list-timers: %w", err) + } + var raw []rawSystemdTimer + if err := json.Unmarshal(stdout, &raw); err != nil { + return nil, listTimersOut{}, fmt.Errorf("parse systemctl JSON: %w", err) + } + parsed := make([]systemdTimer, len(raw)) + for i, r := range raw { + parsed[i] = systemdTimer{ + NextMicros: r.Next, LeftMicros: r.Left, + LastMicros: r.Last, PassedMicros: r.Passed, + Unit: r.Unit, Activates: r.Activates, + } + } + return textResult("%d timer(s)", len(parsed)), + listTimersOut{Count: len(parsed), Timers: parsed}, nil + }) +} + +// listUnitsArgs builds the systemctl argv. Kept pure for unit-testing. +func listUnitsArgs(in listUnitsIn) ([]string, error) { + args := []string{"list-units", "--no-pager", "--all", "--output=json"} + if in.State != "" { + allowed := map[string]bool{ + "active": true, "inactive": true, "failed": true, + "activating": true, "deactivating": true, "reloading": true, + } + if !allowed[in.State] { + return nil, fmt.Errorf("invalid state %q", in.State) + } + args = append(args, "--state="+in.State) + } + if in.Type != "" { + allowed := map[string]bool{ + "service": true, "timer": true, "socket": true, "mount": true, + "target": true, "path": true, "swap": true, "slice": true, + "scope": true, "device": true, "automount": true, + } + if !allowed[in.Type] { + return nil, fmt.Errorf("invalid type %q", in.Type) + } + args = append(args, "--type="+in.Type) + } + return args, nil +} + +// parseSystemctlShow consumes the Key=Value\n stream `systemctl show` emits +// and returns a flat string map. Values that contain '=' are preserved +// verbatim after the first delimiter. +func parseSystemctlShow(s string) map[string]string { + out := map[string]string{} + for _, line := range strings.Split(s, "\n") { + if line == "" { + continue + } + k, v, ok := strings.Cut(line, "=") + if !ok { + continue + } + out[k] = v + } + return out +} diff --git a/internal/tools/systemd_test.go b/internal/tools/systemd_test.go new file mode 100644 index 0000000..9530189 --- /dev/null +++ b/internal/tools/systemd_test.go @@ -0,0 +1,62 @@ +package tools + +import ( + "strings" + "testing" +) + +func TestListUnitsArgs(t *testing.T) { + t.Run("defaults", func(t *testing.T) { + args, err := listUnitsArgs(listUnitsIn{}) + if err != nil { + t.Fatalf("err = %v", err) + } + joined := strings.Join(args, " ") + if !strings.Contains(joined, "list-units") || !strings.Contains(joined, "--output=json") { + t.Errorf("unexpected argv: %q", joined) + } + if strings.Contains(joined, "--state") || strings.Contains(joined, "--type") { + t.Errorf("unexpected filter in defaults: %q", joined) + } + }) + t.Run("with filters", func(t *testing.T) { + args, err := listUnitsArgs(listUnitsIn{State: "failed", Type: "service"}) + if err != nil { + t.Fatalf("err = %v", err) + } + joined := strings.Join(args, " ") + if !strings.Contains(joined, "--state=failed") || !strings.Contains(joined, "--type=service") { + t.Errorf("missing filters: %q", joined) + } + }) + t.Run("rejects bad state", func(t *testing.T) { + if _, err := listUnitsArgs(listUnitsIn{State: "panic"}); err == nil { + t.Fatal("expected error for unknown state") + } + }) + t.Run("rejects bad type", func(t *testing.T) { + if _, err := listUnitsArgs(listUnitsIn{Type: "container"}); err == nil { + t.Fatal("expected error for unknown type") + } + }) +} + +func TestParseSystemctlShow(t *testing.T) { + input := "Id=NetworkManager.service\n" + + "LoadState=loaded\n" + + "ActiveState=active\n" + + "SubState=running\n" + + "FragmentPath=/lib/systemd/system/NetworkManager.service\n" + + "Environment=LANG=C.UTF-8\n" + // value containing '=' + "\n" + out := parseSystemctlShow(input) + if out["Id"] != "NetworkManager.service" { + t.Errorf("Id = %q", out["Id"]) + } + if out["ActiveState"] != "active" || out["SubState"] != "running" { + t.Errorf("active/sub = %q/%q", out["ActiveState"], out["SubState"]) + } + if out["Environment"] != "LANG=C.UTF-8" { + t.Errorf("Environment should preserve embedded '=': got %q", out["Environment"]) + } +} diff --git a/internal/tools/tools.go b/internal/tools/tools.go index eaef028..e52ac63 100644 --- a/internal/tools/tools.go +++ b/internal/tools/tools.go @@ -50,6 +50,12 @@ var AllToolNames = []string{ "list_sensors", "read_journal", "read_dmesg", + "boot_time", + "boot_blame", + "boot_critical_chain", + "list_systemd_units", + "unit_status", + "list_timers", "list_fleet", } @@ -68,6 +74,8 @@ func RegisterAll(s *mcp.Server, d Deps) { registerKernel(s, d) registerHardware(s, d) registerLogs(s, d) + registerBoot(s, d) + registerSystemd(s, d) registerFleet(s, d) } From fded47661ace7436e623ddb9a21dc2e667402216 Mon Sep 17 00:00:00 2001 From: Tejender Upadhyay Date: Wed, 13 May 2026 15:29:28 +0200 Subject: [PATCH 2/2] fix(tools): boot-diagnosis pack v1 + dmesg fix + filter args --- README.md | 17 +- go.mod | 1 + go.sum | 2 + internal/sysd/manager.go | 312 +++++++++++++++++++++++++++++++++ internal/tools/boot.go | 302 +++++++++++++++---------------- internal/tools/boot_test.go | 121 ------------- internal/tools/systemd.go | 230 ++++++++++++------------ internal/tools/systemd_test.go | 62 ------- snap/snapcraft.yaml | 8 + 9 files changed, 586 insertions(+), 469 deletions(-) create mode 100644 internal/sysd/manager.go delete mode 100644 internal/tools/boot_test.go delete mode 100644 internal/tools/systemd_test.go diff --git a/README.md b/README.md index 1ad7b67..43f7528 100644 --- a/README.md +++ b/README.md @@ -26,18 +26,15 @@ The server speaks the [Streamable HTTP MCP transport][mcp-spec] on | `kernel_info`, `list_kernel_modules` | Kernel build, cmdline, loaded modules | | `list_dmi`, `list_sensors` | SMBIOS/DMI strings; hwmon temperatures, voltages, fans | | `read_journal`, `read_dmesg` | Recent journald and kernel ring-buffer entries. `read_journal` supports `boot:-N` for previous-boot reads and `match` for server-side regex grep. | -| `boot_time`, `boot_blame`, `boot_critical_chain` *(†)* | Parsed `systemd-analyze` outputs: per-phase boot timings, per-unit init time, and the critical chain that gated the default target | -| `list_systemd_units`, `unit_status`, `list_timers` *(†)* | systemd unit inventory (with `state`/`type` filters), per-unit detail + journal tail, and all scheduled timers | +| `boot_time`, `boot_blame`, `boot_critical_chain` | Per-phase boot timings, per-unit init time, and the critical chain (slowest predecessor at each After= hop) — read directly from `org.freedesktop.systemd1` over the system D-Bus | +| `list_systemd_units`, `unit_status`, `list_timers` | systemd unit inventory (with `state`/`type` filters), per-unit property bag + journal tail, and all timer units with their next/last elapse timestamps | | `list_fleet` | Every MCP server the local node sees in its fleet (fleet mode) | -*(†) Snap-confinement note:* the `systemd-analyze` and `systemctl` family -require D-Bus access to the systemd system bus, which the current strict -plug list (`system-observe` etc.) does not grant. These six tools therefore -return `isError: true` with `Failed to connect to bus: Permission denied` -inside the snap; they work normally when the daemon is run as a plain binary -outside the snap (the README's "Building locally" recipe). Adding a D-Bus -plug to expose them in the snap is tracked as a follow-up — see the -deferred-items section of the plan in `.claude/plans/`. +The six systemd-aware tools talk to `org.freedesktop.systemd1` via the +well-known socket at `/run/dbus/system_bus_socket` (permitted by the base +AppArmor abstraction). Shelling out to `systemd-analyze` / `systemctl` would +not work under strict confinement because they bind abstract sockets outside +the snap's namespace; the direct-D-Bus approach sidesteps that entirely. ## Building locally diff --git a/go.mod b/go.mod index d9ddf7e..6de9a08 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.25.0 require github.com/modelcontextprotocol/go-sdk v1.6.0 require ( + github.com/godbus/dbus/v5 v5.2.2 // indirect github.com/google/jsonschema-go v0.4.3 // indirect github.com/segmentio/asm v1.1.3 // indirect github.com/segmentio/encoding v0.5.4 // indirect diff --git a/go.sum b/go.sum index 091ac51..a181c90 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= +github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= diff --git a/internal/sysd/manager.go b/internal/sysd/manager.go new file mode 100644 index 0000000..6e0942c --- /dev/null +++ b/internal/sysd/manager.go @@ -0,0 +1,312 @@ +// Package sysd is a small read-only wrapper around the systemd D-Bus API +// (org.freedesktop.systemd1). It exists because shelling out to +// `systemd-analyze` / `systemctl` fails under strict snap confinement — the +// helpers try to bind() an abstract Unix socket outside the snap's namespace +// for their own D-Bus client and AppArmor denies it. Talking to the +// well-known system bus socket (/run/dbus/system_bus_socket) is permitted by +// the base profile, so we go direct. +package sysd + +import ( + "fmt" + "os" + "path/filepath" + "sort" + + "github.com/godbus/dbus/v5" +) + +const ( + managerService = "org.freedesktop.systemd1" + managerPath = "/org/freedesktop/systemd1" + managerIface = "org.freedesktop.systemd1.Manager" + unitIface = "org.freedesktop.systemd1.Unit" + timerIface = "org.freedesktop.systemd1.Timer" + propsIface = "org.freedesktop.DBus.Properties" +) + +// Manager is a session-bound handle to the systemd manager object. Use Close +// when finished; one Manager per tool invocation is fine — connection setup +// is sub-millisecond locally. +type Manager struct { + conn *dbus.Conn + mgr dbus.BusObject +} + +// Open returns a Manager bound to the system bus. Caller must Close. +func Open() (*Manager, error) { + conn, err := dbus.ConnectSystemBus() + if err != nil { + return nil, fmt.Errorf("connect system bus: %w", err) + } + return &Manager{ + conn: conn, + mgr: conn.Object(managerService, dbus.ObjectPath(managerPath)), + }, nil +} + +// Close releases the underlying D-Bus connection. +func (m *Manager) Close() error { + if m.conn == nil { + return nil + } + return m.conn.Close() +} + +// BootTimes is the raw monotonic-microsecond view of the boot sequence as +// systemd records it. Fields are 0 when the phase did not occur on this host +// (e.g. firmware/loader on a VM, initrd on a system booted without one). +type BootTimes struct { + FirmwareMonotonicUsec uint64 // microseconds BEFORE kernel boot + LoaderMonotonicUsec uint64 // microseconds BEFORE kernel boot + InitRDMonotonicUsec uint64 // microseconds AFTER kernel boot, when initrd started + UserspaceMonotonicUsec uint64 // microseconds AFTER kernel boot, when userspace started + FinishMonotonicUsec uint64 // microseconds AFTER kernel boot, when default target became active +} + +// BootTimes reads the relevant Manager properties in one trip. +func (m *Manager) BootTimes() (BootTimes, error) { + var bt BootTimes + if err := m.uintProperty("FirmwareTimestampMonotonic", &bt.FirmwareMonotonicUsec); err != nil { + return bt, err + } + if err := m.uintProperty("LoaderTimestampMonotonic", &bt.LoaderMonotonicUsec); err != nil { + return bt, err + } + if err := m.uintProperty("InitRDTimestampMonotonic", &bt.InitRDMonotonicUsec); err != nil { + return bt, err + } + if err := m.uintProperty("UserspaceTimestampMonotonic", &bt.UserspaceMonotonicUsec); err != nil { + return bt, err + } + if err := m.uintProperty("FinishTimestampMonotonic", &bt.FinishMonotonicUsec); err != nil { + return bt, err + } + return bt, nil +} + +// DefaultTarget returns the unit name of the configured default target +// (typically graphical.target or multi-user.target). Resolved by readlink +// rather than Manager.GetDefaultTarget — the latter is blocked by the +// snapd-generated AppArmor profile, which only whitelists a fixed subset of +// Manager methods. +func (m *Manager) DefaultTarget() (string, error) { + for _, p := range []string{ + "/etc/systemd/system/default.target", + "/usr/lib/systemd/system/default.target", + "/lib/systemd/system/default.target", + } { + dst, err := os.Readlink(p) + if err != nil { + continue + } + return filepath.Base(dst), nil + } + return "default.target", nil +} + +// UnitInfo mirrors the tuple Manager.ListUnits returns. Field order matches +// the D-Bus type signature `a(ssssssouso)` exactly — godbus uses field order +// to deserialize. +type UnitInfo struct { + Name string + Description string + LoadState string + ActiveState string + SubState string + Follower string + Path dbus.ObjectPath + JobID uint32 + JobType string + JobPath dbus.ObjectPath +} + +// ListUnits returns every currently-loaded unit. +func (m *Manager) ListUnits() ([]UnitInfo, error) { + var units []UnitInfo + if err := m.mgr.Call(managerIface+".ListUnits", 0).Store(&units); err != nil { + return nil, fmt.Errorf("ListUnits: %w", err) + } + return units, nil +} + +// UnitTimings is the per-unit boot-timing view used by blame and +// critical-chain. Zero values mean "no transition recorded this boot". +type UnitTimings struct { + Name string + InactiveExitMonotonicUsec uint64 + ActiveEnterMonotonicUsec uint64 +} + +// StartupUsec is the unit's "blame" — the time it spent going from inactive +// to active during this boot. Returns 0 when either edge is missing. +func (t UnitTimings) StartupUsec() uint64 { + if t.ActiveEnterMonotonicUsec == 0 || t.InactiveExitMonotonicUsec == 0 { + return 0 + } + if t.ActiveEnterMonotonicUsec <= t.InactiveExitMonotonicUsec { + return 0 + } + return t.ActiveEnterMonotonicUsec - t.InactiveExitMonotonicUsec +} + +// UnitTimings reads the InactiveExit / ActiveEnter monotonic timestamps for +// a single unit path. Pass a path from ListUnits (avoids the LoadUnit cost). +func (m *Manager) UnitTimings(path dbus.ObjectPath, name string) (UnitTimings, error) { + obj := m.conn.Object(managerService, path) + out := UnitTimings{Name: name} + if err := m.uintPropertyOn(obj, unitIface, "InactiveExitTimestampMonotonic", &out.InactiveExitMonotonicUsec); err != nil { + return out, err + } + if err := m.uintPropertyOn(obj, unitIface, "ActiveEnterTimestampMonotonic", &out.ActiveEnterMonotonicUsec); err != nil { + return out, err + } + return out, nil +} + +// UnitAfter returns the After= dependencies of the named unit. Returns an +// empty slice if the unit doesn't exist or has no After= deps. +func (m *Manager) UnitAfter(path dbus.ObjectPath) ([]string, error) { + obj := m.conn.Object(managerService, path) + var v dbus.Variant + if err := obj.Call(propsIface+".Get", 0, unitIface, "After").Store(&v); err != nil { + return nil, fmt.Errorf("Get After: %w", err) + } + var after []string + if err := v.Store(&after); err != nil { + return nil, fmt.Errorf("decode After variant: %w", err) + } + sort.Strings(after) + return after, nil +} + +// LoadUnit returns the object path for the given unit name, loading it into +// memory if it isn't already. +func (m *Manager) LoadUnit(name string) (dbus.ObjectPath, error) { + var path dbus.ObjectPath + if err := m.mgr.Call(managerIface+".LoadUnit", 0, name).Store(&path); err != nil { + return "", fmt.Errorf("LoadUnit %s: %w", name, err) + } + return path, nil +} + +// GetUnit returns the object path for an already-loaded unit. Unlike LoadUnit +// it will not load a unit on demand, but it is on the snapd AppArmor allowlist +// for Manager methods (LoadUnit is not). For units that became active during +// boot (targets, services in the critical chain), they remain loaded, so +// GetUnit is the right call. +func (m *Manager) GetUnit(name string) (dbus.ObjectPath, error) { + var path dbus.ObjectPath + if err := m.mgr.Call(managerIface+".GetUnit", 0, name).Store(&path); err != nil { + return "", fmt.Errorf("GetUnit %s: %w", name, err) + } + return path, nil +} + +// UnitPropertiesAll returns every property the unit publishes on the given +// interface as a map of D-Bus variants flattened to printable strings. We use +// this for unit_status's property bag. +func (m *Manager) UnitPropertiesAll(path dbus.ObjectPath, iface string) (map[string]string, error) { + obj := m.conn.Object(managerService, path) + raw := map[string]dbus.Variant{} + if err := obj.Call(propsIface+".GetAll", 0, iface).Store(&raw); err != nil { + return nil, fmt.Errorf("GetAll(%s): %w", iface, err) + } + out := make(map[string]string, len(raw)) + for k, v := range raw { + out[k] = formatVariant(v) + } + return out, nil +} + +// TimerInfo bundles the timer-specific properties tools need. +type TimerInfo struct { + NextElapseMonotonicUsec uint64 + NextElapseRealtimeUsec uint64 + LastTriggerUsec uint64 + LastTriggerMonotonicUsec uint64 +} + +// TimerProperties reads the four scheduling properties off a *.timer unit. +func (m *Manager) TimerProperties(path dbus.ObjectPath) (TimerInfo, error) { + obj := m.conn.Object(managerService, path) + var info TimerInfo + props := []struct { + key string + dst *uint64 + }{ + {"NextElapseUSecMonotonic", &info.NextElapseMonotonicUsec}, + {"NextElapseUSecRealtime", &info.NextElapseRealtimeUsec}, + {"LastTriggerUSec", &info.LastTriggerUsec}, + {"LastTriggerUSecMonotonic", &info.LastTriggerMonotonicUsec}, + } + for _, p := range props { + // Best-effort: a missing property on older systemd versions shouldn't + // fail the whole call. + _ = m.uintPropertyOn(obj, timerIface, p.key, p.dst) + } + return info, nil +} + +// TriggersUnit reads the Triggers property (the service this timer activates). +func (m *Manager) TriggersUnit(path dbus.ObjectPath) (string, error) { + obj := m.conn.Object(managerService, path) + var v dbus.Variant + if err := obj.Call(propsIface+".Get", 0, unitIface, "Triggers").Store(&v); err != nil { + return "", fmt.Errorf("Get Triggers: %w", err) + } + var triggers []string + if err := v.Store(&triggers); err != nil { + return "", fmt.Errorf("decode Triggers variant: %w", err) + } + if len(triggers) == 0 { + return "", nil + } + return triggers[0], nil +} + +// --- internal helpers -------------------------------------------------------- + +func (m *Manager) uintProperty(name string, dst *uint64) error { + return m.uintPropertyOn(m.mgr, managerIface, name, dst) +} + +func (m *Manager) uintPropertyOn(obj dbus.BusObject, iface, name string, dst *uint64) error { + var v dbus.Variant + if err := obj.Call(propsIface+".Get", 0, iface, name).Store(&v); err != nil { + return fmt.Errorf("Get %s.%s: %w", iface, name, err) + } + if err := v.Store(dst); err != nil { + return fmt.Errorf("decode %s.%s variant: %w", iface, name, err) + } + return nil +} + +// Format a Variant as a printable string. Numeric, string and []string types +// get clean output; everything else falls back to fmt.Sprint of the Value(). +func formatVariant(v dbus.Variant) string { + val := v.Value() + switch t := val.(type) { + case string: + return t + case []string: + return joinStrings(t, " ") + case bool, int8, int16, int32, int64, uint8, uint16, uint32, uint64, float64: + return fmt.Sprint(t) + case dbus.ObjectPath: + return string(t) + default: + return fmt.Sprint(val) + } +} + +func joinStrings(xs []string, sep string) string { + if len(xs) == 0 { + return "" + } + out := xs[0] + for _, s := range xs[1:] { + out += sep + s + } + return out +} diff --git a/internal/tools/boot.go b/internal/tools/boot.go index 5fd22e7..d826855 100644 --- a/internal/tools/boot.go +++ b/internal/tools/boot.go @@ -3,12 +3,11 @@ package tools import ( "context" "fmt" - "regexp" "sort" - "strconv" - "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/gjolly/fleetmind/internal/sysd" ) // -------- boot_time ---------------------------------------------------------- @@ -24,7 +23,6 @@ type bootTimeOut struct { TotalSec float64 `json:"total_seconds"` TargetReached string `json:"target_reached,omitempty"` TargetReachedSec float64 `json:"target_reached_seconds,omitempty"` - Raw string `json:"raw"` } // -------- boot_blame --------------------------------------------------------- @@ -54,31 +52,46 @@ type chainEntry struct { } type bootCriticalChainOut struct { - Units []chainEntry `json:"units"` - Raw string `json:"raw"` + Default string `json:"default_target"` + Units []chainEntry `json:"units"` } -func registerBoot(s *mcp.Server, d Deps) { +func registerBoot(s *mcp.Server, _ Deps) { mcp.AddTool(s, &mcp.Tool{ Name: "boot_time", - Description: "Parsed `systemd-analyze time` output: per-phase boot timings " + - "(firmware, loader, kernel, initrd, userspace) and the total. " + - "Phases that don't apply on this host are reported as 0 (e.g. firmware on a VM).", - }, func(ctx context.Context, _ *mcp.CallToolRequest, _ bootTimeIn) (*mcp.CallToolResult, bootTimeOut, error) { - stdout, _, err := d.Exec.Run(ctx, "systemd-analyze", "time") + Description: "Per-phase boot timings from the systemd manager: firmware/loader " + + "(when known), kernel, initrd (when used), userspace, and the total. " + + "Pulled from org.freedesktop.systemd1 Manager properties over the system " + + "D-Bus, so it works inside strict snap confinement.", + }, func(_ context.Context, _ *mcp.CallToolRequest, _ bootTimeIn) (*mcp.CallToolResult, bootTimeOut, error) { + m, err := sysd.Open() + if err != nil { + return nil, bootTimeOut{}, fmt.Errorf("open systemd bus: %w", err) + } + defer m.Close() + bt, err := m.BootTimes() if err != nil { - return nil, bootTimeOut{}, fmt.Errorf("systemd-analyze time: %w", err) + return nil, bootTimeOut{}, fmt.Errorf("read boot times: %w", err) + } + out := composeBootTime(bt) + if name, err := m.DefaultTarget(); err == nil && name != "" { + out.TargetReached = name + if path, err := m.GetUnit(name); err == nil { + if t, err := m.UnitTimings(path, name); err == nil && t.ActiveEnterMonotonicUsec > 0 && bt.UserspaceMonotonicUsec > 0 { + out.TargetReachedSec = float64(t.ActiveEnterMonotonicUsec-bt.UserspaceMonotonicUsec) / 1e6 + } + } } - out := parseBootTime(string(stdout)) return textResult("boot total %.2fs (kernel %.2fs · userspace %.2fs)", out.TotalSec, out.KernelSec, out.UserspaceSec), out, nil }) mcp.AddTool(s, &mcp.Tool{ Name: "boot_blame", - Description: "Parsed `systemd-analyze blame` output: per-unit initialization time, " + - "sorted descending. Use `top` to cap the response (default 50).", - }, func(ctx context.Context, _ *mcp.CallToolRequest, in bootBlameIn) (*mcp.CallToolResult, bootBlameOut, error) { + Description: "Per-unit initialization time during the current boot, sorted descending. " + + "Computed from each unit's InactiveExit→ActiveEnter monotonic timestamp delta " + + "via D-Bus. `top` caps the response (default 50, max 500).", + }, func(_ context.Context, _ *mcp.CallToolRequest, in bootBlameIn) (*mcp.CallToolResult, bootBlameOut, error) { top := in.Top if top <= 0 { top = 50 @@ -86,11 +99,30 @@ func registerBoot(s *mcp.Server, d Deps) { if top > 500 { return nil, bootBlameOut{}, fmt.Errorf("top must be <= 500, got %d", top) } - stdout, _, err := d.Exec.Run(ctx, "systemd-analyze", "blame") + m, err := sysd.Open() + if err != nil { + return nil, bootBlameOut{}, fmt.Errorf("open systemd bus: %w", err) + } + defer m.Close() + units, err := m.ListUnits() if err != nil { - return nil, bootBlameOut{}, fmt.Errorf("systemd-analyze blame: %w", err) + return nil, bootBlameOut{}, err } - entries := parseBlame(string(stdout)) + entries := make([]blameEntry, 0, len(units)) + for _, u := range units { + t, err := m.UnitTimings(u.Path, u.Name) + if err != nil { + continue + } + dur := t.StartupUsec() + if dur == 0 { + continue + } + entries = append(entries, blameEntry{Unit: u.Name, InitSeconds: float64(dur) / 1e6}) + } + sort.SliceStable(entries, func(i, j int) bool { + return entries[i].InitSeconds > entries[j].InitSeconds + }) if len(entries) > top { entries = entries[:top] } @@ -100,166 +132,112 @@ func registerBoot(s *mcp.Server, d Deps) { mcp.AddTool(s, &mcp.Tool{ Name: "boot_critical_chain", - Description: "Parsed `systemd-analyze critical-chain`: the serialized chain of units " + - "that gated the default target (typically multi-user.target). Each entry carries " + - "the cumulative active-at time and the unit's own startup time when systemd reports it.", - }, func(ctx context.Context, _ *mcp.CallToolRequest, _ bootCriticalChainIn) (*mcp.CallToolResult, bootCriticalChainOut, error) { - stdout, _, err := d.Exec.Run(ctx, "systemd-analyze", "critical-chain", "--no-pager") + Description: "The chain of units that gated boot, walking After= dependencies " + + "back from the default target, at each level picking the predecessor with " + + "the latest ActiveEnter timestamp (i.e. the bottleneck on that hop).", + }, func(_ context.Context, _ *mcp.CallToolRequest, _ bootCriticalChainIn) (*mcp.CallToolResult, bootCriticalChainOut, error) { + m, err := sysd.Open() + if err != nil { + return nil, bootCriticalChainOut{}, fmt.Errorf("open systemd bus: %w", err) + } + defer m.Close() + target, err := m.DefaultTarget() if err != nil { - return nil, bootCriticalChainOut{}, fmt.Errorf("systemd-analyze critical-chain: %w", err) + return nil, bootCriticalChainOut{}, err } - raw := string(stdout) - units := parseCriticalChain(raw) - return textResult("critical-chain: %d unit(s)", len(units)), - bootCriticalChainOut{Units: units, Raw: raw}, nil + chain, err := walkCriticalChain(m, target) + if err != nil { + return nil, bootCriticalChainOut{}, err + } + return textResult("critical-chain: %d unit(s) (default %s)", len(chain), target), + bootCriticalChainOut{Default: target, Units: chain}, nil }) } -// ---------- parsers (kept pure so they can be unit-tested) ------------------- - -// durRE matches one or more composite tokens (e.g. "5.219s", "1min 23.456s") -// without consuming trailing whitespace. -const durRE = `[0-9]+(?:\.[0-9]+)?(?:y|month|w|d|h|min|ms|s)(?:\s+[0-9]+(?:\.[0-9]+)?(?:y|month|w|d|h|min|ms|s))*` - -var ( - // "5.219s (firmware)" / "234ms (loader)" / "1min 23.456s (userspace)". - segmentRE = regexp.MustCompile(`(` + durRE + `)\s+\(([a-z]+)\)`) - // "= 19.935s" (total, after the chain of segments). - totalRE = regexp.MustCompile(`=\s+(` + durRE + `)`) - // "multi-user.target reached after 12.123s in userspace." - targetRE = regexp.MustCompile(`(\S+\.target)\s+reached after\s+(` + durRE + `)\s+in (?:userspace|initrd)`) - // Units inside critical-chain tree lines: " @ [+]". - chainUnitRE = regexp.MustCompile(`([A-Za-z0-9@:_.\-]+\.(?:target|service|socket|timer|mount|swap|path|slice|scope|device))(?:\s+@(` + durRE + `))?(?:\s+\+(` + durRE + `))?`) -) - -func parseBootTime(s string) bootTimeOut { - out := bootTimeOut{Raw: s} - for _, m := range segmentRE.FindAllStringSubmatch(s, -1) { - secs, ok := parseDurationSecs(m[1]) - if !ok { - continue - } - switch m[2] { - case "firmware": - out.FirmwareSec = secs - case "loader": - out.LoaderSec = secs - case "kernel": - out.KernelSec = secs - case "initrd": - out.InitrdSec = secs - case "userspace": - out.UserspaceSec = secs - } +// composeBootTime converts raw monotonic-microsecond timestamps into the +// per-phase seconds that systemd-analyze prints. Kept pure so it can be +// unit-tested without a live bus. +func composeBootTime(bt sysd.BootTimes) bootTimeOut { + out := bootTimeOut{} + // FirmwareTimestampMonotonic and LoaderTimestampMonotonic are stored as + // the number of microseconds the phase ended BEFORE kernel boot (uint64, + // 0 = phase didn't happen / not recorded). Firmware ran before the + // loader, so firmware time is the part not covered by the loader. + if bt.FirmwareMonotonicUsec > bt.LoaderMonotonicUsec { + out.FirmwareSec = float64(bt.FirmwareMonotonicUsec-bt.LoaderMonotonicUsec) / 1e6 } - if m := totalRE.FindStringSubmatch(s); m != nil { - if secs, ok := parseDurationSecs(m[1]); ok { - out.TotalSec = secs - } + if bt.LoaderMonotonicUsec > 0 { + out.LoaderSec = float64(bt.LoaderMonotonicUsec) / 1e6 } - if m := targetRE.FindStringSubmatch(s); m != nil { - out.TargetReached = m[1] - if secs, ok := parseDurationSecs(m[2]); ok { - out.TargetReachedSec = secs - } + // Kernel = time from kernel boot to either initrd start (when present) + // or userspace start (when no initrd was used). + if bt.InitRDMonotonicUsec > 0 { + out.KernelSec = float64(bt.InitRDMonotonicUsec) / 1e6 + if bt.UserspaceMonotonicUsec > bt.InitRDMonotonicUsec { + out.InitrdSec = float64(bt.UserspaceMonotonicUsec-bt.InitRDMonotonicUsec) / 1e6 + } + } else if bt.UserspaceMonotonicUsec > 0 { + out.KernelSec = float64(bt.UserspaceMonotonicUsec) / 1e6 } - return out -} - -func parseBlame(s string) []blameEntry { - var out []blameEntry - for _, line := range strings.Split(s, "\n") { - trimmed := strings.TrimSpace(line) - if trimmed == "" { - continue - } - fields := strings.Fields(trimmed) - if len(fields) < 2 { - continue - } - unit := fields[len(fields)-1] - durStr := strings.Join(fields[:len(fields)-1], " ") - secs, ok := parseDurationSecs(durStr) - if !ok { - continue - } - out = append(out, blameEntry{Unit: unit, InitSeconds: secs}) + if bt.FinishMonotonicUsec > bt.UserspaceMonotonicUsec { + out.UserspaceSec = float64(bt.FinishMonotonicUsec-bt.UserspaceMonotonicUsec) / 1e6 } - sort.SliceStable(out, func(i, j int) bool { - return out[i].InitSeconds > out[j].InitSeconds - }) + out.TotalSec = out.FirmwareSec + out.LoaderSec + out.KernelSec + out.InitrdSec + out.UserspaceSec return out } -func parseCriticalChain(s string) []chainEntry { +// walkCriticalChain starts at the default target and at each step picks the +// predecessor (by After=) that became active latest. Stops at a unit with no +// After= deps, when we revisit a unit, or after a hard depth limit. +func walkCriticalChain(m *sysd.Manager, start string) ([]chainEntry, error) { + const maxDepth = 32 + visited := map[string]struct{}{} var out []chainEntry - for _, line := range strings.Split(s, "\n") { - m := chainUnitRE.FindStringSubmatch(line) - if m == nil { - continue + cur := start + for i := 0; i < maxDepth; i++ { + if _, seen := visited[cur]; seen { + break } - e := chainEntry{Unit: m[1]} - if m[2] != "" { - if v, ok := parseDurationSecs(m[2]); ok { - e.ActiveAtSeconds = v - } + visited[cur] = struct{}{} + path, err := m.GetUnit(cur) + if err != nil { + return out, err } - if m[3] != "" { - if v, ok := parseDurationSecs(m[3]); ok { - e.StartupSeconds = v + t, err := m.UnitTimings(path, cur) + if err != nil { + return out, err + } + out = append(out, chainEntry{ + Unit: cur, + ActiveAtSeconds: float64(t.ActiveEnterMonotonicUsec) / 1e6, + StartupSeconds: float64(t.StartupUsec()) / 1e6, + }) + after, err := m.UnitAfter(path) + if err != nil || len(after) == 0 { + break + } + // Pick the predecessor with the latest ActiveEnter — that's the unit + // systemd was waiting on at this hop. + var nextName string + var nextWhen uint64 + for _, name := range after { + p, err := m.GetUnit(name) + if err != nil { + continue } - } - out = append(out, e) - } - return out -} - -// parseDurationSecs accepts the composite duration forms systemd-analyze emits: -// "234ms", "1.234s", "1min 23.456s", "2h 3min 4s". Returns total seconds. -// Unknown suffixes (e.g. "y", "month") are tolerated but contribute 0 — boot -// timings should never reach those scales, and if they do the user can read -// the Raw field directly. -func parseDurationSecs(s string) (float64, bool) { - s = strings.TrimSpace(s) - if s == "" { - return 0, false - } - var total float64 - seen := false - for _, f := range strings.Fields(s) { - mult, num, ok := splitDuration(f) - if !ok { - return 0, false - } - total += num * mult - seen = true - } - return total, seen -} - -func splitDuration(f string) (mult, num float64, ok bool) { - // Order matters: "ms" must be checked before "s", "min" before "n", etc. - suffixes := []struct { - s string - mult float64 - }{ - {"ms", 0.001}, - {"min", 60}, - {"month", 30 * 24 * 3600}, - {"s", 1}, - {"h", 3600}, - {"d", 24 * 3600}, - {"w", 7 * 24 * 3600}, - {"y", 365 * 24 * 3600}, - } - for _, suf := range suffixes { - if strings.HasSuffix(f, suf.s) { - v, err := strconv.ParseFloat(strings.TrimSuffix(f, suf.s), 64) + pt, err := m.UnitTimings(p, name) if err != nil { - return 0, 0, false + continue } - return suf.mult, v, true + if pt.ActiveEnterMonotonicUsec > nextWhen { + nextWhen = pt.ActiveEnterMonotonicUsec + nextName = name + } + } + if nextName == "" { + break } + cur = nextName } - return 0, 0, false + return out, nil } diff --git a/internal/tools/boot_test.go b/internal/tools/boot_test.go deleted file mode 100644 index 6478bea..0000000 --- a/internal/tools/boot_test.go +++ /dev/null @@ -1,121 +0,0 @@ -package tools - -import ( - "math" - "testing" -) - -func almost(a, b float64) bool { return math.Abs(a-b) < 1e-6 } - -func TestParseDurationSecs(t *testing.T) { - cases := []struct { - in string - want float64 - ok bool - }{ - {"234ms", 0.234, true}, - {"1.234s", 1.234, true}, - {"1min 23.456s", 83.456, true}, - {"2h 3min 4s", 2*3600 + 3*60 + 4, true}, - {"", 0, false}, - {"garbage", 0, false}, - {"5xy", 0, false}, - } - for _, c := range cases { - got, ok := parseDurationSecs(c.in) - if ok != c.ok || !almost(got, c.want) { - t.Errorf("parseDurationSecs(%q) = (%v, %v); want (%v, %v)", c.in, got, ok, c.want, c.ok) - } - } -} - -func TestParseBootTime(t *testing.T) { - t.Run("typical bare-metal", func(t *testing.T) { - input := "Startup finished in 5.219s (firmware) + 234ms (loader) + 2.137s (kernel) + 12.345s (userspace) = 19.935s\n" + - "multi-user.target reached after 12.123s in userspace.\n" - out := parseBootTime(input) - if !almost(out.FirmwareSec, 5.219) { - t.Errorf("firmware = %v", out.FirmwareSec) - } - if !almost(out.LoaderSec, 0.234) { - t.Errorf("loader = %v", out.LoaderSec) - } - if !almost(out.KernelSec, 2.137) { - t.Errorf("kernel = %v", out.KernelSec) - } - if !almost(out.UserspaceSec, 12.345) { - t.Errorf("userspace = %v", out.UserspaceSec) - } - if !almost(out.TotalSec, 19.935) { - t.Errorf("total = %v", out.TotalSec) - } - if out.TargetReached != "multi-user.target" { - t.Errorf("target = %q", out.TargetReached) - } - if !almost(out.TargetReachedSec, 12.123) { - t.Errorf("target_reached = %v", out.TargetReachedSec) - } - }) - t.Run("vm without firmware/loader", func(t *testing.T) { - input := "Startup finished in 1.876s (kernel) + 4.234s (userspace) = 6.110s\n" - out := parseBootTime(input) - if out.FirmwareSec != 0 || out.LoaderSec != 0 { - t.Errorf("expected zero firmware/loader, got %v/%v", out.FirmwareSec, out.LoaderSec) - } - if !almost(out.TotalSec, 6.110) { - t.Errorf("total = %v", out.TotalSec) - } - }) - t.Run("with initrd", func(t *testing.T) { - input := "Startup finished in 1s (kernel) + 2s (initrd) + 3s (userspace) = 6s\n" - out := parseBootTime(input) - if !almost(out.InitrdSec, 2) { - t.Errorf("initrd = %v", out.InitrdSec) - } - }) -} - -func TestParseBlame(t *testing.T) { - input := "12.345s NetworkManager-wait-online.service\n" + - " 4.567s systemd-networkd-wait-online.service\n" + - " 234ms snapd.service\n" + - "1min 23.456s some-slow.service\n" + - "\n" - got := parseBlame(input) - if len(got) != 4 { - t.Fatalf("got %d entries, want 4", len(got)) - } - // Must be sorted descending. - if got[0].Unit != "some-slow.service" || !almost(got[0].InitSeconds, 83.456) { - t.Errorf("entry[0] = %+v", got[0]) - } - if got[1].Unit != "NetworkManager-wait-online.service" || !almost(got[1].InitSeconds, 12.345) { - t.Errorf("entry[1] = %+v", got[1]) - } - if got[3].Unit != "snapd.service" || !almost(got[3].InitSeconds, 0.234) { - t.Errorf("entry[3] = %+v", got[3]) - } -} - -func TestParseCriticalChain(t *testing.T) { - input := "The time when unit became active or started is printed after the \"@\" character.\n" + - "The time the unit took to start is printed after the \"+\" character.\n" + - "\n" + - "multi-user.target @23.476s\n" + - "└─NetworkManager.service @17.234s +6.234s\n" + - " └─dbus.service @17.123s\n" + - " └─basic.target @17.012s\n" - got := parseCriticalChain(input) - if len(got) != 4 { - t.Fatalf("got %d units, want 4: %+v", len(got), got) - } - if got[0].Unit != "multi-user.target" || !almost(got[0].ActiveAtSeconds, 23.476) { - t.Errorf("got[0] = %+v", got[0]) - } - if got[1].Unit != "NetworkManager.service" || !almost(got[1].ActiveAtSeconds, 17.234) || !almost(got[1].StartupSeconds, 6.234) { - t.Errorf("got[1] = %+v", got[1]) - } - if got[3].StartupSeconds != 0 { - t.Errorf("got[3] should have no startup seconds: %+v", got[3]) - } -} diff --git a/internal/tools/systemd.go b/internal/tools/systemd.go index 807a8eb..57b05d2 100644 --- a/internal/tools/systemd.go +++ b/internal/tools/systemd.go @@ -2,19 +2,21 @@ package tools import ( "context" - "encoding/json" "fmt" + "sort" "strconv" "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/gjolly/fleetmind/internal/sysd" ) // -------- list_systemd_units ------------------------------------------------ type listUnitsIn struct { State string `json:"state,omitempty" jsonschema:"filter by active state: active|inactive|failed|activating|deactivating|reloading"` - Type string `json:"type,omitempty" jsonschema:"filter by unit type: service|timer|socket|mount|target|path|swap|slice|scope|device"` + Type string `json:"type,omitempty" jsonschema:"filter by unit type suffix: service|timer|socket|mount|target|path|swap|slice|scope|device|automount"` Limit int `json:"limit,omitempty" jsonschema:"max number of units to return (default 500, max 2000)"` } @@ -48,27 +50,13 @@ type unitStatusOut struct { type listTimersIn struct{} -// systemdTimer mirrors `systemctl list-timers --output=json` entries. The -// next/left/last/passed fields are emitted as numbers (microseconds since the -// Unix epoch; 0 means "never"), not strings — verified against systemd v255. type systemdTimer struct { - NextMicros int64 `json:"next_micros"` - LeftMicros int64 `json:"left_micros"` - LastMicros int64 `json:"last_micros"` - PassedMicros int64 `json:"passed_micros"` - Unit string `json:"unit"` - Activates string `json:"activates"` -} - -// rawSystemdTimer is the on-the-wire shape. Mapped into systemdTimer before -// returning so the public field names are explicit about the microsecond unit. -type rawSystemdTimer struct { - Next int64 `json:"next"` - Left int64 `json:"left"` - Last int64 `json:"last"` - Passed int64 `json:"passed"` - Unit string `json:"unit"` - Activates string `json:"activates"` + Unit string `json:"unit"` + Activates string `json:"activates"` + NextElapseMonotonicUsec uint64 `json:"next_elapse_monotonic_usec,omitempty"` + NextElapseRealtimeUsec uint64 `json:"next_elapse_realtime_usec,omitempty"` + LastTriggerUsec uint64 `json:"last_trigger_usec,omitempty"` + LastTriggerMonotonicUsec uint64 `json:"last_trigger_monotonic_usec,omitempty"` } type listTimersOut struct { @@ -78,24 +66,33 @@ type listTimersOut struct { // --------------------------------------------------------------------------- +// allowedUnitStates / allowedUnitTypes keep the filter inputs explicit; the +// D-Bus surface itself doesn't validate, so we do it here. +var ( + allowedUnitStates = map[string]bool{ + "active": true, "inactive": true, "failed": true, + "activating": true, "deactivating": true, "reloading": true, + } + allowedUnitTypes = map[string]bool{ + "service": true, "timer": true, "socket": true, "mount": true, + "target": true, "path": true, "swap": true, "slice": true, + "scope": true, "device": true, "automount": true, + } +) + func registerSystemd(s *mcp.Server, d Deps) { mcp.AddTool(s, &mcp.Tool{ Name: "list_systemd_units", - Description: "All loaded systemd units (`systemctl list-units --output=json`) with " + - "optional state/type filters. Default cap is 500 entries — narrow with `state` " + - "(e.g. \"failed\") or `type` (e.g. \"service\", \"timer\") for focused triage.", - }, func(ctx context.Context, _ *mcp.CallToolRequest, in listUnitsIn) (*mcp.CallToolResult, listUnitsOut, error) { - args, err := listUnitsArgs(in) - if err != nil { - return nil, listUnitsOut{}, err - } - stdout, _, err := d.Exec.Run(ctx, "systemctl", args...) - if err != nil { - return nil, listUnitsOut{}, fmt.Errorf("systemctl list-units: %w", err) + Description: "All loaded systemd units (via Manager.ListUnits over D-Bus) with " + + "optional state/type filters. Default cap is 500 entries — narrow with " + + "`state` (e.g. \"failed\") or `type` (e.g. \"service\", \"timer\") for focused " + + "triage.", + }, func(_ context.Context, _ *mcp.CallToolRequest, in listUnitsIn) (*mcp.CallToolResult, listUnitsOut, error) { + if in.State != "" && !allowedUnitStates[in.State] { + return nil, listUnitsOut{}, fmt.Errorf("invalid state %q", in.State) } - var parsed []systemdUnit - if err := json.Unmarshal(stdout, &parsed); err != nil { - return nil, listUnitsOut{}, fmt.Errorf("parse systemctl JSON: %w", err) + if in.Type != "" && !allowedUnitTypes[in.Type] { + return nil, listUnitsOut{}, fmt.Errorf("invalid type %q", in.Type) } limit := in.Limit if limit <= 0 { @@ -104,17 +101,45 @@ func registerSystemd(s *mcp.Server, d Deps) { if limit > 2000 { return nil, listUnitsOut{}, fmt.Errorf("limit must be <= 2000, got %d", limit) } - if len(parsed) > limit { - parsed = parsed[:limit] + m, err := sysd.Open() + if err != nil { + return nil, listUnitsOut{}, fmt.Errorf("open systemd bus: %w", err) + } + defer m.Close() + raw, err := m.ListUnits() + if err != nil { + return nil, listUnitsOut{}, err + } + out := listUnitsOut{Units: make([]systemdUnit, 0, len(raw))} + typeSuffix := "" + if in.Type != "" { + typeSuffix = "." + in.Type } - return textResult("%d unit(s) matched", len(parsed)), - listUnitsOut{Count: len(parsed), Units: parsed}, nil + for _, u := range raw { + if in.State != "" && u.ActiveState != in.State { + continue + } + if typeSuffix != "" && !strings.HasSuffix(u.Name, typeSuffix) { + continue + } + out.Units = append(out.Units, systemdUnit{ + Unit: u.Name, Load: u.LoadState, Active: u.ActiveState, + Sub: u.SubState, Description: u.Description, + }) + } + sort.SliceStable(out.Units, func(i, j int) bool { return out.Units[i].Unit < out.Units[j].Unit }) + if len(out.Units) > limit { + out.Units = out.Units[:limit] + } + out.Count = len(out.Units) + return textResult("%d unit(s) matched", out.Count), out, nil }) mcp.AddTool(s, &mcp.Tool{ Name: "unit_status", - Description: "Detail view of a single systemd unit: every property `systemctl show` " + - "reports, plus a tail of the unit's recent journal. Requires log-observe for journal.", + Description: "Detail view of a single systemd unit: every property the unit " + + "publishes on org.freedesktop.systemd1.Unit, plus a tail of the unit's recent " + + "journal (best-effort; requires log-observe).", }, func(ctx context.Context, _ *mcp.CallToolRequest, in unitStatusIn) (*mcp.CallToolResult, unitStatusOut, error) { if !unitRE.MatchString(in.Unit) { return nil, unitStatusOut{}, fmt.Errorf("invalid unit name %q", in.Unit) @@ -126,17 +151,28 @@ func registerSystemd(s *mcp.Server, d Deps) { if jLines > 500 { return nil, unitStatusOut{}, fmt.Errorf("journal_lines must be <= 500, got %d", jLines) } - showOut, _, err := d.Exec.Run(ctx, "systemctl", "show", "--no-pager", in.Unit) + m, err := sysd.Open() + if err != nil { + return nil, unitStatusOut{}, fmt.Errorf("open systemd bus: %w", err) + } + defer m.Close() + path, err := m.LoadUnit(in.Unit) + if err != nil { + return nil, unitStatusOut{}, err + } + props, err := m.UnitPropertiesAll(path, "org.freedesktop.systemd1.Unit") if err != nil { - return nil, unitStatusOut{}, fmt.Errorf("systemctl show %s: %w", in.Unit, err) + return nil, unitStatusOut{}, err } - props := parseSystemctlShow(string(showOut)) - // Journal is best-effort — log-observe may not be connected. - jOut, _, jErr := d.Exec.Run(ctx, "journalctl", "--no-pager", "-o", "short-iso", - "-n", strconv.Itoa(jLines), "--unit", in.Unit) + // Journal is still pulled via journalctl — log-observe makes journalctl + // work; no D-Bus involved on that side. journal := "" - if jErr == nil { - journal = string(jOut) + if d.Exec != nil { + jOut, _, jErr := d.Exec.Run(ctx, "journalctl", "--no-pager", "-o", "short-iso", + "-n", strconv.Itoa(jLines), "--unit", in.Unit) + if jErr == nil { + journal = string(jOut) + } } summary := props["ActiveState"] if sub := props["SubState"]; sub != "" { @@ -148,71 +184,37 @@ func registerSystemd(s *mcp.Server, d Deps) { mcp.AddTool(s, &mcp.Tool{ Name: "list_timers", - Description: "All systemd timers (`systemctl list-timers --output=json`): next/last " + - "fire times and the unit each one activates.", - }, func(ctx context.Context, _ *mcp.CallToolRequest, _ listTimersIn) (*mcp.CallToolResult, listTimersOut, error) { - stdout, _, err := d.Exec.Run(ctx, "systemctl", "list-timers", "--no-pager", "--all", "--output=json") + Description: "All loaded systemd timer units with their scheduling timestamps " + + "(next/last elapse, monotonic & realtime). Reads org.freedesktop.systemd1.Timer " + + "properties over D-Bus.", + }, func(_ context.Context, _ *mcp.CallToolRequest, _ listTimersIn) (*mcp.CallToolResult, listTimersOut, error) { + m, err := sysd.Open() if err != nil { - return nil, listTimersOut{}, fmt.Errorf("systemctl list-timers: %w", err) - } - var raw []rawSystemdTimer - if err := json.Unmarshal(stdout, &raw); err != nil { - return nil, listTimersOut{}, fmt.Errorf("parse systemctl JSON: %w", err) - } - parsed := make([]systemdTimer, len(raw)) - for i, r := range raw { - parsed[i] = systemdTimer{ - NextMicros: r.Next, LeftMicros: r.Left, - LastMicros: r.Last, PassedMicros: r.Passed, - Unit: r.Unit, Activates: r.Activates, - } + return nil, listTimersOut{}, fmt.Errorf("open systemd bus: %w", err) } - return textResult("%d timer(s)", len(parsed)), - listTimersOut{Count: len(parsed), Timers: parsed}, nil - }) -} - -// listUnitsArgs builds the systemctl argv. Kept pure for unit-testing. -func listUnitsArgs(in listUnitsIn) ([]string, error) { - args := []string{"list-units", "--no-pager", "--all", "--output=json"} - if in.State != "" { - allowed := map[string]bool{ - "active": true, "inactive": true, "failed": true, - "activating": true, "deactivating": true, "reloading": true, - } - if !allowed[in.State] { - return nil, fmt.Errorf("invalid state %q", in.State) - } - args = append(args, "--state="+in.State) - } - if in.Type != "" { - allowed := map[string]bool{ - "service": true, "timer": true, "socket": true, "mount": true, - "target": true, "path": true, "swap": true, "slice": true, - "scope": true, "device": true, "automount": true, - } - if !allowed[in.Type] { - return nil, fmt.Errorf("invalid type %q", in.Type) - } - args = append(args, "--type="+in.Type) - } - return args, nil -} - -// parseSystemctlShow consumes the Key=Value\n stream `systemctl show` emits -// and returns a flat string map. Values that contain '=' are preserved -// verbatim after the first delimiter. -func parseSystemctlShow(s string) map[string]string { - out := map[string]string{} - for _, line := range strings.Split(s, "\n") { - if line == "" { - continue - } - k, v, ok := strings.Cut(line, "=") - if !ok { - continue + defer m.Close() + raw, err := m.ListUnits() + if err != nil { + return nil, listTimersOut{}, err } - out[k] = v - } - return out + out := listTimersOut{Timers: make([]systemdTimer, 0)} + for _, u := range raw { + if !strings.HasSuffix(u.Name, ".timer") { + continue + } + info, _ := m.TimerProperties(u.Path) + activates, _ := m.TriggersUnit(u.Path) + out.Timers = append(out.Timers, systemdTimer{ + Unit: u.Name, + Activates: activates, + NextElapseMonotonicUsec: info.NextElapseMonotonicUsec, + NextElapseRealtimeUsec: info.NextElapseRealtimeUsec, + LastTriggerUsec: info.LastTriggerUsec, + LastTriggerMonotonicUsec: info.LastTriggerMonotonicUsec, + }) + } + sort.SliceStable(out.Timers, func(i, j int) bool { return out.Timers[i].Unit < out.Timers[j].Unit }) + out.Count = len(out.Timers) + return textResult("%d timer(s)", out.Count), out, nil + }) } diff --git a/internal/tools/systemd_test.go b/internal/tools/systemd_test.go deleted file mode 100644 index 9530189..0000000 --- a/internal/tools/systemd_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package tools - -import ( - "strings" - "testing" -) - -func TestListUnitsArgs(t *testing.T) { - t.Run("defaults", func(t *testing.T) { - args, err := listUnitsArgs(listUnitsIn{}) - if err != nil { - t.Fatalf("err = %v", err) - } - joined := strings.Join(args, " ") - if !strings.Contains(joined, "list-units") || !strings.Contains(joined, "--output=json") { - t.Errorf("unexpected argv: %q", joined) - } - if strings.Contains(joined, "--state") || strings.Contains(joined, "--type") { - t.Errorf("unexpected filter in defaults: %q", joined) - } - }) - t.Run("with filters", func(t *testing.T) { - args, err := listUnitsArgs(listUnitsIn{State: "failed", Type: "service"}) - if err != nil { - t.Fatalf("err = %v", err) - } - joined := strings.Join(args, " ") - if !strings.Contains(joined, "--state=failed") || !strings.Contains(joined, "--type=service") { - t.Errorf("missing filters: %q", joined) - } - }) - t.Run("rejects bad state", func(t *testing.T) { - if _, err := listUnitsArgs(listUnitsIn{State: "panic"}); err == nil { - t.Fatal("expected error for unknown state") - } - }) - t.Run("rejects bad type", func(t *testing.T) { - if _, err := listUnitsArgs(listUnitsIn{Type: "container"}); err == nil { - t.Fatal("expected error for unknown type") - } - }) -} - -func TestParseSystemctlShow(t *testing.T) { - input := "Id=NetworkManager.service\n" + - "LoadState=loaded\n" + - "ActiveState=active\n" + - "SubState=running\n" + - "FragmentPath=/lib/systemd/system/NetworkManager.service\n" + - "Environment=LANG=C.UTF-8\n" + // value containing '=' - "\n" - out := parseSystemctlShow(input) - if out["Id"] != "NetworkManager.service" { - t.Errorf("Id = %q", out["Id"]) - } - if out["ActiveState"] != "active" || out["SubState"] != "running" { - t.Errorf("active/sub = %q/%q", out["ActiveState"], out["SubState"]) - } - if out["Environment"] != "LANG=C.UTF-8" { - t.Errorf("Environment should preserve embedded '=': got %q", out["Environment"]) - } -} diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index a31560a..ab06c4b 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -41,6 +41,14 @@ apps: - network-observe - log-observe - kernel-module-observe + slots: + - systemd-bus + +slots: + systemd-bus: + interface: dbus + bus: system + name: org.freedesktop.systemd1 parts: fleetmind: