From 5f4a98fc136d30af2e855fceb9f287f51b157261 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 3 Jul 2026 15:21:23 +0000 Subject: [PATCH 1/4] fix(daemon): capture operator PATH into EnvironmentFile for systemd shell tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A systemd --user service starts with a sanitized environment and does not inherit the operator's login-shell PATH, so the agent's shell tool cannot resolve `netclaw`, `dotnet`, or `~/.local/bin` binaries. `netclaw daemon install` previously baked a hardcoded PATH list into the unit, which can never anticipate every environment (#1544: `~/.dotnet` was invisible). Instead of guessing, capture the operator's real PATH from the CLI process itself (a child of the operator's shell, so no shell is spawned and no dotfiles are sourced) and hand it to the daemon via a netclaw-owned EnvironmentFile: - `NetclawPaths.DaemonEnvironmentFilePath` (config/daemon.env) - `DaemonPathEnvironmentFile`: single capture/compose/render/parse contract shared by the installer (producer), doctor --fix (rehydrator), and the doctor check (validator) - install writes `PATH=:` and wires the unit via `EnvironmentFile=-…` (tolerant load); the inline `Environment=PATH=` is gone - uninstall removes the env file (extracted `RemoveDaemonEnvironmentFile`) - `netclaw doctor --fix` rehydrates the file from the current shell PATH, independent of netclaw.json, and instructs a restart (never restarts the daemon implicitly) - `SystemdUnitPathDoctorCheck` validates the EnvironmentFile wiring + contents; legacy inline-PATH units are routed to reinstall PATH is confirmed security-neutral here: the shell command policy matches the literal typed verb token and never resolves against $PATH, so widening PATH cannot bypass a deny or widen an allow. Docs (SPEC-011, PRD-004) and the netclaw-operations skill updated. Closes #1544 --- docs/prd/PRD-004-cli-onboarding-and-config.md | 15 +- docs/spec/SPEC-011-daemon-architecture.md | 23 ++- .../.system/files/netclaw-operations/SKILL.md | 2 +- .../references/diagnostics.md | 2 +- .../.openspec.yaml | 2 + .../systemd-daemon-path-capture/design.md | 157 +++++++++++++++++ .../systemd-daemon-path-capture/proposal.md | 102 +++++++++++ .../specs/daemon-shell-path/spec.md | 91 ++++++++++ .../systemd-daemon-path-capture/tasks.md | 46 +++++ .../Daemon/DaemonPathEnvironmentFileTests.cs | 115 +++++++++++++ .../Doctor/DoctorFixServiceTests.cs | 155 +++++++++++++---- .../Doctor/SystemdUnitPathDoctorCheckTests.cs | 102 ++++++----- src/Netclaw.Cli/Daemon/DaemonManager.cs | 103 ++++++----- .../Daemon/DaemonPathEnvironmentFile.cs | 161 ++++++++++++++++++ src/Netclaw.Cli/Doctor/DoctorFixService.cs | 108 +++++++++++- .../Doctor/SystemdUnitPathDoctorCheck.cs | 135 +++++++-------- src/Netclaw.Configuration/NetclawPaths.cs | 10 ++ 17 files changed, 1128 insertions(+), 201 deletions(-) create mode 100644 openspec/changes/systemd-daemon-path-capture/.openspec.yaml create mode 100644 openspec/changes/systemd-daemon-path-capture/design.md create mode 100644 openspec/changes/systemd-daemon-path-capture/proposal.md create mode 100644 openspec/changes/systemd-daemon-path-capture/specs/daemon-shell-path/spec.md create mode 100644 openspec/changes/systemd-daemon-path-capture/tasks.md create mode 100644 src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs create mode 100644 src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs 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..db6a8be7f 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,24 @@ 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=:` 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..5480efc4b --- /dev/null +++ b/openspec/changes/systemd-daemon-path-capture/design.md @@ -0,0 +1,157 @@ +## 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 : `. Guarantees the bundled `netclaw` CLI +resolves first (matching current intent) while everything the operator has follows. If the captured +`PATH` already contains `installDir`, it still leads — duplicates are harmless to `PATH` lookup. + +### 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..2025c6ed8 --- /dev/null +++ b/openspec/changes/systemd-daemon-path-capture/specs/daemon-shell-path/spec.md @@ -0,0 +1,91 @@ +## 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, ahead of the +captured operator `PATH`, so the bundled `netclaw` CLI always resolves. + +#### 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 + +`netclaw doctor --fix` SHALL rehydrate the daemon PATH environment file from the current shell's +`PATH` when the file is missing, not referenced by the unit, 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 and SHALL surface an explicit instruction +to run `systemctl --user restart netclaw`; it SHALL NOT restart the daemon implicitly. + +#### 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. On any failure it SHALL return a warning whose +remediation points the operator at `netclaw doctor --fix` (or reinstall) followed by a service +restart. 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: Missing EnvironmentFile directive warns + +- **WHEN** the installed unit does not reference the environment file via `EnvironmentFile=` +- **THEN** the check returns a warning with remediation to run `netclaw doctor --fix` 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..f330ecab7 --- /dev/null +++ b/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs @@ -0,0 +1,115 @@ +// ----------------------------------------------------------------------- +// +// 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_PrependsInstallDir() + => Assert.Equal( + "/opt/netclaw:/home/u/.dotnet:/usr/bin", + DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", "/home/u/.dotnet:/usr/bin")); + + [Fact] + public void ComposePathValue_EmptyCapture_YieldsInstallDirOnly() + { + // A missing PATH on the installing shell is its own broken state — we don't + // paper over it with an invented default directory list. + Assert.Equal("/opt/netclaw", DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", null)); + Assert.Equal("/opt/netclaw", DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", "")); + } + + [Fact] + public void Render_ThenReadPathValue_RoundTrips() + { + var content = DaemonPathEnvironmentFile.Render("/opt/netclaw", "/home/u/.dotnet:/usr/bin"); + + Assert.Equal("PATH=/opt/netclaw:/home/u/.dotnet:/usr/bin\n", content); + Assert.Equal("/opt/netclaw:/home/u/.dotnet:/usr/bin", 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..80515c37e 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,13 @@ namespace Netclaw.Cli.Tests.Doctor; public sealed class DoctorFixServiceTests { + // ── 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 +28,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 +39,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 +49,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 +61,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 +76,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 +87,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 +103,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 +116,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 +126,124 @@ 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 installDir = Path.Combine(paths.BasePath, "bin"); + var unitPath = WriteWiredUnit(paths, installDir); + // 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 installDir = Path.Combine(paths.BasePath, "bin"); + var unitPath = WriteWiredUnit(paths, installDir); + 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 installDir = Path.Combine(paths.BasePath, "bin"); + var unitPath = WriteWiredUnit(paths, installDir); + 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 installDir = Path.Combine(paths.BasePath, "bin"); + 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 installDir = Path.Combine(paths.BasePath, "bin"); + var unitPath = WriteWiredUnit(paths, installDir); + + 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); + } + + 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, string installDir) + => WriteRawUnit(DaemonManager.BuildDaemonUnitContent( + Path.Combine(installDir, "netclawd"), + Path.Combine(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..371f353a9 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,103 @@ public async Task ReturnsPass_WhenUnitFileDoesNotExist() } [Fact] - public async Task ReturnsWarning_WhenPathDirectiveMissing() + public async Task ReturnsWarning_WhenExecStartMissing() { - var unitPath = WriteUnit(""" - [Unit] - Description=Netclaw Daemon - - [Service] - Type=simple - ExecStart=/opt/netclaw/netclawd - Environment=DOTNET_ENVIRONMENT=Production - """); + 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("does not set PATH", result.Message, StringComparison.Ordinal); - Assert.Contains("daemon uninstall", result.Remediation!, StringComparison.Ordinal); + Assert.Contains("Could not determine the daemon install directory", result.Message, StringComparison.Ordinal); } [Fact] - public async Task ReturnsWarning_WhenPathMissingInstallDir() + public async Task ReturnsWarning_WhenEnvironmentFileDirectiveMissing() { - var unitPath = WriteUnit(""" + // A legacy (pre-#1544) unit with an inline Environment=PATH= and no EnvironmentFile=. + // Route these to reinstall, which drops the inline directive and writes the env file. + var (unitPath, _) = WriteUnitDir(); + File.WriteAllText(unitPath, """ [Service] ExecStart=/opt/netclaw/netclawd - Environment=PATH=/usr/local/bin:/usr/bin:/bin + 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 include the daemon's install directory", result.Message, StringComparison.Ordinal); - Assert.Contains("/opt/netclaw", result.Message, StringComparison.Ordinal); + Assert.Contains("does not reference a PATH environment file", 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 +142,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..feac2cda8 --- /dev/null +++ b/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs @@ -0,0 +1,161 @@ +// ----------------------------------------------------------------------- +// +// 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="; + + /// + /// 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 (so the bundled netclaw CLI always + /// resolves), followed by the captured operator PATH. An empty/null + /// captured value yields the install directory alone — if the installing shell + /// had no PATH, that is its own broken state, not something to paper over + /// with an invented default. Separator is the POSIX ':' (systemd is + /// Linux-only). + /// + internal static string ComposePathValue(string installDir, string? capturedPath) + => string.IsNullOrEmpty(capturedPath) ? installDir : $"{installDir}:{capturedPath}"; + + /// + /// 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 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..83ebc6ed7 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,6 +174,79 @@ 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; + if (!string.Equals(Path.GetFullPath(referencedEnvPath), Path.GetFullPath(envPath), StringComparison.Ordinal)) + 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) diff --git a/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs b/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs index a9477c903..d030865ff 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,70 @@ 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=. Route these to reinstall (which drops the inline directive + // and writes the env file) rather than doctor --fix, which only owns the env file. + if (!DaemonPathEnvironmentFile.TryGetEnvironmentFilePath(lines, out var envFilePath)) { 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 reference a PATH environment file " + + "(`EnvironmentFile=`). The daemon's 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"); From 0ce8e22da6896a5f750e6e8f792d4661572f98cc Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 3 Jul 2026 16:13:45 +0000 Subject: [PATCH 2/4] fix(daemon): address code-review findings on PATH capture Follow-up to the capture-not-guess change, fixing defects surfaced by an xhigh code review: - Restore a guaranteed system-directory floor. Composition is now `installDir : : /usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin`, de-duplicated. Dropping the old unit-baked floor meant an empty/unset capture yielded `PATH=installDir` alone (silent total shell-tool break, which the doctor check still passed), and a desktop login PATH omitting /usr/sbin,/sbin lost admin-tool resolution. - Drop empty PATH elements. A POSIX empty element (`::`, from `PATH="$PATH:"`) resolves to the current directory, letting a binary planted in an agent-controlled workspace shadow a system command. - doctor --fix: ApplyAsync now creates the parent directory before writing (was throwing DirectoryNotFoundException and aborting the run if ~/.netclaw/config had been removed), and the EnvironmentFile= path comparison is guarded against malformed values crashing Path.GetFullPath. - Doctor check no longer false-warns a functional legacy inline-PATH unit (e.g. after an in-place binary upgrade without reinstall); it passes with a migration note and only warns when the inline PATH lacks the install dir. - Reconcile the spec deltas with the implementation: doctor --fix owns only the env file (unwired/legacy units route to reinstall), and document the floor + empty-element sanitization. Tests added for empty-capture floor, empty-element stripping, functional vs broken legacy units, config-dir-removed rehydration, and malformed unit path. 286 Daemon+Doctor tests pass; slopwatch 0. --- docs/spec/SPEC-011-daemon-architecture.md | 4 +- .../systemd-daemon-path-capture/design.md | 17 ++++- .../specs/daemon-shell-path/spec.md | 42 +++++++---- .../Daemon/DaemonPathEnvironmentFileTests.cs | 35 +++++++--- .../Doctor/DoctorFixServiceTests.cs | 31 ++++++++ .../Doctor/SystemdUnitPathDoctorCheckTests.cs | 29 ++++++-- .../Daemon/DaemonPathEnvironmentFile.cs | 70 ++++++++++++++++--- src/Netclaw.Cli/Doctor/DoctorFixService.cs | 20 +++++- .../Doctor/SystemdUnitPathDoctorCheck.cs | 23 ++++-- 9 files changed, 226 insertions(+), 45 deletions(-) diff --git a/docs/spec/SPEC-011-daemon-architecture.md b/docs/spec/SPEC-011-daemon-architecture.md index db6a8be7f..c458e216e 100644 --- a/docs/spec/SPEC-011-daemon-architecture.md +++ b/docs/spec/SPEC-011-daemon-architecture.md @@ -250,7 +250,9 @@ non-interactive environment that does not inherit the operator's login-shell (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=:` to `~/.netclaw/config/daemon.env`. The unit +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 diff --git a/openspec/changes/systemd-daemon-path-capture/design.md b/openspec/changes/systemd-daemon-path-capture/design.md index 5480efc4b..78b561b1b 100644 --- a/openspec/changes/systemd-daemon-path-capture/design.md +++ b/openspec/changes/systemd-daemon-path-capture/design.md @@ -81,9 +81,20 @@ references it by resolved absolute path. ### D4: Install-dir is prepended to the captured PATH -Provisioned value = `installDir : `. Guarantees the bundled `netclaw` CLI -resolves first (matching current intent) while everything the operator has follows. If the captured -`PATH` already contains `installDir`, it still leads — duplicates are harmless to `PATH` lookup. +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 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 index 2025c6ed8..0aadde865 100644 --- 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 @@ -9,8 +9,11 @@ a shell or source operator dotfiles to obtain the value. The captured `PATH` SHA 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, ahead of the -captured operator `PATH`, so the bundled `netclaw` CLI always resolves. +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 @@ -29,11 +32,14 @@ captured operator `PATH`, so the bundled `netclaw` CLI always resolves. ### Requirement: `doctor --fix` rehydrates the daemon PATH environment file -`netclaw doctor --fix` SHALL rehydrate the daemon PATH environment file from the current shell's -`PATH` when the file is missing, not referenced by the unit, 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 and SHALL surface an explicit instruction -to run `systemctl --user restart netclaw`; it SHALL NOT restart the daemon implicitly. +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 @@ -54,9 +60,12 @@ to run `systemctl --user restart netclaw`; it SHALL NOT restart the daemon impli `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. On any failure it SHALL return a warning whose -remediation points the operator at `netclaw doctor --fix` (or reinstall) followed by a service -restart. The check SHALL pass silently when no service is installed or on non-Linux platforms. +`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 @@ -64,10 +73,17 @@ restart. The check SHALL pass silently when no service is installed or on non-Li install directory - **THEN** the check passes -#### Scenario: Missing EnvironmentFile directive warns +#### Scenario: Functional legacy inline-PATH unit passes with a migration note -- **WHEN** the installed unit does not reference the environment file via `EnvironmentFile=` -- **THEN** the check returns a warning with remediation to run `netclaw doctor --fix` and restart +- **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 diff --git a/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs b/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs index f330ecab7..db130cdc5 100644 --- a/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs +++ b/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs @@ -21,27 +21,42 @@ namespace Netclaw.Cli.Tests.Daemon; public sealed class DaemonPathEnvironmentFileTests { [Fact] - public void ComposePathValue_PrependsInstallDir() + public void ComposePathValue_InstallDirFirst_ThenCapture_ThenDedupedFloor() => Assert.Equal( - "/opt/netclaw:/home/u/.dotnet:/usr/bin", + "/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_YieldsInstallDirOnly() + public void ComposePathValue_EmptyCapture_StillHasFunctionalFloor() { - // A missing PATH on the installing shell is its own broken state — we don't - // paper over it with an invented default directory list. - Assert.Equal("/opt/netclaw", DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", null)); - Assert.Equal("/opt/netclaw", DaemonPathEnvironmentFile.ComposePathValue("/opt/netclaw", "")); + // 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", "/home/u/.dotnet:/usr/bin"); + var content = DaemonPathEnvironmentFile.Render("/opt/netclaw", "/usr/bin"); - Assert.Equal("PATH=/opt/netclaw:/home/u/.dotnet:/usr/bin\n", content); - Assert.Equal("/opt/netclaw:/home/u/.dotnet:/usr/bin", DaemonPathEnvironmentFile.ReadPathValue(content)); + 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] diff --git a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs index 80515c37e..3c2acbf74 100644 --- a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs @@ -219,6 +219,37 @@ public async Task AppliesEnvFileRehydrationToDisk() 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 installDir = Path.Combine(paths.BasePath, "bin"); + var unitPath = WriteWiredUnit(paths, installDir); + 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()); diff --git a/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs b/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs index 371f353a9..75aaf0bb8 100644 --- a/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/SystemdUnitPathDoctorCheckTests.cs @@ -50,10 +50,11 @@ public async Task ReturnsWarning_WhenExecStartMissing() } [Fact] - public async Task ReturnsWarning_WhenEnvironmentFileDirectiveMissing() + public async Task ReturnsPass_WhenLegacyInlinePathIncludesInstallDir() { - // A legacy (pre-#1544) unit with an inline Environment=PATH= and no EnvironmentFile=. - // Route these to reinstall, which drops the inline directive and writes the env file. + // 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] @@ -64,8 +65,28 @@ public async Task ReturnsWarning_WhenEnvironmentFileDirectiveMissing() var result = await check.RunAsync(TestContext.Current.CancellationToken); + 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_WhenLegacyUnitLacksUsablePath() + { + // 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/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 reference a PATH environment file", 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); } diff --git a/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs b/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs index feac2cda8..a8a6dadd6 100644 --- a/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs +++ b/src/Netclaw.Cli/Daemon/DaemonPathEnvironmentFile.cs @@ -26,6 +26,19 @@ 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. @@ -35,16 +48,40 @@ internal static class DaemonPathEnvironmentFile internal static string? CaptureCurrentPath() => Environment.GetEnvironmentVariable("PATH"); /// - /// Composes the PATH value written to the environment file: the daemon's - /// own install directory first (so the bundled netclaw CLI always - /// resolves), followed by the captured operator PATH. An empty/null - /// captured value yields the install directory alone — if the installing shell - /// had no PATH, that is its own broken state, not something to paper over - /// with an invented default. Separator is the POSIX ':' (systemd is - /// Linux-only). + /// 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) - => string.IsNullOrEmpty(capturedPath) ? installDir : $"{installDir}:{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 @@ -144,6 +181,23 @@ internal static bool TryGetEnvironmentFilePath(IReadOnlyList unitLines, 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). diff --git a/src/Netclaw.Cli/Doctor/DoctorFixService.cs b/src/Netclaw.Cli/Doctor/DoctorFixService.cs index 83ebc6ed7..c77225c88 100644 --- a/src/Netclaw.Cli/Doctor/DoctorFixService.cs +++ b/src/Netclaw.Cli/Doctor/DoctorFixService.cs @@ -213,8 +213,17 @@ private void TryAddDaemonPathEnvironmentFix(List fixes) } var envPath = _paths.DaemonEnvironmentFilePath; - if (!string.Equals(Path.GetFullPath(referencedEnvPath), Path.GetFullPath(envPath), StringComparison.Ordinal)) + 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)) @@ -250,7 +259,16 @@ private void TryAddDaemonPathEnvironmentFix(List fixes) 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 d030865ff..55329f97f 100644 --- a/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs +++ b/src/Netclaw.Cli/Doctor/SystemdUnitPathDoctorCheck.cs @@ -92,15 +92,28 @@ public Task RunAsync(CancellationToken cancellationToken = de } // Legacy/unwired unit: the pre-#1544 install baked an inline Environment=PATH= and - // no EnvironmentFile=. Route these to reinstall (which drops the inline directive - // and writes the env file) rather than doctor --fix, which only owns the env file. + // 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, - $"Systemd unit at {unitPath} does not reference a PATH environment file " - + "(`EnvironmentFile=`). The daemon's shell tool will fall back to the sanitized " - + "systemd PATH and cannot resolve `netclaw`, `dotnet`, or `~/.local/bin` tools.", + $"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)); } From e54584587544c3d24f793ae51c34dc0f9166b7a4 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 3 Jul 2026 16:29:15 +0000 Subject: [PATCH 3/4] test(daemon): use POSIX unit paths in doctor-fix tests (fix Windows CI) The DoctorFixService daemon-PATH tests built ExecStart via Path.Combine, which yields backslashes on Windows. TryGetInstallDir correctly parses POSIX `/` (systemd units are always POSIX), so install-dir parsing failed on windows-latest and no rehydration fix was planned, failing 4 tests. Build the test units with forward slashes and a POSIX install-dir literal, matching real systemd units and the (already-passing) SystemdUnitPathDoctorCheck tests. No production change. --- .../Doctor/DoctorFixServiceTests.cs | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs index 3c2acbf74..540e1e18a 100644 --- a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs @@ -140,7 +140,9 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, public async Task RehydratesEnvFile_WhenMissing_EvenWithoutNetclawJson() { var paths = NewPaths(); - var installDir = Path.Combine(paths.BasePath, "bin"); + // POSIX install dir: systemd units are always POSIX-style regardless of the host OS + // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. + const string installDir = "/opt/netclaw"; var unitPath = WriteWiredUnit(paths, installDir); // No netclaw.json and no env file on disk. @@ -157,7 +159,9 @@ public async Task RehydratesEnvFile_WhenMissing_EvenWithoutNetclawJson() public async Task RehydratesEnvFile_WhenStale_MissingInstallDir() { var paths = NewPaths(); - var installDir = Path.Combine(paths.BasePath, "bin"); + // POSIX install dir: systemd units are always POSIX-style regardless of the host OS + // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. + const string installDir = "/opt/netclaw"; var unitPath = WriteWiredUnit(paths, installDir); await File.WriteAllTextAsync(paths.DaemonEnvironmentFilePath, "PATH=/usr/bin\n", TestContext.Current.CancellationToken); @@ -174,7 +178,9 @@ await File.WriteAllTextAsync(paths.DaemonEnvironmentFilePath, "PATH=/usr/bin\n", public async Task NoEnvFix_WhenHealthy() { var paths = NewPaths(); - var installDir = Path.Combine(paths.BasePath, "bin"); + // POSIX install dir: systemd units are always POSIX-style regardless of the host OS + // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. + const string installDir = "/opt/netclaw"; var unitPath = WriteWiredUnit(paths, installDir); await File.WriteAllTextAsync( paths.DaemonEnvironmentFilePath, @@ -193,7 +199,9 @@ 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 installDir = Path.Combine(paths.BasePath, "bin"); + // POSIX install dir: systemd units are always POSIX-style regardless of the host OS + // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. + const string installDir = "/opt/netclaw"; var unitPath = WriteRawUnit( $"[Service]\nExecStart={installDir}/netclawd\nEnvironment=PATH=/opt/x:/usr/bin\n"); @@ -207,7 +215,9 @@ public async Task NoEnvFix_WhenUnitIsLegacyUnwired() public async Task AppliesEnvFileRehydrationToDisk() { var paths = NewPaths(); - var installDir = Path.Combine(paths.BasePath, "bin"); + // POSIX install dir: systemd units are always POSIX-style regardless of the host OS + // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. + const string installDir = "/opt/netclaw"; var unitPath = WriteWiredUnit(paths, installDir); var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); @@ -225,7 +235,9 @@ 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 installDir = Path.Combine(paths.BasePath, "bin"); + // POSIX install dir: systemd units are always POSIX-style regardless of the host OS + // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. + const string installDir = "/opt/netclaw"; var unitPath = WriteWiredUnit(paths, installDir); Directory.Delete(Path.GetDirectoryName(paths.DaemonEnvironmentFilePath)!, recursive: true); @@ -261,9 +273,11 @@ private static DoctorFixService ConfigOnlyService(NetclawPaths paths) => new(paths, Path.Combine(paths.BasePath, "unused.service"), systemdEnabled: false); private static string WriteWiredUnit(NetclawPaths paths, string installDir) + // Forward-slash concatenation (NOT Path.Combine): systemd ExecStart is POSIX even + // when the test runs on Windows, matching what TryGetInstallDir parses. => WriteRawUnit(DaemonManager.BuildDaemonUnitContent( - Path.Combine(installDir, "netclawd"), - Path.Combine(installDir, "netclaw"), + $"{installDir}/netclawd", + $"{installDir}/netclaw", paths.DaemonEnvironmentFilePath)); private static string WriteRawUnit(string content) From 654a12f1bf664e6b7b5919efbad4ba020e0732ff Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 3 Jul 2026 16:41:58 +0000 Subject: [PATCH 4/4] test(daemon): hoist duplicated POSIX install-dir literal to a class constant Replace the six copy-pasted `const string installDir = "/opt/netclaw"` blocks (and comment) in DoctorFixServiceTests with a single class-level `InstallDir` constant; drop the now-redundant WriteWiredUnit parameter. --- .../Doctor/DoctorFixServiceTests.cs | 48 +++++++------------ 1 file changed, 17 insertions(+), 31 deletions(-) diff --git a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs index 540e1e18a..f673b5443 100644 --- a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs @@ -12,6 +12,10 @@ 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) ── @@ -140,10 +144,7 @@ await File.WriteAllTextAsync(paths.NetclawConfigPath, public async Task RehydratesEnvFile_WhenMissing_EvenWithoutNetclawJson() { var paths = NewPaths(); - // POSIX install dir: systemd units are always POSIX-style regardless of the host OS - // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. - const string installDir = "/opt/netclaw"; - var unitPath = WriteWiredUnit(paths, installDir); + var unitPath = WriteWiredUnit(paths); // No netclaw.json and no env file on disk. var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); @@ -151,7 +152,7 @@ public async Task RehydratesEnvFile_WhenMissing_EvenWithoutNetclawJson() var fix = Assert.Single(plan.Fixes); Assert.Equal(paths.DaemonEnvironmentFilePath, fix.FilePath); - Assert.StartsWith($"PATH={installDir}:", fix.UpdatedText, StringComparison.Ordinal); + Assert.StartsWith($"PATH={InstallDir}:", fix.UpdatedText, StringComparison.Ordinal); Assert.Contains("systemctl --user restart netclaw", fix.Description, StringComparison.Ordinal); } @@ -159,10 +160,7 @@ public async Task RehydratesEnvFile_WhenMissing_EvenWithoutNetclawJson() public async Task RehydratesEnvFile_WhenStale_MissingInstallDir() { var paths = NewPaths(); - // POSIX install dir: systemd units are always POSIX-style regardless of the host OS - // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. - const string installDir = "/opt/netclaw"; - var unitPath = WriteWiredUnit(paths, installDir); + var unitPath = WriteWiredUnit(paths); await File.WriteAllTextAsync(paths.DaemonEnvironmentFilePath, "PATH=/usr/bin\n", TestContext.Current.CancellationToken); @@ -171,20 +169,17 @@ await File.WriteAllTextAsync(paths.DaemonEnvironmentFilePath, "PATH=/usr/bin\n", 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); + Assert.Contains(InstallDir, fix.UpdatedText, StringComparison.Ordinal); } [Fact] public async Task NoEnvFix_WhenHealthy() { var paths = NewPaths(); - // POSIX install dir: systemd units are always POSIX-style regardless of the host OS - // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. - const string installDir = "/opt/netclaw"; - var unitPath = WriteWiredUnit(paths, installDir); + var unitPath = WriteWiredUnit(paths); await File.WriteAllTextAsync( paths.DaemonEnvironmentFilePath, - DaemonPathEnvironmentFile.Render(installDir, "/usr/bin"), + DaemonPathEnvironmentFile.Render(InstallDir, "/usr/bin"), TestContext.Current.CancellationToken); var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); @@ -199,11 +194,8 @@ 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(); - // POSIX install dir: systemd units are always POSIX-style regardless of the host OS - // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. - const string installDir = "/opt/netclaw"; var unitPath = WriteRawUnit( - $"[Service]\nExecStart={installDir}/netclawd\nEnvironment=PATH=/opt/x:/usr/bin\n"); + $"[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); @@ -215,10 +207,7 @@ public async Task NoEnvFix_WhenUnitIsLegacyUnwired() public async Task AppliesEnvFileRehydrationToDisk() { var paths = NewPaths(); - // POSIX install dir: systemd units are always POSIX-style regardless of the host OS - // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. - const string installDir = "/opt/netclaw"; - var unitPath = WriteWiredUnit(paths, installDir); + var unitPath = WriteWiredUnit(paths); var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); @@ -226,7 +215,7 @@ public async Task AppliesEnvFileRehydrationToDisk() Assert.True(File.Exists(paths.DaemonEnvironmentFilePath)); var content = await File.ReadAllTextAsync(paths.DaemonEnvironmentFilePath, TestContext.Current.CancellationToken); - Assert.Contains(installDir, content, StringComparison.Ordinal); + Assert.Contains(InstallDir, content, StringComparison.Ordinal); } [Fact] @@ -235,10 +224,7 @@ 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(); - // POSIX install dir: systemd units are always POSIX-style regardless of the host OS - // running the test, and TryGetInstallDir parses forward-slash ExecStart accordingly. - const string installDir = "/opt/netclaw"; - var unitPath = WriteWiredUnit(paths, installDir); + var unitPath = WriteWiredUnit(paths); Directory.Delete(Path.GetDirectoryName(paths.DaemonEnvironmentFilePath)!, recursive: true); var service = new DoctorFixService(paths, unitPath, systemdEnabled: true); @@ -272,12 +258,12 @@ private static NetclawPaths NewPaths() private static DoctorFixService ConfigOnlyService(NetclawPaths paths) => new(paths, Path.Combine(paths.BasePath, "unused.service"), systemdEnabled: false); - private static string WriteWiredUnit(NetclawPaths paths, string installDir) + 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", + $"{InstallDir}/netclawd", + $"{InstallDir}/netclaw", paths.DaemonEnvironmentFilePath)); private static string WriteRawUnit(string content)