Skip to content

fix(service): repair, uninstall and separate the macOS launch agent - #99

Open
vishr wants to merge 19 commits into
mainfrom
fix/macos-service-lifecycle
Open

vishr wants to merge 19 commits into
mainfrom
fix/macos-service-lifecycle

Conversation

@vishr

@vishr vishr commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

What changed

Fixes the macOS Driga service lifecycle and corrects the MCP and service docs. The findings came from an independent code review; each was checked against main before changing it.

  1. HIGH: repairing a stale launch agent that is loaded but not running. internal/service/launchd_darwin.go now runs launchctl itself instead of going through kardianos. Install always runs launchctl bootout gui/$UID/<label> (and ignores "not loaded") before it writes a changed plist. Start runs bootstrap gui/$UID <plist> on an unloaded job and kickstart on a loaded job whose process has exited. Restart uses kickstart -k. Status comes from launchctl print. Nothing calls load or unload any more. The fake launchctl models a launchd domain. In the new loaded-but-not-running test, a stale plist's executable no longer exists. docs/release.md describes the new sequence.
  2. HIGH: orphaned launch agent after uninstall. The cask adds uninstall launchctl: "run.driga.service", signal: [["TERM", "run.driga"]] and a zap trash: list covering data, logs, config, caches, prefs, WebKit, saved state and the legacy collector plist. Homebrew skips signal on upgrade, where the app restarts itself. I left out quit because it can stall Homebrew for 10 seconds with an Automation prompt. An upgrade runs the old version's uninstall, launchctl included, so uninstall_preflight first saves the plist to ~/Library/Caches/run.driga. The postflight reinstalls from the live plist or that copy (see Review follow-ups, item 5, for how the copy is now tied to the upgrading brew process).
    For an app deleted without brew, KeepAlive is now PathState {<drigad path>: true}, so launchd stops respawning a missing helper and restarts the job when the path comes back. The service also checks its executable once a minute and exits cleanly when it is gone. The docs, the README and the cask caveats say to run driga service uninstall before deleting the app by hand.
  3. MEDIUM: dev and release services collide. Release builds now carry the link-time marker -X github.com/labstack/driga/internal/variant.release=true (moved from internal/service in the follow-ups; empty in snapshots), which .goreleaser.yaml sets on all three builds. Every other build is a development build: just build, just install, go build and go run. It installs run.driga.service.dev, driga-dev on Linux or Driga service (dev) on Windows, and logs under driga-dev. An executable named driga-<name> still uses <name>. docs/usage.md, the README source-build section and CONTRIBUTING are updated.
  4. MEDIUM: docs and MCP truthfulness. The README no longer calls the MCP tools read-only; it says driga mcp serves read views plus Driga dashboard edits. The README's paragraph saying the app "removes the service and runs it itself" now describes the current design and how to uninstall. The site's MCP guide and index are corrected too.
    All Chat action tools now have annotations, following Canon mcp.md:
    • run_command: destructive, open world
    • answer_prompt, kill, cleanup: destructive
    • ack_inbox: idempotent, not destructive
    • jump, approve: not destructive
    • summarize: open world (it goes through the Claude CLI)
      The generated MCP reference now shows Destructive and Idempotent columns. A test requires every served tool to carry annotations, and every state-changing tool to set the destructive and open-world hints explicitly.
  5. LOW: plist and polling. The plist now sets ProcessType Background and ThrottleInterval 30. The app still checks the lock and process every 2 seconds, which needs no subprocess. Lifecycle reuses the service manager's answer (from launchctl, systemd or Task Scheduler) for 30 seconds while those stay the same, and asks again as soon as the service starts or stops.
  6. Environment refresh. On macOS, install compares the complete rendered plist, environment included, with the installed one, so a changed login-shell PATH reaches the service at the next app launch. If the app cannot read the login shell, it reuses the environment the service already records (service.RecordedEnvironment), so a bare Finder PATH never replaces it. On Linux and Windows an existing service keeps its environment, which the docs now state.

Why

kardianos/service's darwin backend uses launchctl load/unload and checks status with list. It cannot tell a loaded-but-not-running job from a stopped one, and load refuses a job launchd already has. The domain commands handle both cases. A link-time marker is the only build-independent way to tell a release from a checkout build, since nothing produced the driga-dev executable name.

Risk and rollout

  • No store migration. Nothing changes in agent settings or captured text, and nothing new signals or kills processes. The cask's signal only TERMs the Driga app when it is uninstalled.
  • After upgrading, every existing macOS plist is rewritten once, because the KeepAlive, ProcessType and ThrottleInterval keys changed. That causes one service restart.
  • Anyone running a checkout build as run.driga.service gets a second service under the .dev label the next time they install. Run driga service uninstall with the old build first (documented in usage.md).
  • ProcessType Background applies launchd's background resource policy (lower CPU and I/O priority) to the collector, which also serves Chat and actions. Watch whether the app feels slower; Adaptive is the fallback.
  • Not verified against real launchd or a real Homebrew install. Both are tested only through fakes: a fake launchctl domain, and the cask's Ruby block run under a fake DSL with a temporary HOME.

Contract changes

  • MCP: Chat action tools gain annotations. Names, arguments and results are unchanged.
  • Service labels: non-release builds install the .dev / -dev label.
  • CLI: driga service install --refresh-env (see Review follow-ups). No config keys or schema changes.

Verified

Run on macOS (darwin/arm64) on the host:

  • gofmt -l . is clean, go mod tidy -diff, go vet ./... (native, GOOS=windows, GOOS=linux)
  • go run ./cmd/driga-docgen --check, go generate ./internal/config with no .env.example diff
  • scripts/isolated.sh go test -count=1 ./...: all packages pass
  • scripts/isolated.sh go test -race on internal/service, internal/mcp, internal/desktop, scripts: pass (I did not run the full just race)
  • Cask Ruby block tested with Ruby 4.0 in four scenarios: fresh install, existing service, upgrade through the kept copy, and a stale copy
  • Not run: just ui-check / design-check (UI untouched), goreleaser check / just snapshot (goreleaser is not installed locally; CI's snapshot job covers it), actionlint, govulncheck

Follow-ups

  • SMAppService migration (SMAppService.agent(plistName:) with the plist inside the bundle) is deliberately left out of this PR. It would replace the launch agent registration and give users the Login Items UI.
  • An event-driven service status in the app (for example a lock-file watch) instead of the 2-second lock poll.
  • Environment refresh for existing Linux and Windows services.
  • Check ProcessType Background against real Chat and actions latency.

Review follow-ups

Findings from the Opus and Codex reviews of this PR, a full Codex review of main (breaking changes accepted), and automated security review of the follow-ups. Each was checked against the code first.

  1. bootout/bootstrap race. bootout now polls launchctl print until launchd has let the job go (100 ms steps, at most 10 s) and treats 36 as in progress. bootstrap retries 5/36/37 with backoff (6 attempts) and counts a job another client loaded only when it runs. Start no longer kicks a job it just bootstrapped, and watches a loaded-but-not-running job for 1 s before kicking it, so a definition being torn down is bootstrapped instead. The fake launchctl gained a mode where a job stays loaded for N prints after bootout (kickstart during teardown fails the test). Tests: teardown wait, Stop then Start, Start on a job being torn down, bootstrap backoff, bounded wait.
  2. No GUI session. Exit 112 / "Could not find domain" maps to ErrNoGUISession ("log in to the desktop session to start the Driga service"). Uninstall treats it as not loaded and still deletes the plist. Stop is a no-op. Start, Restart and Status return the message. Install writes the plist for launchd to load at the next login. Covered by a domain-absent fake.
  3. Environment churn. The app's login-shell environment is now the only source. The app runs driga service install --refresh-env. A plain driga service install keeps the environment the plist records. A restart caused by a changed environment logs the variable names only (service_env_changed). Doctor's stale-plist hint names --refresh-env.
  4. Lifecycle cache. Control clears the registration cache, so the tray asks launchctl again right after any action.
  5. Cask plist heuristic. The 10-minute rule is gone. uninstall_preflight names its copy run.driga.service.<brew pid>.plist and deletes it from at_exit. Only the same brew process's postflight restores it, however long the upgrade takes. Copies from other processes are deleted and never restored. The restored plist keeps its recorded environment, so the postflight runs a plain service install with no /usr/bin/env pass-through. scripts/macos_app_test.go covers fresh, existing, a slow upgrade (day-old copy, must restore), an ordinary uninstall (no copy left after brew exits, and a later install must not restore) and another process's copy. Deviation: a brew reinstall also restores the service, because it runs uninstall launchctl too.
  6. Snapshots marked as releases. The marker is {{ if not .IsSnapshot }}true{{ end }} on all three builds. A test renders the ldflags for both cases. CONTRIBUTING, usage and the release guide are updated.
  7. Dev-build advice. usage.md drops the "uninstall with the old build first" advice: installing a release replaces whatever holds the release's label.
  8. Dev data directory. The marker moved to the new internal/variant package, which config and service both read. Development builds default to driga-dev for data and spool (~/Library/Application Support/driga-dev, ~/.local/share/driga-dev, ~/.local/state/driga-dev/events, %LOCALAPPDATA%\driga-dev). The config file is still shared. The settings reference and .env.example are regenerated.
  9. Source install builds only driga. The README, the site install guide and the release guide now build driga and link drigad (or just build). driga service install checks that the drigad it would run exists and is executable, and errors clearly if not.
  10. Renamed Driga.app. BundlePath accepts any <name>.app with driga/drigad in Contents/MacOS whose Info.plist says CFBundleIdentifier is run.driga. self.CLI maps the nested Driga Service.app back to the outer bundle whatever its name. Tests cover a renamed bundle, another app with the same layout, and a bundle without an Info.plist.
  11. Repair without rollback. Install renders and checks the new plist (label and program) and writes it beside the old one with an fsync. Only then does it boot the job out (waiting as in item 1), rename the new plist into place and bootstrap it. If launchd refuses it, the previous plist is restored and bootstrapped again, and the error says so. The Control comment now describes this. Tests cover a refused definition (the old one runs again) and a bounded teardown wait (old plist untouched, no temp file left behind).
  12. Spool permissions. Append fchmods the day file it writes to 0600 and the directory to 0700. spool.Protect tightens the directory and every file, and the service runs it at start and at each hourly prune (private DACL on Windows). Tested with a 0755 dir and 0644 files.
  13. README "read-only MCP". Verified: driga mcp registers action tools only for Chat's MCP child (o.Chat), so "read views plus dashboard edits" is accurate.

Security review of the follow-ups (internal/command, run_command sandbox policy):

  1. Denylist missed other variants' state. The new config.StateDirs lists the config file and default config dir, this process's data/spool overrides, and the data and state directories of release, -dev and any driga-<name> variant found. The policy denies all of them, plus driga, driga-dev and driga-* under each HOME default root regardless of XDG variables. Tested with defaults, overrides, a symlink and a relative path.
  2. Symlink canonicalization differential. canonical now resolves like the kernel. It walks from the root, splices each symlink target relative to the already-resolved prefix, never cleans .. lexically, bounds hops at 40, and follows dangling links. Existing paths go to EvalSymlinks first. Tests: a relative link inside a symlinked parent, an absolute link, a dangling tail, .. after links, and a loop.
  3. Windows rooted targets. Volume-less rooted targets (\x) resolve at the link's drive. Tests are platform-independent table tests.
  4. Windows device/volume targets. Fail closed: only relative, rooted and drive-absolute (C:\x) targets resolve. UNC, \\?\ (including Volume{GUID} and GLOBALROOT), \\.\, drive-relative C:x and other colon forms return errUnresolvableLink, and the classifier refuses a path through one (Never).

Breaking changes

  • Development builds (just build, go build, go run, GoReleaser snapshots) use their own driga-dev data and spool directories, so they no longer see the release's database. The link-time marker is now -X github.com/labstack/driga/internal/variant.release=true.
  • On macOS, driga service install keeps the service's recorded environment. Pass --refresh-env to record the calling shell's.
  • Driga.app is found by bundle identifier and layout, so renamed bundles work. A bundle whose Info.plist does not say run.driga is no longer treated as Driga.app.
  • run_command refuses a path through a Windows symlink whose target is UNC, \\?\, \\.\ or drive-relative.

Follow-up verification

Run on macOS (darwin/arm64) on the host:

  • gofmt -l . clean, go mod tidy -diff, go vet ./... native, GOOS=windows and GOOS=linux
  • go run ./cmd/driga-docgen --check; go generate ./internal/config leaves no .env.example diff
  • scripts/isolated.sh go test -count=1 ./...: all packages pass
  • -race on internal/service, variant, config, spool, self, command, cli, desktop, collector and scripts: pass
  • Cask Ruby block under the fake DSL: fresh, existing, slow upgrade, ordinary uninstall followed by a later install, another process's copy
  • Still not verified against real launchd, a real Homebrew upgrade, or Windows hosts. The exit codes 112, 5, 36 and 37 come from launchctl documentation and the reviews, not from observation. The Windows path tests are host-independent tables, not runs on Windows. just ui-check, goreleaser check and just snapshot were not run.

https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ

vishr added 19 commits November 1, 2026 21:45
A job launchd still had but whose process was not running (Driga.app moved)
could not be repaired: install unloaded only a running job, and start's
launchctl load refused one already loaded. The launchd controller now
boots the job out before writing a changed plist, bootstraps or kickstarts
it, and never uses the deprecated load/unload.

Install replaces the plist whenever it differs from what it would write,
including the recorded login environment. The plist keeps the job alive
only while its executable exists, runs it as a Background process with a
30-second throttle, and the service exits when its executable is deleted.

Builds without the release link-time marker install the dev label, so
just build and go build no longer take over a release's service.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
brew uninstall left a KeepAlive launch agent pointing at a deleted helper.
The cask now uninstalls run.driga.service, signals a running app, and zaps
Driga's data. Its uninstall_preflight keeps the plist so an upgrade, which
runs the old uninstall, still restores the service. Release builds carry the
internal/service.release marker.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
The app spawned launchctl every two seconds. Observe now reuses the service
manager's answer for 30 seconds while the lock and process are unchanged.
When the login shell cannot be read, the app keeps the environment the
service records instead of reinstalling it with a bare PATH.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
Action tools now declare destructive, idempotent and open-world hints, and
the generated reference lists them. README no longer calls driga mcp
read-only or says the app runs the service itself, and the service docs
describe dev labels, uninstalling and environment refresh.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
The release marker moves to internal/variant, which config and service
both read. A development build now defaults to a driga-dev data and
spool directory, so a development service never shares the release's
database or hook spool; the configuration file stays shared.

BREAKING CHANGE: a development build no longer reads the release's
default data directory, and the link-time marker is now
-X github.com/labstack/driga/internal/variant.release=true.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
A GoReleaser snapshot set the release marker too, so a snapshot
installed the release's service label. The marker is now empty in
snapshots. The usage guide drops its dev-build removal advice: a
release install replaces whatever holds the release's label.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
The run_command denylist named only this process's data directory and
the release's default paths, so with development data directories a
command could read another variant's database, spool or keys. The
policy now denies config.StateDirs (the configuration, this process's
overrides, and the release, development and driga-<name> data and
state directories) plus each variant under the HOME default roots, and
follows a dangling symlink to the path it names.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
canonical joined a relative symlink target to the unresolved directory
of the link, so a link inside a symlinked parent resolved elsewhere than
the kernel resolves it. It now walks the path from the root like the
kernel: each component is checked against the already resolved prefix,
a symlink's target is spliced in relative to that prefix, ".." is never
cleaned lexically, and hops are bounded. A dangling link still resolves
to the path it names.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
A volume-less target such as \Users\x\AppData\Local\driga is rooted at
the link's volume on Windows, but the walker treated it as relative to
the link's directory. linkTarget now applies each platform's rules
whatever the host: rooted volume-less targets, drive-relative C:x,
UNC shares and \\?\ prefixes. An existing path goes to EvalSymlinks
first; the walk covers missing tails and dangling links.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
bootout returns before launchd has let the job go, so the bootstrap
that followed could fail with 5, 36 or 37, and Start could kick a
definition still being torn down. bootout now polls launchctl print
until the job is gone (at most 10 seconds), bootstrap retries a busy
launchd with backoff, and Start watches a loaded job that does not run
before it kicks it.

Install renders and checks the new plist and writes it beside the old
one before it boots the job out, then renames it into place and
bootstraps it; when launchd refuses it, the previous plist is restored
and bootstrapped again. Without a gui/<uid> domain (exit 112, nobody
logged in to the desktop) uninstall still deletes the plist, and start
and status ask for a desktop login. The fake launchctl models a job
that stays loaded after bootout and an absent domain.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
Observe reused the last launchctl answer for 30 seconds while the lock
and process looked the same, so the tray could show a stale
registration right after an install, stop or uninstall. Control now
clears that cache, and its comment says what a failed macOS install
leaves behind.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
A terminal's driga service install recorded the terminal's environment
and the next app launch recorded the login shell's, so each flipped the
plist and restarted the service. The app's login-shell environment is
now the one source: the app installs with --refresh-env, and a plain
driga service install keeps the environment the plist records. A
restart caused by a changed environment logs the variable names, never
their values. Doctor's stale-plist hint names --refresh-env.

BREAKING CHANGE: driga service install on macOS no longer records the
calling shell's environment over an installed service's; pass
--refresh-env to do that.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
The README's source install built only driga, but the service runs
drigad beside it, so the installed service could not start. driga
service install now checks that the drigad it would run exists and is
executable, with a clear error, and the README, install guide and
release guide build both.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
After stripping \\?\, a target such as \\?\Volume{GUID}\, \\?\GLOBALROOT\
or \\.\pipe\ fell through to the relative branch. Rather than grow a
Windows path parser, the walker now accepts only relative, rooted
(\x, at the link's drive) and drive-absolute (C:\x) targets; UNC,
\\?\, \\.\, drive-relative and other colon forms fail closed, and the
classifier refuses a path through such a link. Existing paths still go
to EvalSymlinks first.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
BundlePath required the bundle to be named Driga.app, and self.CLI
mapped the nested service back only into a Driga.app, so renaming the
app lost the bundle association and pointed the service at the wrong
executable. The app is now any <name>.app whose Info.plist names
run.driga, with driga or drigad in Contents/MacOS; the helper maps back
to the outer bundle whatever its name.

BREAKING CHANGE: a bundle without an Info.plist naming run.driga is no
longer treated as Driga.app.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
MkdirPrivate and an O_CREATE open keep an existing mode, so a spool an
older release left group- or world-readable stayed that way. Append now
tightens the day file it writes and the directory, and the service
tightens the whole spool when it starts and at each hourly prune: 0700
and 0600 on Unix, a private DACL on Windows, as the store does.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
… process

The postflight restored any kept plist under ten minutes old, so an
upgrade slower than that lost the service and an ordinary uninstall
followed by an install within ten minutes brought it back. The
preflight now names its copy after the brew process and deletes it when
that process exits; only the same process's postflight restores it,
and copies from other processes are deleted. The restored plist keeps
its recorded environment, so the postflight runs a plain service
install instead of passing the environment through /usr/bin/env.

Claude-Session: https://claude.ai/code/session_01XkoYQXnzjsCqogTYDHrxNJ
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