diff --git a/cmd/gortex/daemon_service.go b/cmd/gortex/daemon_service.go index b67130fe..2576e1fa 100644 --- a/cmd/gortex/daemon_service.go +++ b/cmd/gortex/daemon_service.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "encoding/xml" "errors" "fmt" "io" @@ -63,6 +64,64 @@ func init() { daemonCmd.AddCommand(daemonServiceStatusCmd) } +// serviceEnvVar is a single environment entry rendered into a service +// unit. Render helpers escape Value for the target format. +type serviceEnvVar struct{ Key, Value string } + +// xdgServiceEnv captures the XDG base-directory overrides in effect when +// the service unit is written, so the supervised daemon resolves the +// same config / data / cache locations as the shell that installed it. +// +// launchd and systemd --user start the daemon with a near-empty +// environment — they do NOT inherit the XDG_* variables a user exports +// from their shell or session manager. Without this capture the daemon +// falls back to ~/.gortex even though the user opted into an XDG layout +// (see internal/platform/xdg.go), silently splitting their state across +// two trees. Re-run install-service to re-capture changed values. +// +// Only absolute values are propagated — the rule platform.unifiedDir +// itself applies when honouring an override (a relative XDG path is +// ignored per the XDG Base Directory spec). XDG_RUNTIME_DIR is +// deliberately excluded: the init system sets it per session, so pinning +// an install-time value would point the daemon socket at a stale dir. +func xdgServiceEnv() []serviceEnvVar { + var out []serviceEnvVar + for _, name := range []string{"XDG_CONFIG_HOME", "XDG_DATA_HOME", "XDG_CACHE_HOME"} { + if v := os.Getenv(name); v != "" && filepath.IsAbs(v) { + out = append(out, serviceEnvVar{Key: name, Value: v}) + } + } + return out +} + +// xmlEscape renders s safe for an XML text node (the launchd plist) so a +// home path containing an XML metacharacter can't produce a malformed, +// unloadable plist. +func xmlEscape(s string) string { + var b strings.Builder + if err := xml.EscapeText(&b, []byte(s)); err != nil { + return s + } + return b.String() +} + +// systemdEnvValue renders a value safe for a systemd Environment= line. +// `%` is escaped to `%%` because systemd treats it as a specifier +// introducer across the whole unit file (systemd.unit(5)) — an +// unescaped `%d` in a path would expand to a directory specifier and +// silently change the value the daemon sees. Values containing +// whitespace are additionally double-quoted (with embedded quotes / +// backslashes escaped) per systemd's quoting rules. Plain paths (the +// common case) pass through unchanged. +func systemdEnvValue(v string) string { + v = strings.ReplaceAll(v, "%", "%%") + if !strings.ContainsAny(v, " \t") { + return v + } + r := strings.NewReplacer(`\`, `\\`, `"`, `\"`) + return `"` + r.Replace(v) + `"` +} + // runDaemonInstallService writes the unit file and enables + starts it. // Existing daemon processes are stopped first so the service owns the // only running instance after this returns. @@ -134,6 +193,11 @@ func runDaemonServiceStatus(cmd *cobra.Command, _ []string) error { // // StandardOutPath / StandardErrorPath redirect logs into the same file // `gortex daemon logs` tails, so users don't need to remember two paths. +// +// EnvironmentVariables carries PATH (so a Homebrew-installed binary is +// found in launchd's minimal environment) plus any XDG_* overrides that +// were in effect at install time — see xdgServiceEnv for why that +// capture is necessary. const launchdPlistTemplate = ` @@ -161,11 +225,38 @@ const launchdPlistTemplate = ` PATH /usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin +{{- range .EnvVars}} + {{.Key}} + {{.Value}} +{{- end}} ` +// renderLaunchdPlist fills launchdPlistTemplate. String values are +// XML-escaped so a path containing an XML metacharacter can't produce a +// malformed, unloadable plist. +func renderLaunchdPlist(label, exe, logPath string, env []serviceEnvVar) (string, error) { + data := struct { + Label, Exe, LogPath string + EnvVars []serviceEnvVar + }{ + Label: xmlEscape(label), + Exe: xmlEscape(exe), + LogPath: xmlEscape(logPath), + EnvVars: make([]serviceEnvVar, len(env)), + } + for i, e := range env { + data.EnvVars[i] = serviceEnvVar{Key: e.Key, Value: xmlEscape(e.Value)} + } + var buf bytes.Buffer + if err := template.Must(template.New("plist").Parse(launchdPlistTemplate)).Execute(&buf, data); err != nil { + return "", err + } + return buf.String(), nil +} + func launchdPlistPath() (string, error) { home, err := os.UserHomeDir() if err != nil { @@ -183,15 +274,11 @@ func installLaunchd(w io.Writer, exe string) error { return fmt.Errorf("ensure LaunchAgents dir: %w", err) } - var buf bytes.Buffer - if err := template.Must(template.New("plist").Parse(launchdPlistTemplate)).Execute(&buf, map[string]string{ - "Label": daemonServiceName, - "Exe": exe, - "LogPath": daemon.LogFilePath(), - }); err != nil { + plist, err := renderLaunchdPlist(daemonServiceName, exe, daemon.LogFilePath(), xdgServiceEnv()) + if err != nil { return fmt.Errorf("render plist: %w", err) } - if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + if err := os.WriteFile(path, []byte(plist), 0o644); err != nil { return fmt.Errorf("write plist: %w", err) } // -w persists the load across reboots; without it the service @@ -254,7 +341,9 @@ func statusLaunchd(w io.Writer) error { // systemdUnitTemplate renders a user-level systemd service. Type=simple // because `gortex daemon start` (without --detach) runs in the // foreground; Restart=on-failure covers the crash-restart case without -// pounding on successful exits. +// pounding on successful exits. Environment= lines carry any XDG_* +// overrides that were in effect at install time so the supervised daemon +// resolves the same paths as the installing shell — see xdgServiceEnv. const systemdUnitTemplate = `[Unit] Description=Gortex code intelligence daemon Documentation=https://github.com/zzet/gortex @@ -263,6 +352,9 @@ After=network.target [Service] Type=simple ExecStart={{.Exe}} daemon start +{{- range .EnvVars}} +Environment={{.Key}}={{.Value}} +{{- end}} Restart=on-failure RestartSec=2 StandardOutput=append:{{.LogPath}} @@ -272,6 +364,23 @@ StandardError=append:{{.LogPath}} WantedBy=default.target ` +// renderSystemdUnit fills systemdUnitTemplate, quoting Environment= +// values that need it. +func renderSystemdUnit(exe, logPath string, env []serviceEnvVar) (string, error) { + data := struct { + Exe, LogPath string + EnvVars []serviceEnvVar + }{Exe: exe, LogPath: logPath, EnvVars: make([]serviceEnvVar, len(env))} + for i, e := range env { + data.EnvVars[i] = serviceEnvVar{Key: e.Key, Value: systemdEnvValue(e.Value)} + } + var buf bytes.Buffer + if err := template.Must(template.New("unit").Parse(systemdUnitTemplate)).Execute(&buf, data); err != nil { + return "", err + } + return buf.String(), nil +} + func systemdUnitPath() (string, error) { home, err := os.UserHomeDir() if err != nil { @@ -289,14 +398,11 @@ func installSystemd(w io.Writer, exe string) error { return fmt.Errorf("ensure systemd user dir: %w", err) } - var buf bytes.Buffer - if err := template.Must(template.New("unit").Parse(systemdUnitTemplate)).Execute(&buf, map[string]string{ - "Exe": exe, - "LogPath": daemon.LogFilePath(), - }); err != nil { + unit, err := renderSystemdUnit(exe, daemon.LogFilePath(), xdgServiceEnv()) + if err != nil { return fmt.Errorf("render unit: %w", err) } - if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + if err := os.WriteFile(path, []byte(unit), 0o644); err != nil { return fmt.Errorf("write unit: %w", err) } if err := runCmd(w, "systemctl", "--user", "daemon-reload"); err != nil { diff --git a/cmd/gortex/daemon_service_test.go b/cmd/gortex/daemon_service_test.go index d67e443f..034624f3 100644 --- a/cmd/gortex/daemon_service_test.go +++ b/cmd/gortex/daemon_service_test.go @@ -1,63 +1,177 @@ package main import ( - "bytes" + "encoding/xml" + "io" "path/filepath" "runtime" "strings" "testing" - "text/template" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// TestLaunchdPlistTemplate_Renders proves the launchd plist template -// is syntactically valid and carries the substitutions it must — -// binary path, log path, and label. A regression here would produce -// an unloadable plist with a cryptic launchctl error, so the check is -// just "did we put the right strings in the right places." -func TestLaunchdPlistTemplate_Renders(t *testing.T) { - var buf bytes.Buffer - tmpl := template.Must(template.New("plist").Parse(launchdPlistTemplate)) - require.NoError(t, tmpl.Execute(&buf, map[string]string{ - "Label": "com.zzet.gortex", - "Exe": "/usr/local/bin/gortex", - "LogPath": "/Users/testuser/.cache/gortex/daemon.log", - })) - - out := buf.String() +// assertWellFormedXML fails if s is not parseable XML — a stronger +// guarantee than substring checks that the env injection didn't corrupt +// the plist into something launchctl can't load. +func assertWellFormedXML(t *testing.T, s string) { + t.Helper() + dec := xml.NewDecoder(strings.NewReader(s)) + for { + _, err := dec.Token() + if err == io.EOF { + break + } + require.NoError(t, err, "plist must be well-formed XML") + } +} + +// TestRenderLaunchdPlist_NoXDG proves the launchd plist is valid and +// carries the substitutions it must — binary path, log path, label — +// and that with no XDG override captured it has no extra env entries +// (the pre-fix baseline behaviour, unchanged for the common case). +func TestRenderLaunchdPlist_NoXDG(t *testing.T) { + out, err := renderLaunchdPlist( + "com.zzet.gortex", + "/usr/local/bin/gortex", + "/Users/testuser/.gortex/cache/daemon.log", + nil, + ) + require.NoError(t, err) + assert.Contains(t, out, "Label\n com.zzet.gortex") assert.Contains(t, out, "/usr/local/bin/gortex") assert.Contains(t, out, "daemon") assert.Contains(t, out, "start") assert.Contains(t, out, "RunAtLoad") assert.Contains(t, out, "KeepAlive") - assert.Contains(t, out, "/Users/testuser/.cache/gortex/daemon.log") + assert.Contains(t, out, "/Users/testuser/.gortex/cache/daemon.log") // Homebrew paths must be on PATH so launchd can find a Homebrew- // installed gortex binary even when the LaunchAgent env is minimal. assert.Contains(t, out, "/opt/homebrew/bin") + assert.NotContains(t, out, "XDG_", "no env entries when none captured") + assertWellFormedXML(t, out) +} + +// TestRenderLaunchdPlist_PropagatesXDG is the regression test for the +// service-ignores-XDG bug: when an XDG override is in effect at install +// time it must be baked into the plist so the supervised daemon resolves +// the same paths as the installing shell instead of falling back to +// ~/.gortex. +func TestRenderLaunchdPlist_PropagatesXDG(t *testing.T) { + env := []serviceEnvVar{ + {Key: "XDG_CONFIG_HOME", Value: "/Users/testuser/.config"}, + {Key: "XDG_DATA_HOME", Value: "/Users/testuser/.local/share"}, + } + out, err := renderLaunchdPlist( + "com.zzet.gortex", + "/usr/local/bin/gortex", + "/Users/testuser/.config/gortex/daemon.log", + env, + ) + require.NoError(t, err) + + assert.Contains(t, out, "XDG_CONFIG_HOME\n /Users/testuser/.config") + assert.Contains(t, out, "XDG_DATA_HOME\n /Users/testuser/.local/share") + // PATH must still be present alongside the captured XDG vars. + assert.Contains(t, out, "PATH") + assertWellFormedXML(t, out) +} + +// TestRenderLaunchdPlist_EscapesXML guards against a home path with an +// XML metacharacter producing a malformed, unloadable plist. +func TestRenderLaunchdPlist_EscapesXML(t *testing.T) { + env := []serviceEnvVar{{Key: "XDG_CONFIG_HOME", Value: "/Users/a&b/.config"}} + out, err := renderLaunchdPlist("com.zzet.gortex", "/opt/a&b/gortex", "/log&path", env) + require.NoError(t, err) + + assert.Contains(t, out, "/Users/a&b/.config") + assert.Contains(t, out, "/opt/a&b/gortex") + assert.NotContains(t, out, "a&b", "raw ampersand must be escaped") + assertWellFormedXML(t, out) } -// TestSystemdUnitTemplate_Renders is the analog for Linux. Confirms -// the Type=simple + Restart=on-failure contract the install doc -// promises: simple means the daemon runs in the foreground (no -// --detach), on-failure means crashes restart but clean exits don't -// fight the user if they ran `daemon stop`. -func TestSystemdUnitTemplate_Renders(t *testing.T) { - var buf bytes.Buffer - tmpl := template.Must(template.New("unit").Parse(systemdUnitTemplate)) - require.NoError(t, tmpl.Execute(&buf, map[string]string{ - "Exe": "/home/u/.local/bin/gortex", - "LogPath": "/home/u/.cache/gortex/daemon.log", - })) - - out := buf.String() +// TestRenderSystemdUnit_NoXDG confirms the Type=simple + Restart=on-failure +// contract and that no Environment= line is emitted when nothing was +// captured. +func TestRenderSystemdUnit_NoXDG(t *testing.T) { + out, err := renderSystemdUnit( + "/home/u/.local/bin/gortex", + "/home/u/.gortex/cache/daemon.log", + nil, + ) + require.NoError(t, err) + assert.Contains(t, out, "ExecStart=/home/u/.local/bin/gortex daemon start") assert.Contains(t, out, "Type=simple") assert.Contains(t, out, "Restart=on-failure") - assert.Contains(t, out, "StandardOutput=append:/home/u/.cache/gortex/daemon.log") + assert.Contains(t, out, "StandardOutput=append:/home/u/.gortex/cache/daemon.log") assert.Contains(t, out, "WantedBy=default.target") + assert.NotContains(t, out, "Environment=", "no env line when none captured") +} + +// TestRenderSystemdUnit_PropagatesXDG is the Linux analog of the launchd +// regression test, and also covers systemd quoting for a value with a +// space. +func TestRenderSystemdUnit_PropagatesXDG(t *testing.T) { + env := []serviceEnvVar{ + {Key: "XDG_CACHE_HOME", Value: "/home/u/.cache"}, + {Key: "XDG_DATA_HOME", Value: "/home/u/has space"}, + } + out, err := renderSystemdUnit( + "/home/u/.local/bin/gortex", + "/home/u/.cache/gortex/daemon.log", + env, + ) + require.NoError(t, err) + + assert.Contains(t, out, "Environment=XDG_CACHE_HOME=/home/u/.cache") + // A value containing whitespace is double-quoted per systemd rules. + assert.Contains(t, out, `Environment=XDG_DATA_HOME="/home/u/has space"`) + + // Environment lines must sit inside [Service], ahead of [Install]. + svcStart := strings.Index(out, "[Service]") + installStart := strings.Index(out, "[Install]") + require.Greater(t, svcStart, -1) + require.Greater(t, installStart, svcStart) + svc := out[svcStart:installStart] + assert.Contains(t, svc, "Environment=XDG_CACHE_HOME=") +} + +// TestXDGServiceEnv_OnlyAbsoluteSet pins the capture contract: only XDG +// vars that are set to an absolute path are propagated. Unset and +// relative values are ignored (the latter per the XDG spec, matching +// platform.unifiedDir). +func TestXDGServiceEnv_OnlyAbsoluteSet(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", "/abs/config") + t.Setenv("XDG_DATA_HOME", "relative/data") // ignored — not absolute + t.Setenv("XDG_CACHE_HOME", "") // ignored — empty/unset + + got := map[string]string{} + for _, e := range xdgServiceEnv() { + got[e.Key] = e.Value + } + + assert.Equal(t, "/abs/config", got["XDG_CONFIG_HOME"]) + _, hasData := got["XDG_DATA_HOME"] + assert.False(t, hasData, "relative XDG_DATA_HOME must be ignored") + _, hasCache := got["XDG_CACHE_HOME"] + assert.False(t, hasCache, "empty XDG_CACHE_HOME must be ignored") +} + +func TestSystemdEnvValue_QuotesWhitespace(t *testing.T) { + assert.Equal(t, "/home/u/.config", systemdEnvValue("/home/u/.config")) + assert.Equal(t, `"/home/u/my data"`, systemdEnvValue("/home/u/my data")) +} + +// TestSystemdEnvValue_EscapesPercent guards the systemd specifier escape: +// a literal % in a path must become %% or systemd expands it (e.g. %d) +// and the daemon resolves a different directory than was captured. +func TestSystemdEnvValue_EscapesPercent(t *testing.T) { + assert.Equal(t, "/home/u/100%%dir", systemdEnvValue("/home/u/100%dir")) + // percent + whitespace: escaped and quoted. + assert.Equal(t, `"/home/u/a %%b c"`, systemdEnvValue("/home/u/a %b c")) } func TestLaunchdPlistPath_ResolvesUnderHome(t *testing.T) { @@ -82,8 +196,7 @@ func TestSystemdUnitPath_ResolvesUnderHome(t *testing.T) { // TestServiceCommands_RejectUnsupportedOS keeps the guard // runDaemonInstallService uses from silently succeeding on Windows or -// other platforms we haven't wired — Phase 4 explicitly excludes -// them for now. +// other platforms we haven't wired. func TestServiceCommands_RejectUnsupportedOS(t *testing.T) { if runtime.GOOS == "darwin" || runtime.GOOS == "linux" { t.Skip("this test only runs on unsupported platforms") diff --git a/docs/onboarding.md b/docs/onboarding.md index 0dd7d959..862278f2 100644 --- a/docs/onboarding.md +++ b/docs/onboarding.md @@ -235,6 +235,8 @@ gortex daemon uninstall-service # remove unit, stop service On macOS the unit lands at `~/Library/LaunchAgents/com.zzet.gortex.plist`; on Linux at `~/.config/systemd/user/com.zzet.gortex.service`. After `install-service`, plain `gortex daemon start / stop` still work — they just fight the service for socket ownership, so prefer `gortex daemon service-status` and `launchctl` / `systemctl --user` commands for lifecycle. +If you run an XDG layout (any absolute `XDG_CONFIG_HOME` / `XDG_DATA_HOME` / `XDG_CACHE_HOME`), `install-service` captures those values into the unit so the supervised daemon resolves the same paths as your shell — service supervisors otherwise start with a near-empty environment and the daemon would fall back to `~/.gortex`. Re-run `install-service` if you later change where those variables point. + ### How it works - `gortex mcp` (what Claude Code spawns via `.mcp.json`) auto-detects the daemon. If reachable, it acts as a thin stdio ↔ socket proxy (~5 MB per client). If not, it falls back to the embedded server — global mode is never "required."