Skip to content

[AI-818] feat(daemon): per-user service supervisor (install/uninstall/start/stop) - #137

Merged
alexeyzimarev merged 16 commits into
mainfrom
feat/daemon-service-supervisor
Jun 10, 2026
Merged

alexeyzimarev merged 16 commits into
mainfrom
feat/daemon-service-supervisor

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Summary

Adds kcap daemon service install|uninstall|start|stop|status to run the daemon under a per-user OS service so it auto-restarts after an external SIGKILL (macOS jetsam / Linux OOM / crash) — the root cause traced on 2026-06-10 (exit 137, zero death-rattle output = uncatchable kill, reproduced under detached tmux during a memory-pressure jetsam storm; tmux doesn't shield against kernel memory reclamation).

  • launchd LaunchAgent (macOS), systemd --user one-file-per-id (Linux), logon Scheduled Task + .cmd wrapper (Windows), behind an IServiceManager seam selected by ServiceManagerFactory.
  • Each platform splits a pure generator/vector/parser class (LaunchdUnit/SystemdUnit/WindowsTaskUnit — fully unit-tested, including XDocument-validated escaping) from a thin shell-out manager.
  • Restart fires on crash/SIGKILL but not on a clean stop; stop keeps the unit installed, only uninstall deregisters.
  • install pins the active profile via KCAP_PROFILE and captures PATH/KCAP_* into the unit (a baked --server-url would null out profile resolution and drop claude/codex paths; supervised jobs don't inherit the interactive PATH).
  • Daemon names are sanitized to a portable service id; all interpolated values are XML/systemd/cmd-escaped.
  • kcap daemon status/stop/doctor are now service-aware (list installed-but-stopped services, refuse to raw-kill a supervised daemon, flag a moved binary path).
  • Docs: help-daemon.txt + README.md (getting-started + Daemon section).

Design + plan: docs/superpowers/specs/2026-06-10-daemon-service-supervisor-design.md, docs/superpowers/plans/2026-06-10-daemon-service-supervisor.md.

Test Plan

  • dotnet run --project test/Capacitor.Cli.Tests.Unit — 1317 pass, 0 fail (25 new tests: escaping, three generators, factory, env capture)
  • dotnet publish -c Release — no IL2026/IL3050 warnings; NativeAOT binary runs kcap daemon service status
  • macOS: kcap daemon service install → reboot/relogin → daemon comes back; service stop/start/uninstall
  • Linux: same via systemctl --user
  • Windows: verify the schtasks /End-vs-restart-on-failure behavior on a real host (spec documents the disable→end→re-enable fallback if /End relaunches)

The launchctl/systemctl/schtasks apply paths are not exercised in CI (can't register real units); their pure inputs — generators, command vectors, parsers — are unit-tested.

🤖 Generated with Claude Code

alexeyzimarev and others added 13 commits June 10, 2026 16:43
Per-user OS service integration so the daemon auto-restarts after an
external SIGKILL (jetsam/OOM/crash) — the root cause traced on 2026-06-10:
exit 137 with zero death-rattle output = uncatchable kill, reproduced under
detached tmux during a memory-pressure jetsam storm.

launchd LaunchAgent (macOS), systemd --user unit (Linux), Task Scheduler
logon task (Windows), behind an IServiceManager seam with pure unit-text
generators for testing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Revise the daemon-service-supervisor spec for six review findings:

- systemd: per-instance kcap-daemon-<id>.service files instead of one shared
  @.service template (per-instance args + safe single-unit uninstall)
- pin profile via KCAP_PROFILE + capture PATH/KCAP_* into the unit instead of
  baking --server-url (which nulls profile resolution and drops claude/codex
  paths; supervised jobs don't inherit interactive PATH)
- sanitize the daemon name into a portable service id and XML/systemd-escape
  every interpolated value in generated unit text
- add ListInstalled()/ServiceStatus.BinaryPath so installed-but-stopped
  services show in status and doctor can validate the baked binary path
- define idempotent install (replace existing unit, end running)
- add ServiceManagerFactory.ForPlatform so all generators are testable on one OS

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task Scheduler's Exec action has no Environment element (only Command/
Arguments/WorkingDirectory), so the captured PATH/KCAP_* can't be set in the
task XML. Generate a daemon-service-<id>.cmd wrapper that `set`s the env and
execs kcap-daemon.exe; the task runs `cmd /c <wrapper>`. install rewrites it,
uninstall deletes it. Add cmd batch escaping to the escaping rules.

Generalize the pure generator seam from GenerateUnit (single string) to
GenerateFiles -> [(path, content)] so Windows's two artifacts fit.

stop fallback: if /End triggers a relaunch, disable -> /End -> re-enable so the
task stays enabled for next logon instead of being left disabled.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
10 TDD tasks: contracts+escaping, three pure unit generators
(launchd/systemd/windows), thin managers + process runner + factory,
env capture + profile pin, `kcap daemon service` dispatch,
status/stop/doctor awareness, docs, AOT verify.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qodo-code-review

qodo-code-review Bot commented Jun 10, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)

Grey Divider


Action required

1. PATH capture is case-sensitive 🐞 Bug ☼ Reliability
Description
ServiceEnvironment.Snapshot uses a case-sensitive Dictionary, but Build searches for fixed uppercase
keys like "PATH", so PATH/KCAP_* values can be silently omitted when the source environment uses
different casing.
Code

src/Capacitor.Cli/Services/ServiceEnvironment.cs[R18-31]

+    static Dictionary<string, string> Snapshot() {
+        var d = new Dictionary<string, string>();
+        foreach (DictionaryEntry e in Environment.GetEnvironmentVariables())
+            if (e.Key is string k && e.Value is string v) d[k] = v;
+        return d;
+    }
+
+    /// <summary>Pure: select the relevant keys from <paramref name="source"/>, pin the profile.</summary>
+    public static IReadOnlyDictionary<string, string> Build(string? profileName, IReadOnlyDictionary<string, string> source) {
+        var env = new Dictionary<string, string>(StringComparer.Ordinal);
+        foreach (var key in Keys)
+            if (source.TryGetValue(key, out var v) && !string.IsNullOrEmpty(v)) env[key] = v;
+        if (!string.IsNullOrEmpty(profileName)) env["KCAP_PROFILE"] = profileName; // explicit pin wins
+        return env;
Evidence
Keys are defined as uppercase literals and Build uses TryGetValue for those keys, but Snapshot
stores whatever casing comes from Environment.GetEnvironmentVariables into a default case-sensitive
dictionary, so mismatched casing will fail lookup.

src/Capacitor.Cli/Services/ServiceEnvironment.cs[12-31]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ServiceEnvironment` captures env vars into a case-sensitive dictionary, then `Build()` looks up fixed key names (e.g., `"PATH"`). If the process environment provides keys with different casing, the intended variables (especially PATH) won’t be captured into the service unit.

### Issue Context
This directly affects the core feature: the supervised daemon needs the captured PATH/KCAP_* env to resolve `claude`/`codex` consistently.

### Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceEnvironment.cs[12-31]
- test/Capacitor.Cli.Tests.Unit/Services/ServiceEnvironmentTests.cs[7-26]

### Suggested fix
- Create the snapshot dictionary with `StringComparer.OrdinalIgnoreCase`.
- Consider also returning the built env with `StringComparer.OrdinalIgnoreCase` (or normalize output keys to a canonical casing).
- Add a unit test where the source contains `"Path"` (or mixed-case `Kcap_Profile`) and verify `Build()` still includes the expected entries.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Systemd unit not safely escaped ✓ Resolved 🐞 Bug ☼ Reliability
Description
SystemdUnit.Unit writes ExecStart and Environment lines via simple concatenation without
quoting/escaping paths or values, so spaces/metacharacters in $HOME-derived paths or captured KCAP_*
values can break parsing or change argv/env.
Code

src/Capacitor.Cli/Services/SystemdUnit.cs[R24-33]

+        sb.Append("[Service]\n");
+        foreach (var (k, v) in spec.Environment)
+            sb.Append($"Environment={ServiceText.SystemdValue(k)}={ServiceText.SystemdValue(v)}\n");
+
+        var args = string.Join(' ', new[] { "--name", spec.ServiceId, "--log-file", spec.LogPath }.Concat(spec.ExtraArgs));
+        sb.Append($"ExecStart={spec.DaemonBinaryPath} {args}\n");
+        sb.Append("Restart=on-failure\n");
+        sb.Append("RestartSec=5\n");
+        sb.Append("StartLimitIntervalSec=60\n");
+        sb.Append("StartLimitBurst=5\n\n");
Evidence
HomeDirectory/ConfigPath can incorporate a user profile path, which may contain spaces, but
SystemdUnit emits those paths unquoted; the spec also requires systemd escaping for all interpolated
values while ServiceText.SystemdValue only strips newlines.

src/Capacitor.Cli/Services/SystemdUnit.cs[24-33]
src/Capacitor.Cli/Services/ServiceText.cs[21-23]
src/Capacitor.Cli.Core/PathHelpers.cs[4-16]
docs/superpowers/specs/2026-06-10-daemon-service-supervisor-design.md[88-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SystemdUnit.Unit()` builds `Environment=` and `ExecStart=` by joining tokens with spaces and only collapsing newlines via `ServiceText.SystemdValue()`. This is not sufficient to ensure unit-file correctness when values contain spaces (common in user profile paths) or other characters that require quoting/escaping.

### Issue Context
The design spec explicitly states that *every interpolated value* must be escaped for its target format, including systemd unit lines.

### Fix Focus Areas
- src/Capacitor.Cli/Services/SystemdUnit.cs[24-33]
- src/Capacitor.Cli/Services/ServiceText.cs[21-23]
- src/Capacitor.Cli.Core/PathHelpers.cs[4-16]
- docs/superpowers/specs/2026-06-10-daemon-service-supervisor-design.md[88-92]

### Suggested fix
Implement systemd-appropriate escaping/quoting:
- Emit environment as `Environment="KEY=VALUE"` and escape quotes/backslashes as required.
- Quote/escape `ExecStart` tokens (binary path and args) so paths with spaces remain a single argv token.
- Extend/replace `ServiceText.SystemdValue()` accordingly (it currently only removes CR/LF).
- Add unit tests for systemd generation using a `DaemonBinaryPath` and `LogPath` that include spaces and for an env value containing characters that require escaping.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Launchd binary path wrong ✓ Resolved 🐞 Bug ≡ Correctness
Description
LaunchdServiceManager.Status reads the first <string> from the plist, but LaunchdUnit.Plist emits
the Label string before ProgramArguments, so status/doctor will report the label as the binary path
and can falsely flag it as missing.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R25-31]

+    public ServiceStatus Status(string serviceId) {
+        var path = LaunchdUnit.PlistPath(serviceId);
+        if (!File.Exists(path)) return new ServiceStatus(ServiceState.NotInstalled, null);
+        // First <string> in the plist is the daemon binary (ProgramArguments[0]) — for doctor.
+        var bin = XDocument.Load(path).Descendants("string").FirstOrDefault()?.Value;
+        var (code, stdout, _) = ServiceProcess.Run("launchctl", LaunchdUnit.PrintArgs(Uid(), serviceId));
+        return new ServiceStatus(LaunchdUnit.StatusFromPrint(code, stdout), bin);
Evidence
The plist generator writes the Label string before the ProgramArguments array, so selecting the
first <string> returns the label, contradicting the comment that it returns ProgramArguments[0].

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[25-31]
src/Capacitor.Cli/Services/LaunchdUnit.cs[23-38]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`LaunchdServiceManager.Status()` tries to extract the baked daemon binary path from the installed plist, but it currently takes the first `<string>` element in the document. In the generated plist, the first `<string>` is the service label, not the daemon binary path, so doctor/status output becomes incorrect and may emit false “binary missing” warnings.

### Issue Context
The plist is generated in `LaunchdUnit.Plist()` with the `<key>Label</key>` entry appearing before `<key>ProgramArguments</key>`.

### Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[25-31]
- src/Capacitor.Cli/Services/LaunchdUnit.cs[23-38]

### Suggested fix
Update `LaunchdServiceManager.Status()` to extract `ProgramArguments[0]` specifically (e.g., find the `<key>ProgramArguments</key>` element, then select the first `<string>` inside its following `<array>`), falling back to `null` if not found.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. Systemd --no-start not enforced 🐞 Bug ≡ Correctness
Description
SystemdServiceManager.Install does not stop an already-running unit when startNow is false, so
reinstalling with --no-start can leave an existing service running despite the flag.
Code

src/Capacitor.Cli/Services/SystemdServiceManager.cs[R31-37]

+    public void Install(ServiceSpec spec, bool startNow) {
+        Directory.CreateDirectory(SystemdUnit.UserUnitDir());
+        File.WriteAllText(SystemdUnit.UnitPath(spec.ServiceId), SystemdUnit.Unit(spec));
+        ServiceProcess.Check("systemctl", SystemdUnit.DaemonReloadArgs());
+        ServiceProcess.Check("systemctl", SystemdUnit.EnableArgs(spec.ServiceId));
+        if (startNow) ServiceProcess.Check("systemctl", SystemdUnit.RestartArgs(spec.ServiceId));
+    }
Evidence
The install path only restarts when startNow is true and has no stop call in the false case, so an
already-running service remains running.

src/Capacitor.Cli/Services/SystemdServiceManager.cs[31-37]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SystemdServiceManager.Install(spec, startNow: false)` writes/reloads/enables the unit but never stops it. If the service was already running from a previous install, `--no-start` won’t produce the expected “installed but stopped” state.

### Issue Context
The CLI exposes `--no-start` for `kcap daemon service install`.

### Fix Focus Areas
- src/Capacitor.Cli/Services/SystemdServiceManager.cs[31-37]
- src/Capacitor.Cli/Services/SystemdUnit.cs[49-51]

### Suggested fix
In `Install`, when `startNow` is false, explicitly stop the unit after reload/enable (best-effort):
- call `ServiceProcess.Run("systemctl", SystemdUnit.StopArgs(id))` (not `Check`) to avoid failing when already stopped.
This ensures `--no-start` is honored even on reinstall.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. ServiceProcess can deadlock 🐞 Bug ☼ Reliability
Description
ServiceProcess.Run reads StandardOutput to end before reading StandardError; with redirected streams
this can hang if the child process fills stderr while stdout is still being read.
Code

src/Capacitor.Cli/Services/ServiceProcess.cs[R11-25]

+    public static (int ExitCode, string StdOut, string StdErr) Run(string file, params string[] args) {
+        var psi = new ProcessStartInfo {
+            FileName               = file,
+            UseShellExecute        = false,
+            RedirectStandardOutput = true,
+            RedirectStandardError  = true,
+            CreateNoWindow         = true,
+        };
+        foreach (var a in args) psi.ArgumentList.Add(a);
+
+        using var p = Process.Start(psi) ?? throw new InvalidOperationException($"Failed to start {file}");
+        var stdout = p.StandardOutput.ReadToEnd();
+        var stderr = p.StandardError.ReadToEnd();
+        p.WaitForExit();
+        return (p.ExitCode, stdout, stderr);
Evidence
The code shows sequential blocking reads of the two redirected streams, a known hang pattern when
the unconsumed stream’s buffer fills.

src/Capacitor.Cli/Services/ServiceProcess.cs[11-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ServiceProcess.Run()` reads `StandardOutput.ReadToEnd()` and only then reads `StandardError.ReadToEnd()`. This pattern can deadlock when the child process writes enough data to the non-consumed stream to fill the OS pipe buffer.

### Issue Context
This runner is used for `launchctl/systemctl/schtasks` operations; a hang here blocks install/status/stop.

### Fix Focus Areas
- src/Capacitor.Cli/Services/ServiceProcess.cs[11-25]

### Suggested fix
Read both streams concurrently:
- start `stdoutTask = p.StandardOutput.ReadToEndAsync()` and `stderrTask = p.StandardError.ReadToEndAsync()`
- wait for process exit (`WaitForExitAsync` or `WaitForExit` after starting reads)
- await both tasks
Return collected output. Keep the API synchronous if needed by using `Task.WhenAll(...).GetAwaiter().GetResult()`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Windows binary path not validated ✓ Resolved 🐞 Bug ≡ Correctness
Description
WindowsScheduledTaskServiceManager.Status returns the wrapper .cmd path as ServiceStatus.BinaryPath,
so DaemonCommands.ReportInstalledServices only validates the wrapper exists and cannot detect when
the baked kcap-daemon.exe path has moved.
Code

src/Capacitor.Cli/Services/WindowsScheduledTaskServiceManager.cs[R28-32]

+    public ServiceStatus Status(string serviceId) {
+        var (code, stdout, _) = ServiceProcess.Run("schtasks", WindowsTaskUnit.QueryArgs(serviceId));
+        var bin = File.Exists(WindowsTaskUnit.WrapperPath(serviceId)) ? WindowsTaskUnit.WrapperPath(serviceId) : null;
+        return new ServiceStatus(WindowsTaskUnit.StatusFromQuery(code, stdout), bin);
+    }
Evidence
Doctor determines “binary missing” solely by File.Exists(st.BinaryPath), but Windows Status() sets
BinaryPath to the wrapper path, not the daemon exe path that can move across reinstalls.

src/Capacitor.Cli/Services/WindowsScheduledTaskServiceManager.cs[28-32]
src/Capacitor.Cli/Commands/DaemonCommands.cs[718-731]
docs/superpowers/specs/2026-06-10-daemon-service-supervisor-design.md[358-360]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
On Windows, `Status().BinaryPath` is used by `daemon doctor` to detect a moved/missing daemon executable. The Windows manager currently returns the wrapper `.cmd` path as `BinaryPath`, which means doctor will not flag a missing/moved `kcap-daemon.exe`.

### Issue Context
`DaemonCommands.ReportInstalledServices()` checks `File.Exists(st.BinaryPath)` and prints a warning if it’s missing.

### Fix Focus Areas
- src/Capacitor.Cli/Services/WindowsScheduledTaskServiceManager.cs[28-32]
- src/Capacitor.Cli/Commands/DaemonCommands.cs[718-731]
- src/Capacitor.Cli/Services/WindowsTaskUnit.cs[14-25]

### Suggested fix
Update `WindowsScheduledTaskServiceManager.Status()` to return the *daemon executable path baked into the wrapper* (e.g., parse the exec line in the `.cmd` wrapper and extract the first quoted token), not the wrapper path itself. Optionally keep exposing wrapper path separately via a different field/log line if useful.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

The default AddSimpleConsole provider queues writes on a background thread
(ConsoleLoggerProcessor) that can drop messages enqueued during runtime
teardown — so a ProcessExit / signal-handler log line may never reach the
terminal even though the hook fired. Add a DeathRattle() helper that writes
straight to Console.Error (flushed, best-effort) from every exit hook
(lifetime stopping/stopped, AppDomain unhandled/ProcessExit, unobserved
task, POSIX signals), guaranteeing at least one byte lands before the
logging pipeline tears down.

This can't catch an external SIGKILL (jetsam/OOM) — which is what the new
`kcap daemon service` supervisor addresses — but it does surface every
catchable exit reason.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alexeyzimarev alexeyzimarev changed the title feat(daemon): per-user service supervisor (install/uninstall/start/stop) [AI-818] feat(daemon): per-user service supervisor (install/uninstall/start/stop) Jun 10, 2026
@linear-code

linear-code Bot commented Jun 10, 2026

Copy link
Copy Markdown

AI-818

alexeyzimarev and others added 2 commits June 10, 2026 21:17
- P1: `kcap uninstall` now deregisters installed OS services first
  (ListInstalled → Uninstall) before `daemon stop --yes`. Previously the
  stop-deferral for service-managed daemons left launchd/systemd/Scheduled
  Task units registered while the config dir was deleted.
- P2: systemd Environment= values and ExecStart argv are now double-quoted
  (with \ and " escaped) when they contain whitespace — paths/PATH entries
  with spaces no longer split.
- P2: launchd doctor reads ProgramArguments[0] (LaunchdUnit.BinaryFromPlist)
  instead of the document's first <string>, which was the Label — fixes a
  false "binary missing" for every valid launchd service.
- P2: Windows doctor reads the kcap-daemon.exe path baked inside the .cmd
  wrapper (WindowsTaskUnit.BinaryFromWrapper) instead of the wrapper path, so
  a moved daemon binary is detected even when the wrapper still exists.

+4 unit tests (binary parsers, systemd space-quoting). 1321 pass, AOT clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the systemd Environment=/ExecStart quoting: the doctor reader
still used Split(' ', 2), so a quoted `ExecStart="/opt/k cap/kcap-daemon" …`
(spaced path) parsed to `"/opt/k`, false-reporting a missing binary. Add a
quote-aware SystemdUnit.BinaryFromUnit (reverses the \\ / \" escaping) and use
it in SystemdServiceManager.Status. +1 test covering the whitespace path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit 50b7e6e into main Jun 10, 2026
4 checks passed
@alexeyzimarev
alexeyzimarev deleted the feat/daemon-service-supervisor branch June 10, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant