diff --git a/docs/prd/PRD-004-cli-onboarding-and-config.md b/docs/prd/PRD-004-cli-onboarding-and-config.md index f01d73ad9..03fe4f848 100644 --- a/docs/prd/PRD-004-cli-onboarding-and-config.md +++ b/docs/prd/PRD-004-cli-onboarding-and-config.md @@ -120,8 +120,11 @@ Command ownership stays explicit: - `netclaw daemon status` — check if daemon is running, show PID and uptime - `netclaw daemon install` — register as a systemd user service (`~/.config/systemd/user/netclaw.service`, no sudo). Supports - `loginctl enable-linger` for surviving logout. -- `netclaw daemon uninstall` — remove systemd user service registration + `loginctl enable-linger` for surviving logout. Captures the operator's real + shell `PATH` into `~/.netclaw/config/daemon.env` (loaded via `EnvironmentFile=`) + so the daemon's shell tool resolves the same binaries the operator can. +- `netclaw daemon uninstall` — remove systemd user service registration and the + captured `daemon.env` ### TUI-Interactive Commands (Termina, daemon required) @@ -317,8 +320,12 @@ The CLI SHALL provide commands to manage the daemon lifecycle: - `netclaw daemon status` SHALL report daemon state (running/stopped, PID, uptime) - `netclaw daemon install` SHALL register as a systemd user service (Linux) or LaunchAgent (macOS). No sudo required — uses `systemctl --user` and - `loginctl enable-linger` on Linux. -- `netclaw daemon uninstall` SHALL remove the service registration + `loginctl enable-linger` on Linux. On Linux it SHALL capture the operator's + real `PATH` (from the CLI process, without spawning a shell) into a + netclaw-owned `EnvironmentFile` so the daemon's shell tool resolves + operator-installed binaries; `netclaw doctor --fix` SHALL rehydrate it. +- `netclaw daemon uninstall` SHALL remove the service registration and the + captured environment file ### CLI-013 Daemon Process diff --git a/docs/spec/SPEC-011-daemon-architecture.md b/docs/spec/SPEC-011-daemon-architecture.md index c5035a368..c458e216e 100644 --- a/docs/spec/SPEC-011-daemon-architecture.md +++ b/docs/spec/SPEC-011-daemon-architecture.md @@ -224,15 +224,17 @@ capture crash signals even when the process is unstable. ```ini [Unit] -Description=Netclaw Agent Daemon +Description=Netclaw Daemon After=network.target [Service] Type=simple ExecStart=/path/to/netclawd +ExecStop=/path/to/netclaw daemon stop Restart=always RestartSec=5 Environment=DOTNET_ENVIRONMENT=Production +EnvironmentFile=-/home/you/.netclaw/config/daemon.env [Install] WantedBy=default.target @@ -241,7 +243,26 @@ WantedBy=default.target No sudo required. Uses `systemctl --user enable netclaw` and `loginctl enable-linger $USER` to survive user logout. -`netclaw daemon uninstall` stops the service and removes the unit file. +**Shell-tool PATH.** A systemd `--user` service starts with a sanitized, +non-interactive environment that does not inherit the operator's login-shell +`PATH`, so the agent's shell tool cannot resolve `netclaw`, `dotnet`, or +`~/.local/bin` binaries. Rather than bake a guessed directory list into the unit +(which can never anticipate every environment — see issue #1544), `install` +**captures the operator's real `PATH` from its own process** (the CLI is a child +of the operator's shell, so no shell is spawned and no dotfiles are sourced) and +writes `PATH=::` (de-duplicated, empty elements dropped; the +floor `/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin` keeps the shell functional even if the +captured PATH was empty or partial) to `~/.netclaw/config/daemon.env`. The unit +loads it via `EnvironmentFile=-…` (the `-` makes a missing file degrade tool +resolution rather than block startup). `netclaw doctor --fix` rehydrates the file +from the operator's current `PATH`; `SystemdUnitPathDoctorCheck` validates the +wiring and contents. Producer, rehydrator, and validator share +`DaemonPathEnvironmentFile`. The value is a snapshot as of the last +`install`/`doctor --fix`, so after installing new tools re-run either and then +`systemctl --user restart netclaw`. + +`netclaw daemon uninstall` stops the service and removes the unit file and the +`daemon.env` file. ### Service Registration (macOS) diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index 237685016..a514dbd60 100644 --- a/feeds/skills/.system/files/netclaw-operations/SKILL.md +++ b/feeds/skills/.system/files/netclaw-operations/SKILL.md @@ -3,7 +3,7 @@ name: netclaw-operations description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance." metadata: author: netclaw - version: "2.23.1" + version: "2.24.0" --- # Netclaw Operations diff --git a/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md b/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md index 0cca55f91..ec9ffd12e 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md +++ b/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md @@ -78,7 +78,7 @@ debugging a daemon-wide problem → read `daemon.log`. | Daemon won't start | crash logs at `~/.netclaw/logs/crash-*.log` | | Docker daemon cannot create `/home/netclaw/.netclaw/*` | Official image entrypoint repairs writable bind mounts to UID/GID `1654:1654`; if bypassed or read-only, run `sudo chown -R 1654:1654 ` or use a Docker named volume | | Discord/Slack channel offline | `netclaw status` shows the channel `disconnected` with a reason. Discord may also report `degraded` when Discord.Net says the socket is connected but the gateway is not ready, such as after a resumed session that Netclaw is replacing with a clean reconnect. A misconfigured channel (bad token, missing Discord Message Content intent) degrades only that channel — the daemon keeps running and other channels are unaffected. A transient network failure retries automatically; a config/permission failure stays offline until the operator fixes the config and restarts the daemon. | -| `command not found` for `netclaw` from shell tool when daemon runs as systemd service | `netclaw doctor` (the **Systemd Unit PATH** check warns when the unit was installed before PATH was baked in) | +| `command not found` for `netclaw`/`dotnet`/a user tool from the shell tool when the daemon runs as a systemd service | The systemd `--user` service does not inherit your login-shell `PATH`; `netclaw daemon install` captures it into `~/.netclaw/config/daemon.env`. Run `netclaw doctor` (the **Systemd Unit PATH** check flags a missing/stale/legacy env file), then `netclaw doctor --fix` to rehydrate `PATH` from your current shell (or re-run `netclaw daemon install`), and finally `systemctl --user restart netclaw`. Installed a new tool after install? Its dir won't be seen until you re-run one of those and restart. Per-directory managers (`mise`/`asdf`/`direnv`) are not captured. | If webhook notifications are configured, daemon crash paths emit `daemon.crashing` operational alerts with context (PID, reason, and latest known diff --git a/openspec/changes/systemd-daemon-path-capture/.openspec.yaml b/openspec/changes/systemd-daemon-path-capture/.openspec.yaml new file mode 100644 index 000000000..43e65ca6e --- /dev/null +++ b/openspec/changes/systemd-daemon-path-capture/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-03 diff --git a/openspec/changes/systemd-daemon-path-capture/design.md b/openspec/changes/systemd-daemon-path-capture/design.md new file mode 100644 index 000000000..78b561b1b --- /dev/null +++ b/openspec/changes/systemd-daemon-path-capture/design.md @@ -0,0 +1,168 @@ +## Context + +A systemd `--user` service starts with a sanitized, non-interactive environment. It does **not** +read `~/.bashrc`/`~/.profile` and does **not** inherit the operator's login-shell `PATH` — by +design, so services are reproducible. Netclaw's agent shell tool (and +`BackgroundJobExecutionActor`) spawn `bash -c` from the daemon, so under the installed service they +see only whatever `PATH` the unit provides. + +Today `netclaw daemon install` compensates by baking a **hardcoded** `Environment=PATH=` list into +the unit (`DaemonManager.ComposeSystemdUnitPath`): install dir, `~/.local/bin`, and the standard +system dirs. GitHub #1544 is the failure mode: `~/.dotnet` is not in that list, so the daemon's +shell tool cannot find `dotnet`. Any hardcoded list is a guess that will be wrong for some +operator. + +The operator's shell already knows the correct `PATH`. Crucially, so does any `netclaw` CLI process +the operator launches from that shell — it inherits `PATH` as a normal environment variable. This +design captures that inherited value instead of guessing, and never runs operator shell code from +the daemon. + +Confirmed premise (from code review of `Netclaw.Security`): the shell command policy is +`PATH`-independent. `ShellCommandPolicy` is a deny-list matching the literal typed verb token +(tokenized, punctuation-trimmed); `ApprovalPatternMatching` keys on verb + directory scoping. No +enforcement path resolves a command against `$PATH`, and there is no resolved-path allow-list. +Therefore widening the daemon's `PATH` changes only bare-name *resolution* (ergonomics), never the +security decision — so capturing the operator's `PATH` is safe. + +## Goals / Non-Goals + +**Goals:** + +- The installed daemon's shell tool resolves the same tools the operator can resolve, without + hand-maintaining a directory list. +- Never execute operator shell/dotfiles from the daemon (no boot hang, no surprise side effects). +- Keep the value refreshable through commands the operator already runs (`daemon install`, + `doctor --fix`). +- Keep the producer (`DaemonManager`) and consumer (`SystemdUnitPathDoctorCheck`) in exact + agreement on the file, wiring, and contents. + +**Non-Goals:** + +- Live/continuous `PATH` sync. The value is a snapshot as of the last install / `doctor --fix`. +- Capturing dynamic per-directory `PATH` managers (`mise`, `asdf`, `direnv`). +- Changing manual `netclaw daemon start` (non-systemd), which already inherits the operator `PATH`. +- macOS/Windows service install (still unsupported). + +## Decisions + +### D1: Capture the CLI's inherited PATH — don't guess, don't source a shell + +Read `Environment.GetEnvironmentVariable("PATH")` in the CLI process at install / `doctor --fix` +time. The CLI is a child of the operator's interactive shell, so this is the operator's real `PATH` +with **zero** shell execution. + +- **vs. hardcoded list (status quo):** guaranteed to miss someone's tools (the #1544 bug). Rejected. +- **vs. daemon spawns `bash -lc` at startup:** would be fresher, but runs operator dotfiles in an + unsupervised background service — can hang boot, has unpredictable side effects, and to "fail + loud" on probe failure we'd need a fallback anyway. Rejected; violates the repo's + no-silent-fallback posture. +- **vs. global `~/.config/environment.d/`:** the systemd-blessed mechanism, but it (a) changes + `PATH` for **every** user service, not just netclaw, and (b) its `${PATH}` resolves to the + manager's sanitized default — it would **not** capture `~/.dotnet` without the operator hand- + listing dirs, so it does not actually solve #1544. Rejected as the primary mechanism. + +### D2: Deliver via `EnvironmentFile=` (unit-scoped), not inline `Environment=` or environment.d + +The unit references a netclaw-owned file: `EnvironmentFile=` with a single `PATH=...` line. + +- Unit-scoped → zero blast radius on other user services (unlike environment.d). +- Separately rewritable → `doctor --fix` can rehydrate `PATH` without rewriting the unit. +- `daemon-reload` + service restart deterministically re-reads it (vs. environment.d's fuzzy + re-read semantics). +- Clean removal on uninstall. + +### D3: Env-file location reuses `NetclawPaths` + +`DaemonManager` already receives a `NetclawPaths`. Add a `DaemonEnvironmentFilePath` property under +the existing `ConfigDirectory` (e.g. `/config/daemon.env`) rather than computing a new +ad-hoc path in `DaemonManager`. This follows the repo's "reuse before you add" rule and keeps the +path a single source of truth shared by the producer, the doctor check, and uninstall. The unit +references it by resolved absolute path. + +### D4: Install-dir is prepended to the captured PATH + +Provisioned value = `installDir : : `, de-duplicated with +empty elements dropped. installDir leads (bundled CLI wins), the operator's real dirs follow (the +point of #1544), and a guaranteed floor (`/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin`) is always +appended. + +**The floor is not a guess at the operator's tools — it is a functional baseline** (a POSIX shell, +coreutils, admin `sbin` tools) that the old unit-baked PATH provided unconditionally. Code review +caught that dropping it regressed two cases the old code could never hit: an empty/unset capture +would yield `PATH=installDir` alone (silently breaking *every* shell command, and the doctor check +would still pass it), and a normal desktop login PATH omits `/usr/sbin`,`/sbin` (so `ip`/`iptables` +would stop resolving). Appending the floor restores the old guarantee while keeping the captured +operator dirs. **Empty `PATH` elements are dropped** because POSIX resolves an empty element to the +current directory — with the daemon running `bash -c` in an agent-controlled workspace, a captured +`::` (common from `PATH="$PATH:"` in a dotfile) would let a planted binary shadow a system command. + +### D5: `doctor --fix` rehydration lives outside the config-file gate + +`DoctorFixService.BuildPlanAsync` currently early-returns when `netclaw.json` is absent and only +emits `DoctorFileFix`es against the config JSON. The daemon env file is independent of app config, +so its rehydration fix is evaluated **before/around** that early-return. It reuses the existing +`DoctorFileFix` (path + original + updated text) + `ApplyAsync` file-write model. The fix is emitted +only when the file is missing, unwired, or missing the install dir — avoiding needless churn. The +fix description/plan surfaces the required `systemctl --user restart netclaw`; `ApplyAsync` writes +files only and never restarts the daemon. + +### D6: Doctor check validates wiring + file, not an inline PATH line + +`SystemdUnitPathDoctorCheck` moves from parsing the unit's `Environment=PATH=` to: (1) unit has +`EnvironmentFile=` → env file; (2) env file exists; (3) env file `PATH` includes the install dir +(install dir still derived from the unit's `ExecStart`). This keeps the check as the enforcement of +the producer/consumer contract, now against the new shape. + +### D8: `EnvironmentFile=-` is tolerant of a missing file (surfaced during implementation) + +The unit uses the `-` prefix (`EnvironmentFile=-`), so a deleted/missing env file +degrades the daemon's shell-tool PATH to the sanitized systemd default rather than preventing +the entire daemon (Slack, connectors, everything) from starting. The strict alternative +(`EnvironmentFile=`, service fails to start on a missing file) is "louder", but taking the +whole daemon down over a missing PATH helper file is disproportionate — and PATH is *not* a +security boundary here (see the confirmed premise), so a degraded PATH is a functionality gap, not +a privilege issue. The `SystemdUnitPathDoctorCheck` warning + `doctor --fix` make the degraded +state discoverable and repairable, which is the right altitude for a non-security degradation. + +### D7: Freshness & restart are explicit, not implicit + +`PATH` is current as of the last `install` / `doctor --fix`. Installing a new tool afterward +requires re-running either, then restarting the service. Both commands print the restart +instruction. The doctor **check** nudges when the file is missing/stale, closing the loop. + +## Risks / Trade-offs + +- **[Snapshot goes stale after installing new tools]** → `doctor --fix` rehydrates in one command; + the doctor check warns when the install dir is absent from `PATH`. Documented in the + `netclaw-operations` skill. +- **[`EnvironmentFile=` only re-read on unit (re)start]** → install and `doctor --fix` both surface + the explicit `systemctl --user restart netclaw` step; no silent restart. +- **[Captured PATH is only as good as the shell that ran the command]** → if the operator installs + from a minimal shell missing a tool dir, the daemon inherits that gap. Re-run from a normal shell + or after fixing the shell; `doctor --fix` re-captures. Documented. +- **[Dynamic per-directory PATH managers not captured]** → out of scope; documented limitation. +- **[Backward compat: existing installs still carry inline `Environment=PATH=`]** → they keep + working; the rewritten doctor check flags them (no `EnvironmentFile=`), and the next + `daemon install` (or `doctor --fix` + restart) migrates them to the env-file shape and drops the + inline directive. + +## Migration Plan + +1. Ship the code. No config-schema change — the env file is not part of `netclaw.json`, so no + `netclaw-config.v1.schema.json` update and no `SchemaFixResolver` interaction. +2. Existing installed services are unaffected until acted on. On upgrade, the doctor check surfaces + a warning for the old inline-PATH shape. +3. Operator remediation: `netclaw daemon install` (re-run, idempotent — rewrites unit + writes env + file) **or** `netclaw doctor --fix`, then `systemctl --user restart netclaw`. +4. Rollback: revert the code. Old and new unit shapes both keep the service runnable; a stray + `config/daemon.env` left by a newer build is inert to older builds (they ignore it). + +## Open Questions + +- Env-file basename under `ConfigDirectory`: `daemon.env` proposed — confirm no collision with + existing config assets in that directory. +- Should the doctor check treat "install dir present but a *previously captured* dir now missing" + as stale? MVP: only warns when the install dir is absent or the file is missing/unwired, to avoid + false positives from legitimately changed PATHs. +- Should `daemon install` offer to restart the service for the operator? MVP: instruct only, to + keep install non-disruptive and consistent with `doctor --fix`. diff --git a/openspec/changes/systemd-daemon-path-capture/proposal.md b/openspec/changes/systemd-daemon-path-capture/proposal.md new file mode 100644 index 000000000..d4668a767 --- /dev/null +++ b/openspec/changes/systemd-daemon-path-capture/proposal.md @@ -0,0 +1,102 @@ +## Why + +When Netclaw is installed as a systemd `--user` service, the daemon starts with systemd's +sanitized, non-interactive environment — it does **not** inherit the operator's login-shell +`PATH`. To compensate, `netclaw daemon install` today bakes a **hardcoded** `Environment=PATH=` +list into the generated unit file (`installDir:~/.local/bin:/usr/local/bin:/usr/bin:/bin:...`). +That list can never anticipate a real operator's environment: the failure that surfaced this +(GitHub #1544) was `~/.dotnet/dotnet` being invisible to the agent's shell tool during a CI +restore, because `~/.dotnet` is not in the hardcoded list. Guessing at the operator's `PATH` is +structurally wrong — the operator's shell already knows the answer. This change captures the +operator's real `PATH` from the CLI process (which is a child of their shell) instead of +guessing, and keeps it fresh via `netclaw daemon install` and `netclaw doctor --fix`. + +Source PRD: **PRD-004** (CLI onboarding and config) — daemon install/doctor operator flows. + +## What Changes + +- **BREAKING (unit file shape):** `netclaw daemon install` no longer writes a hardcoded + `Environment=PATH=` directive into `~/.config/systemd/user/netclaw.service`. Existing installs + keep working until re-run, but the generated unit file shape changes on the next install. +- `netclaw daemon install` captures the operator's **real** `PATH` from the CLI's own inherited + environment (`Environment.GetEnvironmentVariable("PATH")`) — zero shell execution, no dotfile + sourcing — and writes it to a netclaw-owned environment file that the unit references via + `EnvironmentFile=`. The daemon only ever *reads* that file. +- The captured value prepends the daemon's own install directory (so the bundled `netclaw` CLI + always resolves) ahead of the operator's captured `PATH`. +- `netclaw doctor --fix` **rehydrates** the environment file from the current shell's `PATH` when + it is missing, unwired, or stale, then instructs the operator to + `systemctl --user restart netclaw`. The fix writes files only; it does **not** silently restart + the daemon. +- `SystemdUnitPathDoctorCheck` is rewritten: instead of parsing the unit's `Environment=PATH=` + line, it validates that the unit references the environment file (`EnvironmentFile=`) and that + the file exists and contains a `PATH` including the daemon's install directory. +- `netclaw daemon uninstall` removes the netclaw-owned environment file alongside the unit. +- Explicitly **rejected** (documented as a considered alternative in design): having the daemon + spawn a login shell (`bash -lc`) at startup to derive `PATH`. Running operator dotfiles in an + unsupervised background service can hang boot and is unpredictable — it violates the repo's + no-silent-fallback / fail-loud posture. + +### In scope (MVP) + +- Linux systemd `--user` service install/uninstall path. +- Capture-at-install and rehydrate-on-`doctor --fix`. +- Doctor check + fix, docs, and system-skill guidance updates. + +### Out of scope + +- Dynamic per-directory toolchain managers (`mise`, `asdf`, `direnv`) that inject `PATH` on `cd`. + A one-shot capture cannot represent a per-directory `PATH`; documented as a known limitation. +- Deriving `PATH` live at daemon startup (rejected — see above). +- macOS / Windows service install (already unsupported; unchanged). +- Manual `netclaw daemon start` (non-systemd): the daemon is a child of the operator's terminal + and already inherits the real `PATH`, so no change is needed there. + +## Capabilities + +### New Capabilities + +- `daemon-shell-path`: How the installed systemd `--user` daemon provisions the operator's `PATH` + for the agent's shell tool — captured (not guessed) from the CLI environment at install and + rehydrated by `doctor --fix`, delivered via a netclaw-owned `EnvironmentFile=`, and validated by + the systemd PATH doctor check. + +### Modified Capabilities + + + +## Impact + +- **Code** + - `src/Netclaw.Cli/Daemon/DaemonManager.cs` — `InstallAsync` (write `EnvironmentFile=`, capture + real `PATH`), `ComposeSystemdUnitPath` (repurpose to build the captured value with install-dir + prepend), `UninstallAsync` (delete the env file), install/upgrade messaging. + - `src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs` — validate `EnvironmentFile=` wiring + + env-file contents instead of the unit's inline PATH. + - `src/Netclaw.Cli/Doctor/DoctorFixService.cs` — add an env-file rehydration fix. Must run + **outside** the current `netclaw.json`-existence early-return, since the systemd env file is + independent of the app config file. +- **Cross-boundary contract** (producer → consumer): `DaemonManager` (producer) writes the env + file; `SystemdUnitPathDoctorCheck` (consumer) validates it. Both must agree on the file path, + the `EnvironmentFile=` directive, and that the install directory appears on `PATH`. Tests must + prove the produced file is exactly what the check accepts. +- **Tests** — `SystemdUnitPathDoctorCheckTests`, `SystemdUserServiceTests`, and `DaemonManager` + install/uninstall tests (unit content, env-file creation/removal, doctor-fix rehydration incl. + the no-config-file case). +- **Docs** — `docs/spec/SPEC-011-daemon-architecture.md`, `docs/prd/PRD-004-cli-onboarding-and-config.md`. +- **System skill** (System Skills Sync Rule) — `feeds/skills/.system/files/netclaw-operations/SKILL.md` + (daemon install / doctor guidance): how the shell tool gets its `PATH`, and the "re-run install + or `doctor --fix` after installing new tools" refresh loop. +- **Security / operational** + - Security: **no privilege change.** The shell command policy is `PATH`-independent — + `ShellCommandPolicy` is a deny-list matching the literal typed verb token, and + `ApprovalPatternMatching` keys on verb + directory scoping; neither resolves commands against + `$PATH`. Widening the daemon's `PATH` only affects bare-name *resolution*, which is downstream + of both gates. It cannot bypass a deny (deny fires on the token regardless of resolution) or + widen an allow (there is no resolved-path allow-list). This premise is why capturing the + operator's `PATH` is safe. + - Operational: env file changes require a service restart to take effect (systemd only re-reads + `EnvironmentFile=` on unit (re)start). Both install and `doctor --fix` surface the restart + instruction explicitly rather than restarting the daemon implicitly. diff --git a/openspec/changes/systemd-daemon-path-capture/specs/daemon-shell-path/spec.md b/openspec/changes/systemd-daemon-path-capture/specs/daemon-shell-path/spec.md new file mode 100644 index 000000000..0aadde865 --- /dev/null +++ b/openspec/changes/systemd-daemon-path-capture/specs/daemon-shell-path/spec.md @@ -0,0 +1,107 @@ +## ADDED Requirements + +### Requirement: Installed service PATH is captured from the operator environment, not guessed + +When installing the systemd `--user` service, `netclaw daemon install` SHALL provision the +daemon's `PATH` by capturing the operator's real `PATH` from the CLI process's own inherited +environment. It SHALL NOT write a hardcoded/guessed list of directories, and it SHALL NOT execute +a shell or source operator dotfiles to obtain the value. The captured `PATH` SHALL be delivered to +the daemon via a netclaw-owned environment file referenced by the unit's `EnvironmentFile=` +directive, and the generated unit SHALL NOT contain an inline `Environment=PATH=` directive. + +The provisioned `PATH` value SHALL place the daemon's own install directory first, then the +captured operator `PATH`, then a guaranteed system-directory floor +(`/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin`) so the daemon's shell remains functional even when +the installing shell's `PATH` was empty or partial. Entries SHALL be de-duplicated, and empty +`PATH` elements (which POSIX resolves to the current directory) SHALL be dropped. + +#### Scenario: Install captures the caller's PATH into the environment file + +- **WHEN** the operator runs `netclaw daemon install` from a shell whose `PATH` includes + `~/.dotnet` +- **THEN** the netclaw-owned environment file contains a `PATH=` line that includes `~/.dotnet` +- **AND** the file's `PATH` begins with the daemon's install directory +- **AND** no shell process was spawned to read the `PATH` + +#### Scenario: Generated unit wires the environment file and omits inline PATH + +- **WHEN** `netclaw daemon install` writes `~/.config/systemd/user/netclaw.service` +- **THEN** the unit contains an `EnvironmentFile=` directive pointing at the netclaw-owned + environment file +- **AND** the unit does NOT contain an `Environment=PATH=` directive + +### Requirement: `doctor --fix` rehydrates the daemon PATH environment file + +When the installed unit references the daemon PATH environment file, `netclaw doctor --fix` SHALL +rehydrate that file from the current shell's `PATH` when it is missing or does not include the +daemon's install directory. Rehydration SHALL run independently of whether the application config +file (`netclaw.json`) exists. The fix SHALL write files only — creating the parent directory if it +is absent — and SHALL surface an explicit instruction to run `systemctl --user restart netclaw`; it +SHALL NOT restart the daemon implicitly. `doctor --fix` SHALL NOT rewrite the systemd unit: a unit +that does not reference the environment file (a legacy inline-`PATH` unit) is a reinstall case, +routed by the doctor check. + +#### Scenario: Missing environment file is recreated by the fix + +- **WHEN** the systemd unit is installed but the daemon PATH environment file is absent +- **AND** the operator runs `netclaw doctor --fix` +- **THEN** the fix writes the environment file from the current shell's `PATH` +- **AND** the fix output instructs the operator to `systemctl --user restart netclaw` +- **AND** the fix does not restart the daemon + +#### Scenario: Rehydration runs even when the app config file is absent + +- **WHEN** `netclaw.json` does not exist +- **AND** the systemd unit is installed but its PATH environment file is stale or missing +- **AND** the operator runs `netclaw doctor --fix` +- **THEN** the environment-file rehydration fix is still evaluated and applied + +### Requirement: Systemd PATH doctor check validates the environment-file wiring + +`SystemdUnitPathDoctorCheck` SHALL validate that the installed unit references the daemon PATH +environment file via `EnvironmentFile=`, that the referenced file exists, and that the file's +`PATH` includes the daemon's install directory. When the referenced file is missing or omits the +install directory, remediation SHALL point the operator at `netclaw doctor --fix` followed by a +restart. A legacy unit that supplies its `PATH` inline (`Environment=PATH=`) SHALL pass when that +inline `PATH` includes the install directory (with a note to migrate via reinstall), and SHALL warn +with a reinstall remediation otherwise. The check SHALL pass silently when no service is installed +or on non-Linux platforms. + +#### Scenario: Wired, present, and install-dir on PATH passes + +- **WHEN** the unit references the environment file, the file exists, and its `PATH` includes the + install directory +- **THEN** the check passes + +#### Scenario: Functional legacy inline-PATH unit passes with a migration note + +- **WHEN** the installed unit has no `EnvironmentFile=` but supplies an inline `Environment=PATH=` + that includes the install directory +- **THEN** the check passes and notes that re-running `netclaw daemon install` migrates it + +#### Scenario: Unwired or broken legacy unit warns with reinstall remediation + +- **WHEN** the installed unit has no `EnvironmentFile=` and no inline `PATH` that includes the + install directory +- **THEN** the check returns a warning with remediation to reinstall and restart + +#### Scenario: Referenced environment file absent warns + +- **WHEN** the unit references the environment file but the file does not exist on disk +- **THEN** the check returns a warning with remediation to run `netclaw doctor --fix` and restart + +#### Scenario: No service installed skips + +- **WHEN** no netclaw systemd unit file exists +- **THEN** the check passes without warning + +### Requirement: Uninstall removes the daemon PATH environment file + +`netclaw daemon uninstall` SHALL remove the netclaw-owned daemon PATH environment file in addition +to the unit file, leaving no orphaned environment file behind. + +#### Scenario: Uninstall deletes the environment file + +- **WHEN** the operator runs `netclaw daemon uninstall` with an installed service and an existing + daemon PATH environment file +- **THEN** both the unit file and the daemon PATH environment file are deleted diff --git a/openspec/changes/systemd-daemon-path-capture/tasks.md b/openspec/changes/systemd-daemon-path-capture/tasks.md new file mode 100644 index 000000000..50cee065a --- /dev/null +++ b/openspec/changes/systemd-daemon-path-capture/tasks.md @@ -0,0 +1,46 @@ +## 1. Paths & environment-file model + +- [x] 1.1 Add `DaemonEnvironmentFilePath` to `NetclawPaths` under `ConfigDirectory` (e.g. `config/daemon.env`); confirm no collision with existing config assets. +- [x] 1.2 Add a small helper (in `DaemonManager` or alongside `NetclawPaths`) that renders the env-file content: a single `PATH=:` line, prepending the install dir. + +## 2. Install: capture instead of guess + +- [x] 2.1 In `DaemonManager.InstallAsync`, capture the operator `PATH` via `Environment.GetEnvironmentVariable("PATH")` (no shell spawn) and write the env file to `DaemonEnvironmentFilePath` (creating `ConfigDirectory`). +- [x] 2.2 Change the generated unit to reference the env file via `EnvironmentFile=` and REMOVE the inline `Environment=PATH=` directive. +- [x] 2.3 Repurpose/rename `ComposeSystemdUnitPath` to build the captured-PATH value (install dir + captured), or fold it into the render helper from 1.2; keep the install-dir-first ordering. +- [x] 2.4 Update install/upgrade messaging to mention the env file and the required `systemctl --user restart netclaw` after changes. + +## 3. Uninstall cleanup + +- [x] 3.1 In `DaemonManager.UninstallAsync`, delete `DaemonEnvironmentFilePath` alongside the unit file (idempotent if absent). + +## 4. Doctor check rewrite (consumer side of the contract) + +- [x] 4.1 Rewrite `SystemdUnitPathDoctorCheck` to: read the unit, find `EnvironmentFile=`, confirm the referenced file exists, and confirm its `PATH` includes the install dir (still derived from `ExecStart`). +- [x] 4.2 Warnings point remediation at `netclaw doctor --fix` (or reinstall) + `systemctl --user restart netclaw`; keep the not-installed / non-Linux pass-through behavior. + +## 5. Doctor --fix rehydration + +- [x] 5.1 In `DoctorFixService`, add env-file rehydration that runs OUTSIDE the `netclaw.json`-existence early-return (independent of app config). +- [x] 5.2 Emit a `DoctorFileFix` for the env file only when it is missing, unwired, or missing the install dir; capture the current shell `PATH`. Reuse `ApplyAsync` (file write only — no daemon restart). +- [x] 5.3 Ensure the fix plan/description surfaces the `systemctl --user restart netclaw` instruction to the operator. + +## 6. Tests + +- [x] 6.1 Install-content test: `BuildDaemonUnitContent` has `EnvironmentFile=-` and NO `Environment=PATH=`; `DaemonPathEnvironmentFile.Render` puts install dir first. (Full `InstallAsync` not driven — it runs real `systemctl`/`loginctl` against the live service; the pure builders are exactly what it writes.) +- [x] 6.2 Uninstall env-file removal extracted to `DaemonManager.RemoveDaemonEnvironmentFile()` and unit-tested (deletes the file + idempotent). The seam exists because full `UninstallAsync` runs real `systemctl stop/disable netclaw.service` and would mutate the developer's own service. +- [x] 6.3 Rewrite `SystemdUnitPathDoctorCheckTests`: pass (wired + present + install dir), warn (no `EnvironmentFile=` → reinstall), warn (file absent → doctor --fix), warn (install dir missing from PATH), warn (malformed ExecStart), skip (no unit), skip (non-Linux). +- [x] 6.4 `DoctorFixService` tests: rehydrates when env file missing/stale, INCLUDING when `netclaw.json` is absent; no-op when healthy; no-op when unit legacy/unwired; surfaces the restart instruction; applies to disk. +- [x] 6.5 Producer→consumer contract test: the exact env file + unit the installer builders produce are accepted by `SystemdUnitPathDoctorCheck` (Pass) and reported healthy by the doctor-fix path (no fix). + +## 7. Docs & system skill + +- [x] 7.1 Update `docs/spec/SPEC-011-daemon-architecture.md` (env-file model; remove hardcoded-PATH description). +- [x] 7.2 Update `docs/prd/PRD-004-cli-onboarding-and-config.md` install/doctor operator flow. +- [x] 7.3 Update `feeds/skills/.system/files/netclaw-operations/SKILL.md`: how the shell tool gets its `PATH`, the "re-run install or `doctor --fix` + restart after installing new tools" loop, and the mise/asdf/direnv limitation. Bump `metadata.version`. + +## 8. Quality gates + +- [x] 8.1 `dotnet slopwatch analyze` — 0 issues found (verified). +- [x] 8.2 `./scripts/Add-FileHeaders.ps1 -Verify` — all files have headers (verified). +- [x] 8.3 Eval suite N/A: change is a diagnostics-table addition + skill version bump, not skill-matching/tool/identity/memory/system-prompt logic (the categories the suite guards); the live-model suite needs a configured model target. No eval-case triggers apply. diff --git a/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs b/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs new file mode 100644 index 000000000..db130cdc5 --- /dev/null +++ b/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs @@ -0,0 +1,130 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Netclaw.Cli.Daemon; +using Netclaw.Configuration; +using Xunit; + +namespace Netclaw.Cli.Tests.Daemon; + +/// +/// Unit tests for the daemon PATH provisioning contract shared by install +/// (producer), doctor --fix (rehydrator), and the systemd PATH doctor check +/// (validator). InstallAsync/UninstallAsync themselves are not +/// driven here: they invoke real systemctl/loginctl against the +/// live netclaw.service and the real ~/.config unit path, so +/// exercising them in-process would mutate the developer's own service. The +/// pure builders below are exactly the content those methods write and read. +/// +public sealed class DaemonPathEnvironmentFileTests +{ + [Fact] + public void ComposePathValue_InstallDirFirst_ThenCapture_ThenDedupedFloor() + => Assert.Equal( + "/opt/netclaw:/home/u/.dotnet:/usr/bin:/usr/local/bin:/bin:/usr/sbin:/sbin", + DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", "/home/u/.dotnet:/usr/bin")); + + [Fact] + public void ComposePathValue_EmptyCapture_StillHasFunctionalFloor() + { + // An empty PATH on the installing shell must NOT leave the daemon with installDir + // alone — the system floor is always guaranteed so /bin/sh etc. still resolve. + const string expected = "/opt/netclaw:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin"; + Assert.Equal(expected, DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", null)); + Assert.Equal(expected, DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", "")); + } + + [Fact] + public void ComposePathValue_DropsEmptyElements() + { + // POSIX treats an empty PATH element as the current directory — an exec-hijack + // vector for a daemon running `bash -c` in an agent-controlled workspace. + var value = DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", "/a::/b:"); + + Assert.DoesNotContain("::", value, StringComparison.Ordinal); + Assert.All(value.Split(':'), Assert.NotEmpty); + Assert.StartsWith("/opt/netclaw:/a:/b:", value, StringComparison.Ordinal); + } + + [Fact] + public void Render_ThenReadPathValue_RoundTrips() + { + var content = DaemonPathEnvironmentFile.Render("/opt/netclaw", "/usr/bin"); + + Assert.Equal("PATH=/opt/netclaw:/usr/bin:/usr/local/bin:/bin:/usr/sbin:/sbin\n", content); + Assert.Equal( + "/opt/netclaw:/usr/bin:/usr/local/bin:/bin:/usr/sbin:/sbin", + DaemonPathEnvironmentFile.ReadPathValue(content)); + } + + [Fact] + public void ReadPathValue_NoPathAssignment_ReturnsNull() + => Assert.Null(DaemonPathEnvironmentFile.ReadPathValue("FOO=bar\nBAZ=qux\n")); + + [Theory] + [InlineData("/opt/netclaw:/usr/bin", "/opt/netclaw", true)] + [InlineData("/usr/local/bin:/usr/bin", "/opt/netclaw", false)] + public void PathContainsDirectory_MatchesExactEntry(string pathValue, string dir, bool expected) + => Assert.Equal(expected, DaemonPathEnvironmentFile.PathContainsDirectory(pathValue, dir)); + + [Fact] + public void TryGetInstallDir_FromExecStart_StripsBinaryAndArgs() + { + var lines = new[] { "[Service]", "ExecStart=/opt/netclaw/netclawd --foreground" }; + + Assert.True(DaemonPathEnvironmentFile.TryGetInstallDir(lines, out var dir)); + Assert.Equal("/opt/netclaw", dir); + } + + [Fact] + public void TryGetInstallDir_NoExecStart_ReturnsFalse() + => Assert.False(DaemonPathEnvironmentFile.TryGetInstallDir(new[] { "[Service]" }, out _)); + + [Fact] + public void TryGetEnvironmentFilePath_StripsTolerantDashPrefix() + { + var lines = new[] { "EnvironmentFile=-/home/u/.netclaw/config/daemon.env" }; + + Assert.True(DaemonPathEnvironmentFile.TryGetEnvironmentFilePath(lines, out var path)); + Assert.Equal("/home/u/.netclaw/config/daemon.env", path); + } + + [Fact] + public void TryGetEnvironmentFilePath_Absent_ReturnsFalse() + => Assert.False(DaemonPathEnvironmentFile.TryGetEnvironmentFilePath( + new[] { "ExecStart=/opt/netclaw/netclawd" }, out _)); + + [Fact] + public void BuildDaemonUnitContent_WiresEnvironmentFile_AndOmitsInlinePath() + { + var unit = DaemonManager.BuildDaemonUnitContent( + "/opt/netclaw/netclawd", + "/opt/netclaw/netclaw", + "/home/u/.netclaw/config/daemon.env"); + + Assert.Contains("EnvironmentFile=-/home/u/.netclaw/config/daemon.env", unit, StringComparison.Ordinal); + Assert.DoesNotContain("Environment=PATH=", unit, StringComparison.Ordinal); + Assert.Contains("ExecStart=/opt/netclaw/netclawd", unit, StringComparison.Ordinal); + Assert.Contains("ExecStop=/opt/netclaw/netclaw daemon stop", unit, StringComparison.Ordinal); + } + + [Fact] + public void RemoveDaemonEnvironmentFile_DeletesFile_AndIsIdempotent() + { + // Covers the uninstall env-file-removal contract without driving the + // systemctl-coupled UninstallAsync. + var basePath = Path.Combine(Path.GetTempPath(), "netclaw-tests", Guid.NewGuid().ToString("N")); + var paths = new NetclawPaths(basePath); + paths.EnsureDirectoriesExist(); + File.WriteAllText(paths.DaemonEnvironmentFilePath, "PATH=/opt/netclaw:/usr/bin\n"); + var manager = new DaemonManager(paths, TimeProvider.System); + + manager.RemoveDaemonEnvironmentFile(); + Assert.False(File.Exists(paths.DaemonEnvironmentFilePath)); + + // Idempotent — a second call on an already-removed file must not throw. + manager.RemoveDaemonEnvironmentFile(); + } +} diff --git a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs index 3bfc433db..f673b5443 100644 --- a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs @@ -1,8 +1,9 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Netclaw.Cli.Daemon; using Netclaw.Cli.Doctor; using Netclaw.Configuration; using Xunit; @@ -11,13 +12,17 @@ namespace Netclaw.Cli.Tests.Doctor; public sealed class DoctorFixServiceTests { + // POSIX install dir: systemd units are always POSIX-style regardless of the host OS + // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. + private const string InstallDir = "/opt/netclaw"; + + // ── Config-file fixes (systemd PATH rehydration disabled so these stay hermetic + // on machines where netclaw is actually installed as a --user service) ── + [Fact] public async Task PlansConfigVersionFix_WhenMissing() { - var basePath = CreateTempBasePath(); - var paths = new NetclawPaths(basePath); - paths.EnsureDirectoriesExist(); - + var paths = NewPaths(); await File.WriteAllTextAsync(paths.NetclawConfigPath, """ { @@ -27,7 +32,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, } """, TestContext.Current.CancellationToken); - var service = new DoctorFixService(paths); + var service = ConfigOnlyService(paths); var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); Assert.True(plan.HasChanges); @@ -38,10 +43,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, [Fact] public async Task AppliesFixPlanToDisk() { - var basePath = CreateTempBasePath(); - var paths = new NetclawPaths(basePath); - paths.EnsureDirectoriesExist(); - + var paths = NewPaths(); await File.WriteAllTextAsync(paths.NetclawConfigPath, """ { @@ -51,7 +53,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, } """, TestContext.Current.CancellationToken); - var service = new DoctorFixService(paths); + var service = ConfigOnlyService(paths); var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); await service.ApplyAsync(plan, TestContext.Current.CancellationToken); @@ -63,10 +65,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, [Fact] public async Task AddsSlackFormat_WhenSlackWebhookMissingFormat() { - var basePath = CreateTempBasePath(); - var paths = new NetclawPaths(basePath); - paths.EnsureDirectoriesExist(); - + var paths = NewPaths(); await File.WriteAllTextAsync(paths.NetclawConfigPath, """ { @@ -81,7 +80,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, } """, TestContext.Current.CancellationToken); - var service = new DoctorFixService(paths); + var service = ConfigOnlyService(paths); var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); Assert.True(plan.HasChanges); @@ -92,11 +91,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, [Fact] public async Task RemovesStalePropertyViaSchemaFix() { - var basePath = CreateTempBasePath(); - var paths = new NetclawPaths(basePath); - paths.EnsureDirectoriesExist(); - - // Config with a stale property that the schema no longer defines + var paths = NewPaths(); await File.WriteAllTextAsync(paths.NetclawConfigPath, """ { @@ -112,14 +107,12 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, } """, TestContext.Current.CancellationToken); - var service = new DoctorFixService(paths); + var service = ConfigOnlyService(paths); var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); Assert.True(plan.HasChanges); Assert.Single(plan.Fixes); - // CapabilityClass was removed from schema — should be cleaned up Assert.DoesNotContain("CapabilityClass", plan.Fixes[0].UpdatedText, StringComparison.Ordinal); - // Other properties should be preserved Assert.Contains("memorizer", plan.Fixes[0].UpdatedText, StringComparison.Ordinal); Assert.Contains("stdio", plan.Fixes[0].UpdatedText, StringComparison.Ordinal); } @@ -127,10 +120,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, [Fact] public async Task DynamicDescriptionReflectsAppliedFixes() { - var basePath = CreateTempBasePath(); - var paths = new NetclawPaths(basePath); - paths.EnsureDirectoriesExist(); - + var paths = NewPaths(); await File.WriteAllTextAsync(paths.NetclawConfigPath, """ { @@ -140,15 +130,151 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, } """, TestContext.Current.CancellationToken); - var service = new DoctorFixService(paths); + var service = ConfigOnlyService(paths); var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); Assert.True(plan.HasChanges); - // Description should mention what was actually fixed Assert.Contains("configVersion", plan.Fixes[0].Description, StringComparison.Ordinal); Assert.Contains("Slack ACL defaults", plan.Fixes[0].Description, StringComparison.Ordinal); } + // ── Daemon shell-tool PATH rehydration ── + + [Fact] + public async Task RehydratesEnvFile_WhenMissing_EvenWithoutNetclawJson() + { + var paths = NewPaths(); + var unitPath = WriteWiredUnit(paths); + // No netclaw.json and no env file on disk. + + var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + + var fix = Assert.Single(plan.Fixes); + Assert.Equal(paths.DaemonEnvironmentFilePath, fix.FilePath); + Assert.StartsWith($"PATH={InstallDir}:", fix.UpdatedText, StringComparison.Ordinal); + Assert.Contains("systemctl --user restart netclaw", fix.Description, StringComparison.Ordinal); + } + + [Fact] + public async Task RehydratesEnvFile_WhenStale_MissingInstallDir() + { + var paths = NewPaths(); + var unitPath = WriteWiredUnit(paths); + await File.WriteAllTextAsync(paths.DaemonEnvironmentFilePath, "PATH=/usr/bin\n", + TestContext.Current.CancellationToken); + + var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + + var fix = Assert.Single(plan.Fixes, f => f.FilePath == paths.DaemonEnvironmentFilePath); + Assert.Equal("PATH=/usr/bin\n", fix.OriginalText); + Assert.Contains(InstallDir, fix.UpdatedText, StringComparison.Ordinal); + } + + [Fact] + public async Task NoEnvFix_WhenHealthy() + { + var paths = NewPaths(); + var unitPath = WriteWiredUnit(paths); + await File.WriteAllTextAsync( + paths.DaemonEnvironmentFilePath, + DaemonPathEnvironmentFile.Render(InstallDir, "/usr/bin"), + TestContext.Current.CancellationToken); + + var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + + Assert.DoesNotContain(plan.Fixes, f => f.FilePath == paths.DaemonEnvironmentFilePath); + } + + [Fact] + public async Task NoEnvFix_WhenUnitIsLegacyUnwired() + { + // Legacy unit (inline PATH, no EnvironmentFile=) is routed to reinstall by the + // doctor check, not rehydrated here — doctor --fix does not rewrite systemd units. + var paths = NewPaths(); + var unitPath = WriteRawUnit( + $"[Service]\nExecStart={InstallDir}/netclawd\nEnvironment=PATH=/opt/x:/usr/bin\n"); + + var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + + Assert.DoesNotContain(plan.Fixes, f => f.FilePath == paths.DaemonEnvironmentFilePath); + } + + [Fact] + public async Task AppliesEnvFileRehydrationToDisk() + { + var paths = NewPaths(); + var unitPath = WriteWiredUnit(paths); + + var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + await service.ApplyAsync(plan, TestContext.Current.CancellationToken); + + Assert.True(File.Exists(paths.DaemonEnvironmentFilePath)); + var content = await File.ReadAllTextAsync(paths.DaemonEnvironmentFilePath, TestContext.Current.CancellationToken); + Assert.Contains(InstallDir, content, StringComparison.Ordinal); + } + + [Fact] + public async Task AppliesRehydration_WhenConfigDirectoryWasRemoved() + { + // Operator wiped ~/.netclaw/config but left the installed service. ApplyAsync must + // recreate the parent dir instead of throwing DirectoryNotFoundException and aborting. + var paths = NewPaths(); + var unitPath = WriteWiredUnit(paths); + Directory.Delete(Path.GetDirectoryName(paths.DaemonEnvironmentFilePath)!, recursive: true); + + var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + await service.ApplyAsync(plan, TestContext.Current.CancellationToken); + + Assert.True(File.Exists(paths.DaemonEnvironmentFilePath)); + } + + [Fact] + public async Task DoesNotThrow_WhenUnitEnvironmentFilePathIsMalformed() + { + // A hand-edited unit with an invalid EnvironmentFile= value must not crash the whole + // doctor --fix run via Path.GetFullPath. + var paths = NewPaths(); + var unitPath = WriteRawUnit("[Service]\nExecStart=/opt/netclaw/netclawd\nEnvironmentFile=-/bad\0path\n"); + + var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + + Assert.DoesNotContain(plan.Fixes, f => f.FilePath == paths.DaemonEnvironmentFilePath); + } + + private static NetclawPaths NewPaths() + { + var paths = new NetclawPaths(CreateTempBasePath()); + paths.EnsureDirectoriesExist(); + return paths; + } + + private static DoctorFixService ConfigOnlyService(NetclawPaths paths) + => new(paths, Path.Combine(paths.BasePath, "unused.service"), systemdEnabled: false); + + private static string WriteWiredUnit(NetclawPaths paths) + // Forward-slash concatenation (NOT Path.Combine): systemd ExecStart is POSIX even + // when the test runs on Windows, matching what TryGetInstallDir parses. + => WriteRawUnit(DaemonManager.BuildDaemonUnitContent( + $"{InstallDir}/netclawd", + $"{InstallDir}/netclaw", + paths.DaemonEnvironmentFilePath)); + + private static string WriteRawUnit(string content) + { + var dir = Path.Combine(Path.GetTempPath(), "netclaw-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + var unitPath = Path.Combine(dir, "netclaw.service"); + File.WriteAllText(unitPath, content); + return unitPath; + } + private static string CreateTempBasePath() { var path = Path.Combine(Path.GetTempPath(), "netclaw-tests", Guid.NewGuid().ToString("N")); diff --git a/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs b/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs index 380acdb4b..75aaf0bb8 100644 --- a/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Netclaw.Cli.Daemon; using Netclaw.Cli.Doctor; using Xunit; @@ -13,7 +14,8 @@ public sealed class SystemdUnitPathDoctorCheckTests [Fact] public async Task ReturnsPass_WhenPlatformDisabled() { - var unitPath = WriteUnit("[Service]\nExecStart=/opt/netclaw/netclawd\n"); + var (unitPath, _) = WriteUnitDir(); + File.WriteAllText(unitPath, "[Service]\nExecStart=/opt/netclaw/netclawd\n"); var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: false); var result = await check.RunAsync(TestContext.Current.CancellationToken); @@ -35,83 +37,124 @@ public async Task ReturnsPass_WhenUnitFileDoesNotExist() } [Fact] - public async Task ReturnsWarning_WhenPathDirectiveMissing() + public async Task ReturnsWarning_WhenExecStartMissing() { - var unitPath = WriteUnit(""" - [Unit] - Description=Netclaw Daemon + var (unitPath, _) = WriteUnitDir(); + File.WriteAllText(unitPath, "[Service]\nType=simple\nEnvironmentFile=-/tmp/daemon.env\n"); + var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: true); + var result = await check.RunAsync(TestContext.Current.CancellationToken); + + Assert.Equal(DoctorSeverity.Warning, result.Severity); + Assert.Contains("Could not determine the daemon install directory", result.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task ReturnsPass_WhenLegacyInlinePathIncludesInstallDir() + { + // A functional legacy (pre-#1544) unit: inline Environment=PATH= that resolves the + // install dir (e.g. after an in-place binary upgrade without reinstall). Must NOT + // false-alarm; pass with a migration note. + var (unitPath, _) = WriteUnitDir(); + File.WriteAllText(unitPath, """ [Service] - Type=simple ExecStart=/opt/netclaw/netclawd - Environment=DOTNET_ENVIRONMENT=Production + Environment=PATH=/opt/netclaw:/usr/bin """); var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: true); var result = await check.RunAsync(TestContext.Current.CancellationToken); - Assert.Equal(DoctorSeverity.Warning, result.Severity); - Assert.Contains("does not set PATH", result.Message, StringComparison.Ordinal); - Assert.Contains("daemon uninstall", result.Remediation!, StringComparison.Ordinal); + Assert.Equal(DoctorSeverity.Pass, result.Severity); + Assert.Contains("Legacy unit", result.Message, StringComparison.Ordinal); + Assert.Contains("daemon install", result.Message, StringComparison.Ordinal); } [Fact] - public async Task ReturnsWarning_WhenPathMissingInstallDir() + public async Task ReturnsWarning_WhenLegacyUnitLacksUsablePath() { - var unitPath = WriteUnit(""" + // Legacy unit whose inline PATH does NOT include the install dir → genuinely + // broken, route to reinstall. + var (unitPath, _) = WriteUnitDir(); + File.WriteAllText(unitPath, """ [Service] ExecStart=/opt/netclaw/netclawd - Environment=PATH=/usr/local/bin:/usr/bin:/bin + Environment=PATH=/usr/bin:/bin """); var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: true); var result = await check.RunAsync(TestContext.Current.CancellationToken); Assert.Equal(DoctorSeverity.Warning, result.Severity); - Assert.Contains("does not include the daemon's install directory", result.Message, StringComparison.Ordinal); - Assert.Contains("/opt/netclaw", result.Message, StringComparison.Ordinal); + Assert.Contains("does not supply the daemon's shell-tool PATH", result.Message, StringComparison.Ordinal); + Assert.Contains("daemon install", result.Remediation!, StringComparison.Ordinal); } [Fact] - public async Task ReturnsPass_WhenPathContainsInstallDir() + public async Task ReturnsWarning_WhenEnvironmentFileMissingOnDisk() { - var unitPath = WriteUnit(""" - [Service] - ExecStart=/home/user/.local/bin/netclawd - Environment=PATH=/home/user/.local/bin:/usr/local/bin:/usr/bin:/bin - """); + var (unitPath, dir) = WriteUnitDir(); + var envPath = Path.Combine(dir, "daemon.env"); // referenced but never written + File.WriteAllText(unitPath, DaemonManager.BuildDaemonUnitContent( + "/opt/netclaw/netclawd", "/opt/netclaw/netclaw", envPath)); var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: true); var result = await check.RunAsync(TestContext.Current.CancellationToken); - Assert.Equal(DoctorSeverity.Pass, result.Severity); - Assert.Contains("/home/user/.local/bin", result.Message, StringComparison.Ordinal); + Assert.Equal(DoctorSeverity.Warning, result.Severity); + Assert.Contains("is missing", result.Message, StringComparison.Ordinal); + Assert.Contains("doctor --fix", result.Remediation!, StringComparison.Ordinal); } [Fact] - public async Task ReturnsWarning_WhenExecStartMissing() + public async Task ReturnsWarning_WhenEnvPathMissingInstallDir() { - var unitPath = WriteUnit(""" - [Service] - Type=simple - Environment=PATH=/usr/local/bin:/usr/bin:/bin - """); + var (unitPath, dir) = WriteUnitDir(); + var envPath = Path.Combine(dir, "daemon.env"); + File.WriteAllText(envPath, "PATH=/usr/local/bin:/usr/bin\n"); // no /opt/netclaw + File.WriteAllText(unitPath, DaemonManager.BuildDaemonUnitContent( + "/opt/netclaw/netclawd", "/opt/netclaw/netclaw", envPath)); var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: true); var result = await check.RunAsync(TestContext.Current.CancellationToken); Assert.Equal(DoctorSeverity.Warning, result.Severity); - Assert.Contains("missing ExecStart", result.Message, StringComparison.Ordinal); + Assert.Contains("does not include the daemon's install directory", result.Message, StringComparison.Ordinal); + Assert.Contains("/opt/netclaw", result.Message, StringComparison.Ordinal); + Assert.Contains("doctor --fix", result.Remediation!, StringComparison.Ordinal); + } + + [Fact] + public async Task ReturnsPass_WhenWiredAndInstallDirPresent() + { + // Producer→consumer contract: the exact artifacts install writes + // (env file via Render, unit via BuildDaemonUnitContent) are accepted by the check. + var (unitPath, dir) = WriteUnitDir(); + const string installDir = "/home/user/.local/bin"; + var envPath = Path.Combine(dir, "daemon.env"); + File.WriteAllText(envPath, DaemonPathEnvironmentFile.Render(installDir, "/usr/local/bin:/usr/bin")); + File.WriteAllText(unitPath, DaemonManager.BuildDaemonUnitContent( + $"{installDir}/netclawd", $"{installDir}/netclaw", envPath)); + var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: true); + + var result = await check.RunAsync(TestContext.Current.CancellationToken); + + Assert.Equal(DoctorSeverity.Pass, result.Severity); + Assert.Contains(installDir, result.Message, StringComparison.Ordinal); } [Fact] public async Task ParsesExecStart_StrippingArguments() { - // ExecStart with arguments — install directory is the binary's parent. - var unitPath = WriteUnit(""" + var (unitPath, dir) = WriteUnitDir(); + const string installDir = "/opt/netclaw"; + var envPath = Path.Combine(dir, "daemon.env"); + File.WriteAllText(envPath, DaemonPathEnvironmentFile.Render(installDir, "/usr/bin")); + // ExecStart carries an argument — install dir is still the binary's parent. + File.WriteAllText(unitPath, $""" [Service] - ExecStart=/opt/netclaw/netclawd --foreground - Environment=PATH=/opt/netclaw:/usr/bin + ExecStart={installDir}/netclawd --foreground + EnvironmentFile=-{envPath} """); var check = new SystemdUnitPathDoctorCheck(unitPath, enabledOnThisPlatform: true); @@ -120,12 +163,10 @@ public async Task ParsesExecStart_StrippingArguments() Assert.Equal(DoctorSeverity.Pass, result.Severity); } - private static string WriteUnit(string content) + private static (string unitPath, string dir) WriteUnitDir() { var dir = Path.Combine(Path.GetTempPath(), "netclaw-tests", Guid.NewGuid().ToString("N")); Directory.CreateDirectory(dir); - var path = Path.Combine(dir, "netclaw.service"); - File.WriteAllText(path, content); - return path; + return (Path.Combine(dir, "netclaw.service"), dir); } } diff --git a/src/Netclaw.Cli/Daemon/DaemonManager.cs b/src/Netclaw.Cli/Daemon/DaemonManager.cs index e9c7ed2db..57ddaabba 100644 --- a/src/Netclaw.Cli/Daemon/DaemonManager.cs +++ b/src/Netclaw.Cli/Daemon/DaemonManager.cs @@ -274,40 +274,30 @@ public async Task InstallAsync() "Cannot find netclawd binary. Set NETCLAW_DAEMON_PATH or ensure it is " + "in the same directory as the CLI."); - var userHome = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); Directory.CreateDirectory(SystemdUserUnitDirectory); // CLI binary is in the same directory as the daemon binary var installDir = Path.GetDirectoryName(binaryPath)!; var cliBinaryPath = Path.Combine(installDir, "netclaw"); - // systemd --user services start with a sanitized PATH that does not include - // installDir or ~/.local/bin, so the agent's shell tool cannot resolve - // `netclaw` (or other user-installed binaries) when invoked from the daemon. - // We compose PATH explicitly: installDir first (so the daemon's bundled CLI - // wins), then ~/.local/bin (common user-bin location), then the systemd - // default. Keep this in sync with SystemdUnitPathDoctorCheck. - var unitPathEnv = ComposeSystemdUnitPath(installDir, userHome); + // A systemd --user service starts with a sanitized PATH that excludes installDir, + // ~/.dotnet, ~/.local/bin, and everything else the operator has on their shell + // PATH, so the agent's shell tool cannot resolve `netclaw`, `dotnet`, etc. Rather + // than guess a directory list (which can never anticipate every environment — + // #1544, where ~/.dotnet was invisible), capture the operator's REAL PATH from + // this CLI process — a child of the operator's shell, so it already holds the live + // PATH with no shell spawned — and hand it to the daemon via a netclaw-owned + // EnvironmentFile. The daemon only ever reads that file; `doctor --fix` rehydrates + // it and SystemdUnitPathDoctorCheck validates it. All three go through + // DaemonPathEnvironmentFile so the contract stays in lockstep. + var envFilePath = _paths.DaemonEnvironmentFilePath; + var capturedPath = DaemonPathEnvironmentFile.CaptureCurrentPath(); + Directory.CreateDirectory(Path.GetDirectoryName(envFilePath)!); + await File.WriteAllTextAsync(envFilePath, DaemonPathEnvironmentFile.Render(installDir, capturedPath)); var unitPath = SystemdUserUnitFilePath; var isUpgrade = File.Exists(unitPath); - var unitContent = $""" - [Unit] - Description=Netclaw Daemon - After=network.target - - [Service] - Type=simple - ExecStart={binaryPath} - ExecStop={cliBinaryPath} daemon stop - Restart=always - RestartSec=5 - Environment=DOTNET_ENVIRONMENT=Production - Environment=PATH={unitPathEnv} - - [Install] - WantedBy=default.target - """; + var unitContent = BuildDaemonUnitContent(binaryPath, cliBinaryPath, envFilePath); await File.WriteAllTextAsync(unitPath, unitContent); @@ -327,8 +317,8 @@ public async Task InstallAsync() var startMessage = $"Service installed at {unitPath}. Start with: systemctl --user start netclaw"; if (isUpgrade) { - startMessage += "\nUnit file refreshed (PATH for the daemon's shell tool) — " + - "restart the service to pick up the change."; + startMessage += $"\nUnit refreshed and shell-tool PATH captured to {envFilePath} — " + + "restart to pick up the change: systemctl --user restart netclaw."; } return new DaemonResult(true, startMessage); @@ -346,30 +336,47 @@ public async Task InstallAsync() internal static string SystemdUserUnitFilePath => Path.Combine(SystemdUserUnitDirectory, "netclaw.service"); /// - /// Builds the PATH value baked into the systemd user unit so the daemon's - /// shell tool can resolve user-installed binaries like netclaw. + /// Deletes the netclaw-owned PATH env file written at install so uninstall leaves no + /// orphan behind. Idempotent. Separated from (which is + /// coupled to real systemctl/loginctl and cannot be driven in-process + /// without mutating the developer's live service) so the deletion contract is + /// unit-testable on its own. /// - /// - /// systemd --user services start with a minimal default PATH that does - /// not include ~/.local/bin or any custom install directory, so we - /// compose one explicitly. The doctor check - /// SystemdUnitPathDoctorCheck validates that an existing unit file - /// contains on PATH; keep both call sites in - /// agreement. - /// - internal static string ComposeSystemdUnitPath(string installDir, string userHome) + internal void RemoveDaemonEnvironmentFile() { - var localBin = Path.Combine(userHome, ".local", "bin"); - return string.Join(':', - installDir, - localBin, - "/usr/local/bin", - "/usr/bin", - "/bin", - "/usr/sbin", - "/sbin"); + var envFilePath = _paths.DaemonEnvironmentFilePath; + if (File.Exists(envFilePath)) + File.Delete(envFilePath); } + /// + /// Builds the systemd --user unit content. The daemon's shell-tool PATH is + /// supplied out-of-band via EnvironmentFile= (see + /// ) rather than an inline + /// Environment=PATH=, so it can be captured from the operator's real + /// environment and rehydrated by doctor --fix without rewriting the unit. + /// The - prefix makes systemd tolerant of a missing env file: a deleted PATH + /// file degrades tool resolution (which SystemdUnitPathDoctorCheck flags) + /// rather than preventing the entire daemon from starting. + /// + internal static string BuildDaemonUnitContent(string binaryPath, string cliBinaryPath, string environmentFilePath) => $""" + [Unit] + Description=Netclaw Daemon + After=network.target + + [Service] + Type=simple + ExecStart={binaryPath} + ExecStop={cliBinaryPath} daemon stop + Restart=always + RestartSec=5 + Environment=DOTNET_ENVIRONMENT=Production + EnvironmentFile=-{environmentFilePath} + + [Install] + WantedBy=default.target + """; + /// /// Uninstalls the systemd user service (Linux only). /// @@ -391,6 +398,8 @@ public async Task UninstallAsync() if (File.Exists(unitPath)) File.Delete(unitPath); + RemoveDaemonEnvironmentFile(); + await RunCommandAsync("systemctl", "--user daemon-reload"); return new DaemonResult(true, "Service uninstalled."); diff --git a/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs b/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs new file mode 100644 index 000000000..a8a6dadd6 --- /dev/null +++ b/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs @@ -0,0 +1,215 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +namespace Netclaw.Cli.Daemon; + +/// +/// The single contract for the netclaw-owned systemd EnvironmentFile= that +/// supplies the installed daemon's shell-tool PATH. The producer +/// ( install), the rehydrator (DoctorFixService), +/// and the validator (SystemdUnitPathDoctorCheck) all go through this type so +/// the file's format, the operator-PATH capture, and the unit-parsing rules stay in +/// lockstep. See the repo's Cross-Boundary Contract Rule. +/// +/// +/// A systemd --user service starts with a sanitized, non-interactive +/// environment and does NOT inherit the operator's login-shell PATH. Rather +/// than guess a directory list (the failure behind #1544, where ~/.dotnet was +/// invisible), install captures the operator's real PATH from the CLI process +/// — a child of the operator's shell — with zero shell execution / dotfile sourcing, +/// and hands it to the daemon via this file. +/// +internal static class DaemonPathEnvironmentFile +{ + internal const string PathAssignmentPrefix = "PATH="; + internal const string ExecStartPrefix = "ExecStart="; + internal const string EnvironmentFilePrefix = "EnvironmentFile="; + internal const string InlinePathPrefix = "Environment=PATH="; + + /// + /// The set of directories that must always be resolvable for the daemon's shell tool + /// to function at all (a POSIX shell, coreutils, and admin sbin tools). This is + /// NOT a guess at the operator's tools — the captured operator PATH supplies those — it + /// is a functional floor guaranteed regardless of what the installing shell's PATH + /// happened to contain, so an empty or partial capture can never leave the daemon + /// unable to resolve /bin/sh, ip, etc. Mirrors the guarantee the old + /// unit-baked PATH made unconditionally. + /// + private static readonly string[] SystemPathFloor = + ["/usr/local/bin", "/usr/bin", "/bin", "/usr/sbin", "/sbin"]; + + /// + /// Reads the operator's real PATH from the current process environment. + /// The netclaw CLI is a child of the operator's interactive shell, so this is + /// the operator's live PATH — no shell spawned, no dotfiles sourced. + /// + internal static string? CaptureCurrentPath() => Environment.GetEnvironmentVariable("PATH"); + + /// + /// Composes the PATH value written to the environment file: + /// + /// the daemon's own install directory first (bundled netclaw CLI wins); + /// then the captured operator PATH (their real tool dirs — the point of #1544); + /// then , a guaranteed functional baseline. + /// + /// Entries are de-duplicated (order-preserving, ordinal) and empty elements are + /// dropped — a POSIX empty PATH entry (from :: or a leading/trailing + /// :, common when a dotfile does PATH="$PATH:") means "current directory", + /// which would let a binary planted in an agent-controlled workspace shadow a system + /// command when the daemon runs bash -c. Separator is the POSIX ':' + /// (systemd is Linux-only). Because the floor is always appended, an empty/unset + /// captured PATH still yields a fully functional PATH rather than installDir alone. + /// + internal static string ComposePathValue(string installDir, string? capturedPath) + { + var ordered = new List { installDir }; + if (!string.IsNullOrEmpty(capturedPath)) + ordered.AddRange(capturedPath.Split(':')); + ordered.AddRange(SystemPathFloor); + + var seen = new HashSet(StringComparer.Ordinal); + var result = new List(ordered.Count); + foreach (var entry in ordered) + { + // Drop empty elements (the CWD-resolution hazard); keep everything else verbatim. + if (entry.Length == 0) + continue; + if (seen.Add(entry)) + result.Add(entry); + } + + return string.Join(':', result); + } + + /// + /// Renders the full environment-file content: a single PATH= assignment + /// with a trailing newline. systemd EnvironmentFile= parses bare + /// KEY=VALUE lines and does not perform shell expansion, so the literal + /// captured value is written verbatim. + /// + internal static string Render(string installDir, string? capturedPath) + => $"{PathAssignmentPrefix}{ComposePathValue(installDir, capturedPath)}\n"; + + /// + /// Extracts the PATH value from environment-file content, or null + /// when no PATH= assignment is present. Leading whitespace is tolerated; + /// other keys are ignored. + /// + internal static string? ReadPathValue(string fileContent) + { + foreach (var raw in fileContent.Split('\n')) + { + var line = raw.Trim(); + if (line.StartsWith(PathAssignmentPrefix, StringComparison.Ordinal)) + return line[PathAssignmentPrefix.Length..]; + } + + return null; + } + + /// + /// True when (a ':'-separated PATH) contains + /// as one of its entries (ordinal, exact). + /// + internal static bool PathContainsDirectory(string pathValue, string directory) + { + var entries = pathValue.Split(':', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + return entries.Any(e => string.Equals(e, directory, StringComparison.Ordinal)); + } + + // ── systemd unit parsing (POSIX semantics regardless of host OS) ── + + /// + /// Returns the first unit line whose whitespace-trimmed start matches + /// , or null. systemd allows leading whitespace + /// before directives; we accept it. + /// + internal static string? FindDirective(IReadOnlyList lines, string prefix) + { + foreach (var rawLine in lines) + { + var line = rawLine.TrimStart(); + if (line.StartsWith(prefix, StringComparison.Ordinal)) + return line; + } + + return null; + } + + /// + /// Derives the daemon's install directory from the unit's ExecStart= + /// (the parent directory of the first whitespace-delimited token). Returns + /// false when ExecStart= is absent or has no directory component. + /// + internal static bool TryGetInstallDir(IReadOnlyList unitLines, out string installDir) + { + installDir = string.Empty; + + var execStart = FindDirective(unitLines, ExecStartPrefix); + if (execStart is null) + return false; + + var binaryPath = ExtractFirstToken(execStart); + var lastSlash = binaryPath.LastIndexOf('/'); + if (lastSlash <= 0) + return false; + + installDir = binaryPath[..lastSlash]; + return installDir.Length > 0; + } + + /// + /// Extracts the environment-file path referenced by the unit's + /// EnvironmentFile= directive, stripping systemd's optional - + /// tolerant-load prefix. Returns false when the directive is absent. + /// + internal static bool TryGetEnvironmentFilePath(IReadOnlyList unitLines, out string environmentFilePath) + { + environmentFilePath = string.Empty; + + var directive = FindDirective(unitLines, EnvironmentFilePrefix); + if (directive is null) + return false; + + var value = directive[EnvironmentFilePrefix.Length..].Trim(); + if (value.StartsWith('-')) + value = value[1..]; + + environmentFilePath = value; + return environmentFilePath.Length > 0; + } + + /// + /// Extracts the value of a legacy inline Environment=PATH= directive (the + /// pre-#1544 unit shape), or false when absent. Used to tell a still-functional + /// legacy unit (inline PATH that resolves the install dir) apart from a broken one. + /// + internal static bool TryGetInlinePath(IReadOnlyList unitLines, out string pathValue) + { + pathValue = string.Empty; + + var directive = FindDirective(unitLines, InlinePathPrefix); + if (directive is null) + return false; + + pathValue = directive[InlinePathPrefix.Length..].Trim(); + return pathValue.Length > 0; + } + + /// + /// Extracts the first whitespace-delimited token from a directive value + /// (e.g. ExecStart=/path/netclawd --flag → /path/netclawd). + /// + private static string ExtractFirstToken(string directive) + { + var equalsIndex = directive.IndexOf('='); + if (equalsIndex < 0 || equalsIndex == directive.Length - 1) + return string.Empty; + + var value = directive[(equalsIndex + 1)..].TrimStart(); + var spaceIndex = value.IndexOf(' '); + return spaceIndex < 0 ? value : value[..spaceIndex]; + } +} diff --git a/src/Netclaw.Cli/Doctor/DoctorFixService.cs b/src/Netclaw.Cli/Doctor/DoctorFixService.cs index 4a8e9b208..c77225c88 100644 --- a/src/Netclaw.Cli/Doctor/DoctorFixService.cs +++ b/src/Netclaw.Cli/Doctor/DoctorFixService.cs @@ -6,25 +6,52 @@ using System.Text.Json.Nodes; using Json.Schema; using Netclaw.Cli.Config; +using Netclaw.Cli.Daemon; using Netclaw.Cli.Json; using Netclaw.Configuration; namespace Netclaw.Cli.Doctor; -public sealed class DoctorFixService(NetclawPaths paths) +public sealed class DoctorFixService { + private readonly NetclawPaths _paths; + private readonly string _systemdUnitPath; + private readonly bool _systemdEnabled; + + public DoctorFixService(NetclawPaths paths) + : this(paths, DaemonManager.SystemdUserUnitFilePath, OperatingSystem.IsLinux()) + { + } + + /// + /// Test seam: explicit systemd unit path and platform gate so the daemon PATH + /// rehydration fix can be exercised hermetically, without depending on the host's + /// real ~/.config/systemd/user/netclaw.service. + /// + internal DoctorFixService(NetclawPaths paths, string systemdUnitPath, bool systemdEnabled) + { + _paths = paths; + _systemdUnitPath = systemdUnitPath; + _systemdEnabled = systemdEnabled; + } + public Task BuildPlanAsync(CancellationToken cancellationToken = default) { var fixes = new List(); - if (!File.Exists(paths.NetclawConfigPath)) + // Daemon shell-tool PATH rehydration is independent of netclaw.json — it must be + // evaluated even when the app config file is absent, so it runs before the + // config-file early-return below. + TryAddDaemonPathEnvironmentFix(fixes); + + if (!File.Exists(_paths.NetclawConfigPath)) return Task.FromResult(new DoctorFixPlan(fixes)); string original; JsonObject? obj; try { - original = File.ReadAllText(paths.NetclawConfigPath); + original = File.ReadAllText(_paths.NetclawConfigPath); obj = JsonNode.Parse(original) as JsonObject; } catch @@ -106,7 +133,7 @@ public Task BuildPlanAsync(CancellationToken cancellationToken = : normalized + Environment.NewLine; fixes.Add(new DoctorFileFix( - FilePath: paths.NetclawConfigPath, + FilePath: _paths.NetclawConfigPath, Description: $"Apply safe configuration autofixes ({string.Join(", ", appliedFixes)}).", OriginalText: original, UpdatedText: replacement)); @@ -147,10 +174,101 @@ private static void TryApplySchemaFixes(JsonObject config, List appliedF appliedFixes.AddRange(schemaFixes); } + /// + /// Rehydrates the daemon's shell-tool PATH environment file + /// () from the operator's + /// current, real PATH when it is missing or no longer includes the daemon's install + /// directory. The CLI process running doctor --fix is a child of the operator's + /// shell, so its PATH is the value we want — captured with zero shell execution. + /// + /// + /// Only acts when the installed unit already references this env file. Legacy units + /// (inline Environment=PATH=, no EnvironmentFile=) are routed to + /// netclaw daemon install by SystemdUnitPathDoctorCheck; doctor --fix + /// does not rewrite systemd units. The fix writes the file only — the operator must run + /// systemctl --user restart netclaw (surfaced in the description) for the daemon + /// to pick it up; we never restart the daemon implicitly. + /// + private void TryAddDaemonPathEnvironmentFix(List fixes) + { + if (!_systemdEnabled || !File.Exists(_systemdUnitPath)) + return; + + string[] unitLines; + try + { + unitLines = File.ReadAllLines(_systemdUnitPath); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + return; + } + + // Require the unit to reference OUR env file and to expose an install dir; anything + // else is a reinstall case, not a file-content rehydration. + if (!DaemonPathEnvironmentFile.TryGetEnvironmentFilePath(unitLines, out var referencedEnvPath) + || !DaemonPathEnvironmentFile.TryGetInstallDir(unitLines, out var installDir)) + { + return; + } + + var envPath = _paths.DaemonEnvironmentFilePath; + try + { + if (!string.Equals(Path.GetFullPath(referencedEnvPath), Path.GetFullPath(envPath), StringComparison.Ordinal)) + return; + } + catch (Exception ex) when (ex is ArgumentException or PathTooLongException or NotSupportedException) + { + // A hand-edited/malformed EnvironmentFile= value is not our managed file; skip the + // daemon-PATH fix rather than aborting the whole doctor --fix run on GetFullPath. + return; + } + + string? existing = null; + if (File.Exists(envPath)) + { + try + { + existing = File.ReadAllText(envPath); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + return; + } + } + + var healthy = existing is not null + && DaemonPathEnvironmentFile.ReadPathValue(existing) is { } current + && DaemonPathEnvironmentFile.PathContainsDirectory(current, installDir); + + if (healthy) + return; + + var captured = DaemonPathEnvironmentFile.CaptureCurrentPath(); + var updated = DaemonPathEnvironmentFile.Render(installDir, captured); + + fixes.Add(new DoctorFileFix( + FilePath: envPath, + Description: "Rehydrate the daemon's shell-tool PATH from your current environment. " + + "Run `systemctl --user restart netclaw` afterward for the daemon to pick it up.", + OriginalText: existing ?? string.Empty, + UpdatedText: updated)); + } + public async Task ApplyAsync(DoctorFixPlan plan, CancellationToken cancellationToken = default) { foreach (var fix in plan.Fixes) + { + // Ensure the parent directory exists before writing. The daemon-PATH fix can + // target ~/.netclaw/config even after that directory has been removed, so a bare + // File.WriteAllTextAsync would throw DirectoryNotFoundException and abort the run. + var dir = Path.GetDirectoryName(fix.FilePath); + if (!string.IsNullOrEmpty(dir)) + Directory.CreateDirectory(dir); + await File.WriteAllTextAsync(fix.FilePath, fix.UpdatedText, cancellationToken); + } } } diff --git a/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs b/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs index a9477c903..55329f97f 100644 --- a/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs +++ b/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs @@ -9,24 +9,33 @@ namespace Netclaw.Cli.Doctor; /// /// Validates that the systemd --user unit installed by -/// netclaw daemon install bakes a PATH that resolves the daemon's -/// install directory. Without this, ShellTool and -/// BackgroundJobExecutionActor spawn bash -c with the -/// sanitized systemd default PATH and cannot find netclaw, -/// ~/.local/bin tools, or anything else outside the system path. +/// netclaw daemon install supplies the daemon's shell-tool PATH via a +/// netclaw-owned EnvironmentFile= that resolves the daemon's install +/// directory. Without it, ShellTool and BackgroundJobExecutionActor +/// spawn bash -c with the sanitized systemd default PATH and cannot find +/// netclaw, dotnet, or anything else outside the system path. /// /// -/// This is a Linux-only diagnostic. On non-Linux platforms — and on Linux -/// boxes where the user runs netclaw daemon start directly instead -/// of installing the service — this check passes silently because the -/// daemon inherits the operator's interactive shell PATH and the failure -/// mode does not apply. +/// This is the consumer side of the PATH provisioning contract; the producer is +/// install and the rehydrator is DoctorFixService. +/// All three share so the file format, the +/// EnvironmentFile= wiring, and the install-dir semantics stay in agreement. +/// +/// Linux-only. On non-Linux platforms — and on Linux boxes where the operator runs +/// netclaw daemon start directly instead of installing the service — this check +/// passes silently, because the manual daemon inherits the operator's interactive shell +/// PATH and the failure mode does not apply. /// public sealed class SystemdUnitPathDoctorCheck : IDoctorCheck { private const string CheckName = "Systemd Unit PATH"; - private const string ExecStartPrefix = "ExecStart="; - private const string PathDirectivePrefix = "Environment=PATH="; + + private const string ReinstallRemediation = + "Reinstall to migrate to the environment-file model: " + + "`netclaw daemon uninstall && netclaw daemon install`, then `systemctl --user restart netclaw`."; + + private const string RehydrateRemediation = + "Rehydrate it: `netclaw doctor --fix`, then `systemctl --user restart netclaw`."; private readonly string _unitFilePath; private readonly bool _enabledOnThisPlatform; @@ -37,8 +46,8 @@ public SystemdUnitPathDoctorCheck() } /// - /// Test seam: explicit unit path and platform gate so tests can exercise - /// the parser on any host without needing a real systemd installation. + /// Test seam: explicit unit path and platform gate so tests can exercise the + /// parser on any host without needing a real systemd installation. /// internal SystemdUnitPathDoctorCheck(string unitFilePath, bool enabledOnThisPlatform) { @@ -73,86 +82,83 @@ public Task RunAsync(CancellationToken cancellationToken = de "Check file permissions.")); } - var execStart = FindDirective(lines, ExecStartPrefix); - if (execStart is null) + if (!DaemonPathEnvironmentFile.TryGetInstallDir(lines, out var installDir)) { return Task.FromResult(DoctorCheckResult.Warning( CheckName, - $"{unitPath} is missing ExecStart=. Unit file may be malformed.", - "Reinstall: `netclaw daemon uninstall && netclaw daemon install`.")); + $"Could not determine the daemon install directory from ExecStart in {unitPath}. " + + "The unit file may be malformed.", + ReinstallRemediation)); } - // systemd unit paths are always POSIX-style; use forward-slash semantics - // regardless of host OS so the parser is portable across CI runners. - var binaryPath = ExtractFirstToken(execStart); - var lastSlash = binaryPath.LastIndexOf('/'); - var installDir = lastSlash > 0 ? binaryPath[..lastSlash] : string.Empty; - if (string.IsNullOrEmpty(installDir)) + // Legacy/unwired unit: the pre-#1544 install baked an inline Environment=PATH= and + // no EnvironmentFile=. Such a unit is still fully functional if its inline PATH + // resolves the install dir (e.g. after an in-place binary upgrade without reinstall), + // so pass with a migration nudge rather than a false alarm. Only warn when the inline + // PATH is missing/incomplete, and route to reinstall (doctor --fix owns only the env + // file, not unit rewrites). + if (!DaemonPathEnvironmentFile.TryGetEnvironmentFilePath(lines, out var envFilePath)) { + if (DaemonPathEnvironmentFile.TryGetInlinePath(lines, out var inlinePath) + && DaemonPathEnvironmentFile.PathContainsDirectory(inlinePath, installDir)) + { + return Task.FromResult(DoctorCheckResult.Pass( + CheckName, + $"Legacy unit supplies PATH inline and includes {installDir}. Re-run " + + "`netclaw daemon install` to migrate to the managed environment file.")); + } + return Task.FromResult(DoctorCheckResult.Warning( CheckName, - $"Could not determine install directory from ExecStart in {unitPath}.", - "Reinstall: `netclaw daemon uninstall && netclaw daemon install`.")); + $"Systemd unit at {unitPath} does not supply the daemon's shell-tool PATH " + + "(no `EnvironmentFile=`, and no inline PATH that includes the install " + + "directory). The shell tool will fall back to the sanitized systemd PATH and " + + "cannot resolve `netclaw`, `dotnet`, or `~/.local/bin` tools.", + ReinstallRemediation)); } - var pathDirective = FindDirective(lines, PathDirectivePrefix); - if (pathDirective is null) + if (!File.Exists(envFilePath)) { return Task.FromResult(DoctorCheckResult.Warning( CheckName, - $"Systemd unit at {unitPath} does not set PATH. The daemon's shell tool will fail to resolve `netclaw`, " + - "`~/.local/bin` tools, and anything outside the systemd default PATH.", - "Reinstall to refresh the unit file: `netclaw daemon uninstall && netclaw daemon install`, " + - "then `systemctl --user restart netclaw`.")); + $"The PATH environment file referenced by {unitPath} is missing ({envFilePath}). " + + "The daemon's shell tool will fall back to the sanitized systemd PATH.", + RehydrateRemediation)); } - var pathValue = pathDirective[PathDirectivePrefix.Length..]; - var entries = pathValue.Split(':', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - var hasInstallDir = entries.Any(e => string.Equals(e, installDir, StringComparison.Ordinal)); - - if (!hasInstallDir) + string envContent; + try + { + envContent = File.ReadAllText(envFilePath); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { return Task.FromResult(DoctorCheckResult.Warning( CheckName, - $"Systemd unit PATH at {unitPath} does not include the daemon's install directory ({installDir}). " + - "Shell tool invocations may fail to resolve `netclaw`.", - "Reinstall: `netclaw daemon uninstall && netclaw daemon install`.")); + $"Could not read {envFilePath}: {ex.Message}", + "Check file permissions.")); } - return Task.FromResult(DoctorCheckResult.Pass( - CheckName, - $"Systemd unit PATH includes {installDir} ({entries.Length} entries).")); - } - - /// - /// Returns the first line whose trimmed start matches , - /// stripped of leading whitespace. systemd unit files allow whitespace before - /// directives; we accept it. Returns null if no match exists. - /// - private static string? FindDirective(string[] lines, string prefix) - { - foreach (var rawLine in lines) + var pathValue = DaemonPathEnvironmentFile.ReadPathValue(envContent); + if (pathValue is null) { - var line = rawLine.TrimStart(); - if (line.StartsWith(prefix, StringComparison.Ordinal)) - return line; + return Task.FromResult(DoctorCheckResult.Warning( + CheckName, + $"The PATH environment file {envFilePath} does not set PATH.", + RehydrateRemediation)); } - return null; - } - - /// - /// Extracts the first whitespace-delimited token from a directive value - /// (e.g., ExecStart=/path/to/netclawd --flag → /path/to/netclawd). - /// - private static string ExtractFirstToken(string directive) - { - var equalsIndex = directive.IndexOf('='); - if (equalsIndex < 0 || equalsIndex == directive.Length - 1) - return string.Empty; + if (!DaemonPathEnvironmentFile.PathContainsDirectory(pathValue, installDir)) + { + return Task.FromResult(DoctorCheckResult.Warning( + CheckName, + $"The PATH in {envFilePath} does not include the daemon's install directory " + + $"({installDir}). Shell tool invocations may fail to resolve `netclaw`.", + RehydrateRemediation)); + } - var value = directive[(equalsIndex + 1)..].TrimStart(); - var spaceIndex = value.IndexOf(' '); - return spaceIndex < 0 ? value : value[..spaceIndex]; + return Task.FromResult(DoctorCheckResult.Pass( + CheckName, + $"Daemon shell-tool PATH is sourced from {envFilePath} and includes {installDir}.")); } } diff --git a/src/Netclaw.Configuration/NetclawPaths.cs b/src/Netclaw.Configuration/NetclawPaths.cs index 7a254ab64..1d18307bf 100644 --- a/src/Netclaw.Configuration/NetclawPaths.cs +++ b/src/Netclaw.Configuration/NetclawPaths.cs @@ -89,6 +89,16 @@ public string ServerFeedSyncStatePath(string feedName) /// public string HardDenyOverridesPath => Path.Combine(ConfigDirectory, "hard-deny-overrides.json"); public string NetclawConfigPath => Path.Combine(ConfigDirectory, "netclaw.json"); + + /// + /// Netclaw-owned systemd EnvironmentFile= that supplies the installed + /// daemon's shell-tool PATH. Written by netclaw daemon install + /// and rehydrated by netclaw doctor --fix from the operator's real + /// (captured, not guessed) PATH; the daemon only ever reads it. Single + /// source of truth shared by the installer, the systemd PATH doctor check, and + /// uninstall so the producer/consumer contract stays in lockstep. + /// + public string DaemonEnvironmentFilePath => Path.Combine(ConfigDirectory, "daemon.env"); public string ClientConfigPath => Path.Combine(ClientDirectory, "config.json"); public string SecretsPath => Path.Combine(ConfigDirectory, "secrets.json"); public string DevicesPath => Path.Combine(ConfigDirectory, "devices.json");