From db95aed5a7496a5b6efd2f6e600a77a6a22e47d9 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Mon, 13 Jul 2026 04:27:03 -0700 Subject: [PATCH] feat(service): Windows display name, clean-reinstall, macOS ensure-hosts, 3-OS smoke CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - install now clean-reinstalls (stop -> delete -> wait -> create) instead of a bare create, so re-running it against an already-registered service no longer hits Windows CreateService error 1073. It deliberately never auto-starts (unlike the dig-dns pattern this mirrors), since the dig-installer issues its own start afterward and treats a start failure as fatal for that step. - Windows: sc config sets the SCM display name to "DIG NETWORK: NODE" after create, then sc qc reads it back to confirm the override actually took (result.display_name_verified in --json). Verified live in CI: sc qc reports DISPLAY_NAME: DIG NETWORK: NODE and display_name_verified:true. - The existing-service probe uses the identifier service-manager's OWN backend actually registers under: to_qualified_name() for Windows/launchd, but to_script_name() for systemd, whose unit-file naming diverges silently. Caught by a real ubuntu-latest smoke run (a second install reported reinstalled:false until fixed); regression test added. - macOS postinstall now calls the binary's own idempotent `ensure-hosts` so dig.local -> 127.0.0.2 is registered on .pkg install (Windows/.deb already did this). - New service-smoke CI job (windows-latest/macos-14/ubuntu-latest): build, install, start, poll for serving, assert the Windows display name, install a second time to prove no 1073, stop, uninstall, and assert the registration is gone. All three legs green against the fix. - config::DEFAULT_PORT now sources dig_constants::DIG_NODE_PORT (single-sourced §5.3 default) via a rev-pinned dig-constants 0.3.0 dependency, kept a cargo- distinct source from dig-node-core's bare-git 0.2.x pin so the P2P crate chain (dig-nat/dig-gossip/dig-dht/dig-onion) does not need to move in this PR. - SPEC.md: document the display-name + clean-reinstall contract (#494), and the previously-undocumented 9444 DIG_PEER_PORT P2P listener default. - DEVELOPMENT_LOG.md: durable realizations on the bare-git version-unification gotcha, the installer's install->start sequencing constraint, and the systemd script-name vs qualified-name divergence. Minor version bump (0.28.0 -> 0.29.0): new capability, no breaking change. Closes #494, refs #502. Co-Authored-By: Claude --- .github/workflows/service-smoke.yml | 163 ++++ Cargo.lock | 13 +- Cargo.toml | 2 +- DEVELOPMENT_LOG.md | 69 ++ README.md | 6 +- SPEC.md | 34 +- crates/dig-node-service/Cargo.toml | 15 + crates/dig-node-service/src/config.rs | 6 +- crates/dig-node-service/src/service.rs | 1035 ++++++++++++++++++++---- packaging/macos/scripts/postinstall | 10 +- 10 files changed, 1177 insertions(+), 176 deletions(-) create mode 100644 .github/workflows/service-smoke.yml create mode 100644 DEVELOPMENT_LOG.md diff --git a/.github/workflows/service-smoke.yml b/.github/workflows/service-smoke.yml new file mode 100644 index 0000000..1b7a5c7 --- /dev/null +++ b/.github/workflows/service-smoke.yml @@ -0,0 +1,163 @@ +# A REAL, 3-OS end-to-end smoke test of `dig-node install`/`start`/`uninstall` against the +# actual OS service manager (Windows SCM / systemd / launchd) — the thing ci.yml's mocked +# `ServiceBackend` unit tests (crates/dig-node-service/src/service.rs) cannot prove: that +# `install` registers a service the OS actually starts and serves from, that a SECOND `install` +# cleanly recreates it instead of hitting Windows `CreateService` error 1073 (#494), and that +# `uninstall` actually removes the registration. +# +# Kept as its OWN workflow (not folded into ci.yml) because it performs REAL, mutating +# OS-service registration on every runner — a materially different (and slower/less hermetic) +# kind of check than the rest of the PR gate set. +name: Service smoke test + +on: + pull_request: + paths: + - "crates/dig-node-service/src/service.rs" + - "crates/dig-node-service/src/config.rs" + - "crates/dig-node-service/src/hosts.rs" + - "packaging/**" + - ".github/workflows/service-smoke.yml" + push: + branches: + - main + workflow_dispatch: + +concurrency: + group: ${{ github.ref }}-${{ github.workflow }}-${{ github.event_name }} + cancel-in-progress: true + +env: + CARGO_TERM_COLOR: always + +jobs: + smoke: + name: install/start/reinstall/uninstall (${{ matrix.os }}) + strategy: + fail-fast: false + matrix: + os: [windows-latest, macos-14, ubuntu-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + + - uses: Swatinem/rust-cache@v2 + with: + key: service-smoke + + - name: Build dig-node (release) + run: cargo build --release --locked --bin dig-node + + # `dig-node install` registers a USER-level systemd unit on Linux (no root needed), which + # needs a real user D-Bus session — the GH-hosted runner's non-interactive job shell does + # not start one implicitly. `enable-linger` + pointing at the per-uid runtime dir brings one + # up so `systemctl --user ...` (what service-manager shells out to) has a bus to talk to. + - name: (Linux) bring up a user systemd/D-Bus session + if: runner.os == 'Linux' + run: | + sudo loginctl enable-linger "$(whoami)" + echo "XDG_RUNTIME_DIR=/run/user/$(id -u)" >> "$GITHUB_ENV" + + - name: Resolve the built binary path + id: bin + shell: bash + run: | + if [ "${{ runner.os }}" = "Windows" ]; then + echo "path=target/release/dig-node.exe" >> "$GITHUB_OUTPUT" + else + echo "path=target/release/dig-node" >> "$GITHUB_OUTPUT" + fi + + - name: Install (1st) — fresh registration + shell: bash + run: | + "${{ steps.bin.outputs.path }}" install --json | tee install-1.json + grep -q '"installed":true' install-1.json + grep -q '"reinstalled":false' install-1.json + + - name: Windows — assert the SCM display name + if: runner.os == 'Windows' + shell: bash + run: | + sc.exe qc net.dignetwork.dig-node | tee sc-qc-1.txt + grep -Eq 'DISPLAY_NAME[[:space:]]*: DIG NETWORK: NODE' sc-qc-1.txt + + - name: Start + poll for RUNNING + shell: bash + run: | + "${{ steps.bin.outputs.path }}" start --json || true + ok=0 + for _ in $(seq 1 20); do + if "${{ steps.bin.outputs.path }}" status --json | tee status-1.json | grep -q '"serving":true'; then + ok=1 + break + fi + sleep 1 + done + test "$ok" -eq 1 + + - name: Install (2nd) — proves clean-reinstall avoids CreateService 1073 + shell: bash + run: | + "${{ steps.bin.outputs.path }}" install --json | tee install-2.json + grep -q '"installed":true' install-2.json + grep -q '"reinstalled":true' install-2.json + + - name: Windows — re-assert the SCM display name after reinstall + if: runner.os == 'Windows' + shell: bash + run: | + sc.exe qc net.dignetwork.dig-node | tee sc-qc-2.txt + grep -Eq 'DISPLAY_NAME[[:space:]]*: DIG NETWORK: NODE' sc-qc-2.txt + + - name: Start again + poll for RUNNING + shell: bash + run: | + "${{ steps.bin.outputs.path }}" start --json || true + ok=0 + for _ in $(seq 1 20); do + if "${{ steps.bin.outputs.path }}" status --json | tee status-2.json | grep -q '"serving":true'; then + ok=1 + break + fi + sleep 1 + done + test "$ok" -eq 1 + + - name: Stop + shell: bash + run: | + "${{ steps.bin.outputs.path }}" stop --json + + - name: Uninstall + assert the service is gone + shell: bash + run: | + "${{ steps.bin.outputs.path }}" uninstall --json | tee uninstall.json + grep -q '"installed":false' uninstall.json + if [ "${{ runner.os }}" = "Windows" ]; then + ! sc.exe query net.dignetwork.dig-node + elif [ "${{ runner.os }}" = "macOS" ]; then + ! launchctl print "gui/$(id -u)/net.dignetwork.dig-node" + else + ! systemctl --user cat net.dignetwork.dig-node.service + fi + + - name: Upload diagnostics + if: always() + uses: actions/upload-artifact@v4 + with: + name: service-smoke-${{ matrix.os }} + path: | + install-1.json + install-2.json + status-1.json + status-2.json + uninstall.json + sc-qc-1.txt + sc-qc-2.txt + if-no-files-found: ignore diff --git a/Cargo.lock b/Cargo.lock index ef8e571..62327aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1800,6 +1800,16 @@ dependencies = [ "hex-literal", ] +[[package]] +name = "dig-constants" +version = "0.3.0" +source = "git+https://github.com/DIG-Network/dig-constants?rev=438ac8caf9ffb80cadd10362deacd63763a7f141#438ac8caf9ffb80cadd10362deacd63763a7f141" +dependencies = [ + "chia-consensus", + "chia-protocol", + "hex-literal", +] + [[package]] name = "dig-dht" version = "0.1.0" @@ -1956,11 +1966,12 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.28.0" +version = "0.29.0" dependencies = [ "axum", "base64", "clap", + "dig-constants 0.3.0", "dig-node-core", "dig-wallet", "digstore-core", diff --git a/Cargo.toml b/Cargo.toml index 5f4c9e8..88df313 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ edition = "2021" # (`[workspace.package].version`), so it MUST be set here for a release to fire # (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) keep their own # independent versions — only the released binary tracks the workspace version. -version = "0.28.0" +version = "0.29.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over diff --git a/DEVELOPMENT_LOG.md b/DEVELOPMENT_LOG.md new file mode 100644 index 0000000..4890b33 --- /dev/null +++ b/DEVELOPMENT_LOG.md @@ -0,0 +1,69 @@ +# Development Log — dig-node + +High-signal realizations from debugging/development: non-obvious cross-system couplings, +sharp edges, and gotchas. Concise durable facts with context — NOT a change diary. See +`CLAUDE.md` §4.5 for the maintenance contract (a curator periodically re-verifies + prunes). + +## Bare-git dependency version pins unify across the WHOLE dependency graph, not per-manifest (#494) + +A `git`-sourced Cargo dependency with NO `rev`/`branch`/`tag` is identified purely by its URL — +every crate in the build graph that declares `dig-constants = { git = "https://github.com/..." }` +with no ref resolves to the SAME package instance, whatever `version =` requirement each manifest +states individually. If two manifests state incompatible 0.x requirements against that one bare +source (e.g. `dig-node-core`'s transitive `dig-nat`/`dig-gossip`/`dig-dht`/`dig-onion` chain pins +`dig-constants = "0.2"`, i.e. `^0.2` = 0.2.x ONLY), cargo cannot resolve a single version +satisfying both — bumping one manifest's bare-git requirement to `"0.3"` breaks the whole graph +until every bare-git consumer moves together. + +**The escape hatch:** an EXPLICIT `rev =` pin is a cargo-DISTINCT source from a bare git dep, +even at the identical commit — so a crate that needs a newer version of a NOT-yet-crates.io'd +dependency can `rev`-pin its OWN copy without forcing every other bare-git consumer forward. This +is exactly how `dig-node-service` picked up `dig_constants::DIG_NODE_PORT` (added in 0.3.0) +without waiting on `dig-nat`/`dig-gossip`/`dig-dht`/`dig-onion` to move off their `^0.2` pin: it +added `dig-constants = { version = "0.3", git = "...", rev = "" }`, giving the +crate its own 0.3.0 instance living alongside the graph's existing 0.2.1 (bare-git) and 0.1.0 +(crates.io registry) instances. Safe whenever the only thing crossing the boundary between the +two instances is a plain value type (here, a `u16` constant) — never safe if a type from one +instance needs to be passed to/from code built against the other. + +## The dig-installer's `install` → `start` sequencing constrains what `dig-node install` may do + +`dig-installer`'s `register_dig_node` step calls `dig-node install` and then, when configured to +start it (the default), a SEPARATE `dig-node start` — and treats a `start` FAILURE as fatal for +that installer step (unlike the tolerant treatment of an `install` failure). This means +`dig-node install` must NEVER auto-start the service itself: if it did, the installer's follow-up +`start` would hit "service already running" (Windows SCM 1056, or a systemd/launchd +no-op-or-error depending on backend) and could flip the installer's REPORTED `installed` status +to `false` even though the service is actually up and running fine. `dig-dns`'s equivalent +`reinstall()` DOES auto-start at the end of its clean-reinstall — that pattern was deliberately +NOT mirrored here for this reason. Any future change to `dig-node install`'s start behavior must +also update `dig-installer`'s `register_dig_node`/`install_service` in the SAME unit of work. + +## `service-manager`'s systemd backend registers under `to_script_name()`, not `to_qualified_name()` (#494) + +`ServiceLabel` has TWO different string renderings — `to_qualified_name()` (`{qualifier}. +{organization}.{application}`, e.g. `net.dignetwork.dig-node`) and `to_script_name()` +(`{organization}-{application}`, e.g. `dignetwork-dig-node` — the qualifier is DROPPED +entirely). `service-manager` 0.7's Windows (`sc.rs`) and launchd (`launchd.rs`) backends both +register the service under `to_qualified_name()`, but its **systemd** backend (`systemd.rs`) +names the actual unit file from `to_script_name()` instead — a real, silent divergence with no +compile-time signal. Any code that probes "is this service registered?" by shelling out +directly (`service-manager` itself exposes no such query) MUST use the SAME name the relevant +backend actually registered under, per-platform — using `to_qualified_name()` uniformly makes +the probe always report "not found" on Linux, invisibly. This was caught only by a REAL 3-OS +`service-smoke` CI run (mocked-backend unit tests, being backend-agnostic by design, cannot +catch it) — a second `dig-node install` on `ubuntu-latest` reported `reinstalled:false` because +`is_installed()` never saw the service it had just registered. + +## Windows `sc create` always names the display the same as the service id + +`service-manager` 0.7's `sc.rs` backend hardcodes the Windows SCM display name to the service id +at `sc create` time — there is no `ServiceInstallCtx` field for it. The only way to set a +friendly display name is a POST-create `sc config displayname= ""` follow-up (and, +per #494, a `sc qc ` read-back to actually confirm it took, rather than trusting the `sc +config` exit code). `service-manager`'s systemd/launchd backends have no display-name-equivalent +override either — for systemd the closest analog is `Description=`, generated from the label with +no override field; for launchd there is no such key at all. The NATIVE `.deb`/`.pkg`/`.msi` +packages (`packaging/`) sidestep this entirely by shipping their own static unit +file/plist/WiX-`ServiceInstall` with the friendly name baked in — only the bare `dig-node install` +CLI path (not via a native package) needs the `sc config`/`sc qc` dance. diff --git a/README.md b/README.md index ae008cf..2c4fc66 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,10 @@ dig-node uninstall kill it with error 1053). After install it auto-starts on boot, and `install` also configures SCM **recovery actions** (`sc.exe failure` — restart 5s/10s/30s after the 1st/2nd/subsequent crash, resetting after a day with no further crashes) so a crashed service comes back up on its own, same as the systemd/launchd behavior below; check with `sc qfailure net.dignetwork.dig-node`. + It shows up in the Services console as **"DIG NETWORK: NODE"** (set via a post-create + `sc config` + a `sc qc` read-back to confirm it took). Re-running `install` against an + already-registered service cleanly stops + deregisters + recreates it instead of hitting + `CreateService` error 1073 ("the specified service already exists"). - **Linux / macOS:** the service installs at **user level** (systemd `--user` / a launchd GUI agent), so no `sudo` is needed and it runs as you. (systemd user services start at login; enable linger — `loginctl enable-linger $USER` — if you want it running without an active session.) @@ -273,7 +277,7 @@ dig-node status --json # {"ok":true,"action":"status","service":"dig-node","version":"0.5.0","serving":false,"addr":"127.0.0.1:9778",…} dig-node install --json -# {"ok":true,"action":"install","installed":true,"registered":true,"started":false,"label":"…","scope":"system","addr":"127.0.0.1:9778",…} +# {"ok":true,"action":"install","installed":true,"reinstalled":false,"registered":true,"started":false,"label":"…","display_name":"DIG NETWORK: NODE","scope":"system","addr":"127.0.0.1:9778",…} ``` On failure: `{ "ok":false, "action":…, "error":{ "code", "exit_code", "message", "hint" } }`. diff --git a/SPEC.md b/SPEC.md index 51fffcf..dac6066 100644 --- a/SPEC.md +++ b/SPEC.md @@ -181,7 +181,12 @@ does not own them (except `DIG_NODE_UPSTREAM`, which the shell SETS — see belo | `DIG_NODE_MAX_OUTGOING_BYTES_PER_SEC` | outgoing-bandwidth throttle cap, in bytes/second (§17) | `0` (UNLIMITED — opt-in) | Parsed as `u64`; `0`, unparsable, or unset ⇒ unlimited (the throttle is a no-op until an operator configures a cap). Resolved ONCE at node construction. | The peer-network layer additionally honors `DIG_PEER_NETWORK` (set to a falsy value to disable the L7 -peer network) and `DIG_RELAY_URL` (override or disable the relay), which gate the P2P bring-up. +peer network) and `DIG_RELAY_URL` (override or disable the relay), which gate the P2P bring-up, and +**`DIG_PEER_PORT`** — the P2P listen port for the mTLS peer-RPC server (dig-node-to-dig-node +traffic, §5.2/§14 — distinct from the client-facing `DIG_NODE_PORT` JSON-RPC listener above). +Parsed as `u16`; unparsable/unset ⇒ the default **`9444`** (`peer::DEFAULT_P2P_PORT`, matching +dig-gossip's own `DEFAULT_P2P_PORT` so a node and its gossip peers agree on the conventional port +with no configuration). Bound dual-stack IPv6-first with an IPv4 fallback, per §5.2. ### 3.3. Upstream normalization @@ -1198,6 +1203,33 @@ up on its own, not sit stopped until a human restarts it: `result.recovery_configured: false` in `--json` output (`true` otherwise, and always `true` on Linux/macOS since their defaults already apply). +9.2b. **Display name + clean-reinstall (`install`, #494).** `install` is a stop→delete→wait→create +CLEAN-REINSTALL, never a reconfigure-in-place, so re-running it against an already-registered +service does not hit Windows `CreateService` error 1073 ("the specified service already exists"): + +- If the service is not yet registered, `install` simply creates it. +- If it IS already registered, `install` best-effort stops it, deletes (deregisters) it, polls for + the deregistration to actually take effect (bounded, `TimedOut` if it never does — a lingering + Windows deletion can hold on until open handles close), and only THEN recreates it. +- **`install` never starts the service** (fresh or reinstalled) — it only registers + `autostart: true` for the next boot/login. A caller starts it explicitly with `dig-node start`. + This is deliberate: the dig-installer calls `install` then, when configured to start it, a + SEPARATE `start` and treats a `start` failure as fatal for that step; if `install` also started + the service, that second `start` would hit "already running" and could flip the installer's + reported outcome to failed even though the service is up. +- **Windows display name.** `sc create` (via `service-manager`) always sets the SCM display name to + the service id; `install` follows it with `sc config displayname= "DIG NETWORK: NODE"`, then + reads the config back with `sc qc ` to CONFIRM the override took (rather than trusting the + `sc config` exit code alone). Both steps are best-effort — a failure leaves the service + registered and usable, just possibly showing its id instead of the friendly name in the Services + console; `result.display_name_verified` (`--json`) reports whether the read-back confirmed it. +- **macOS/Linux friendly name.** launchd has no display-name-equivalent plist key, so the daemon is + identified by its `Label` (`net.dignetwork.dig-node`) only. systemd's `Description=` DOES carry a + friendly name; the native `.deb`'s STATIC unit file (§9.7) already sets + `Description=DIG NETWORK: NODE`. A bare `dig-node install` (not via the `.deb`) registers a + service-manager-generated unit whose `Description` is the service id, matching `dig-dns`'s own + established precedent for the CLI-only path. + 9.3. **Entrypoint per platform.** The installed service runs `dig-node run-service` on Windows and `dig-node run` on systemd/launchd (which exec the foreground process directly). diff --git a/crates/dig-node-service/Cargo.toml b/crates/dig-node-service/Cargo.toml index 54d4b08..e9daea1 100644 --- a/crates/dig-node-service/Cargo.toml +++ b/crates/dig-node-service/Cargo.toml @@ -66,6 +66,21 @@ service-manager = "0.7" # system OpenSSL), same as the node + store crates. reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } +# `dig_constants::DIG_NODE_PORT` (§5.3) — the single-sourced canonical default localhost port +# for client→node connections; `config.rs`'s DEFAULT_PORT reads it rather than re-declaring the +# literal. Rev-PINNED deliberately, to a DISTINCT source from dig-node-core's own bare-git +# `dig-constants = { version = "0.2", git = "..." }` (which the P2P stack — dig-nat/dig-gossip/ +# dig-dht/dig-onion — is release-ordered to stay on until 0.2.x reaches crates.io): a bare-git +# dependency has no `rev`, so cargo unifies EVERY bare-git reference to that URL into ONE +# resolved version, and ^0.2 + ^0.3 cannot both be satisfied by a single instance. Pinning this +# crate's dependency to the exact v0.3.0 commit makes it a cargo-DISTINCT source (a `rev`-pinned +# git dependency never unifies with a bare one, even at the identical commit), so this crate gets +# its own 0.3.0 instance without forcing dig-node-core's 0.2.x pin forward. Safe because the only +# thing crossing the boundary is a plain `u16` constant — the two instances never need to +# interoperate. Bump the `rev` (and the `version` req) when dig-node-core's own pin moves past +# 0.3.x and unification becomes possible again. +dig-constants = { version = "0.3", git = "https://github.com/DIG-Network/dig-constants", rev = "438ac8caf9ffb80cadd10362deacd63763a7f141" } + # Windows Service Control Protocol. service-manager only REGISTERS the service in the # SCM; the binary the SCM launches must itself speak the service protocol # (StartServiceCtrlDispatcher → report RUNNING/STOPPED) or the SCM kills it with error diff --git a/crates/dig-node-service/src/config.rs b/crates/dig-node-service/src/config.rs index 0e88367..65f0445 100644 --- a/crates/dig-node-service/src/config.rs +++ b/crates/dig-node-service/src/config.rs @@ -44,7 +44,11 @@ use std::net::{IpAddr, Ipv4Addr}; /// consumer of the §5.3 `localhost` tier (the extension's `server.host` default, the /// installer, the DIG Browser) MUST target `9778` to match. `DIG_NODE_PORT` overrides /// it. (`dig.local` on `127.0.0.2:80` is unaffected — only this localhost port moves.) -pub const DEFAULT_PORT: u16 = 9778; +/// +/// Single-sourced from the shared `dig-constants` crate (`DIG_NODE_PORT`) rather than +/// re-declared here, so every §5.3 client→node consumer that also imports the constant agrees +/// with this service byte-for-byte with no copy to drift. +pub const DEFAULT_PORT: u16 = dig_constants::DIG_NODE_PORT; /// Default upstream DIG RPC the embedded node proxies to on a local cache miss. pub const DEFAULT_UPSTREAM: &str = "https://rpc.dig.net"; diff --git a/crates/dig-node-service/src/service.rs b/crates/dig-node-service/src/service.rs index e3e371b..b1ee961 100644 --- a/crates/dig-node-service/src/service.rs +++ b/crates/dig-node-service/src/service.rs @@ -7,6 +7,39 @@ //! removes it; `start`/`stop` control the registered service; `status` reports //! whether it is registered and actually serving. //! +//! This module owns the service IDENTITY and the **clean-reinstall** contract +//! (mirrors the sibling `dig-dns` service module): +//! +//! * **Service id** — [`SERVICE_LABEL`] `net.dignetwork.dig-node`, the reverse-DNS name used +//! verbatim as the Windows SCM service name (`sc create`/`query`/`start`/`stop`/`delete`) and +//! the launchd plist label — `ServiceLabel::to_qualified_name()`. On **systemd** the actual +//! registered unit name is DIFFERENT: `service-manager`'s systemd backend derives it from +//! `to_script_name()` instead (`dignetwork-dig-node`, dropping the `net` qualifier) — see +//! [`os_native_service_name`], which a real 3-OS CI run proved MUST be used for any direct +//! existence probe (getting this wrong silently defeats clean-reinstall on Linux, #494). +//! Distinct from [`crate::meta::SERVICE_NAME`] (`"dig-node"`, the RPC/build-info identity) — +//! the two never need to agree. +//! * **Display name** — [`SERVICE_DISPLAY_NAME`] "DIG NETWORK: NODE", the human-friendly name +//! shown in the Windows Services console (set with `sc config … displayname=` after create, +//! because `service-manager` 0.7's `sc create` hardcodes the display name to the service id — +//! see [`SystemServiceBackend::create`]), then read back with `sc qc` to verify the override +//! actually took (see [`query_windows_display_name`]). The native macOS/Linux packages +//! (`packaging/macos`, `packaging/linux`) carry the same friendly name via their own static +//! unit files (the systemd unit's `Description=`; launchd has no equivalent display-name key, +//! so the plist's `Label` — already `net.dignetwork.dig-node` — is the only OS-visible name). +//! * **Clean-reinstall** — [`reinstall`]: if the service ALREADY EXISTS, **stop → delete +//! (deregister) → wait for removal → (re)create** — a clean recreate, never a +//! reconfigure-in-place. This is what avoids Windows `CreateService 1073 "the specified +//! service already exists"` on a re-run of `dig-node install`. +//! +//! **`install` never auto-starts** (deliberately unlike `dig-dns`'s equivalent): the +//! dig-installer's `register_dig_node` step calls `dig-node install` and then, when +//! configured to start it, a SEPARATE `dig-node start` — and treats a `start` failure as a +//! hard error for that step. If `install` also started the service, that second `start` would +//! hit "service already running" (SCM 1056 / a systemd/launchd no-op-or-error depending on +//! backend) and could flip the installer's reported `installed` status to `false` even though +//! the service is up. So `reinstall` here stops at **create** — a caller starts it explicitly. +//! //! Install level by platform: //! * Linux (systemd) / macOS (launchd) — **user-level** by default (`--user` / //! `gui` domain), so no root/sudo is needed and the service runs as the @@ -15,9 +48,18 @@ //! per-user services, so `install`/`uninstall` require an **elevated //! (Administrator)** console. This is detected up front and reported with a //! clear message rather than failing deep inside `sc.exe`. +//! +//! The OS calls are behind the [`ServiceBackend`] trait so the clean-reinstall ORDER is +//! unit-tested against a recording mock — CI never shells out to `sc`/`launchctl`/`systemctl` +//! for that part; a real 3-OS install/uninstall round-trip is exercised by the +//! `service-smoke` CI job (`.github/workflows/service-smoke.yml`). +use std::cell::Cell; use std::ffi::OsString; +use std::io; +use std::path::PathBuf; use std::str::FromStr; +use std::time::Duration; use serde_json::json; use service_manager::{ @@ -36,6 +78,20 @@ use crate::config::Config; /// recovery-action config below) all address the same service. pub const SERVICE_LABEL: &str = "net.dignetwork.dig-node"; +/// The human-friendly display name shown in the Windows Services console. On launchd/systemd +/// the service id IS the visible name (systemd's own `Description=` carries the friendly text +/// on the native `.deb`/`.pkg` install — see the module doc), so this constant is primarily a +/// Windows-facing label. +pub const SERVICE_DISPLAY_NAME: &str = "DIG NETWORK: NODE"; + +/// How many times [`reinstall`] polls for a deleted service to disappear before giving up. A +/// Windows service marked for deletion (`sc delete`) can linger until its open handles close; +/// `40 × 500ms = 20s` is generous for a loopback node with no long-lived clients. +const REMOVAL_POLL_ATTEMPTS: u32 = 40; + +/// The interval between removal polls (see [`REMOVAL_POLL_ATTEMPTS`]). +const REMOVAL_POLL_INTERVAL: Duration = Duration::from_millis(500); + /// Whether user-level (no-elevation) install is supported on this OS. Windows SCM /// is system-only; systemd/launchd support a user domain. #[cfg(windows)] @@ -43,58 +99,11 @@ const PREFERS_USER_LEVEL: bool = false; #[cfg(not(windows))] const PREFERS_USER_LEVEL: bool = true; -/// Build the parsed service label (infallible for our constant, but the crate -/// returns a Result, so surface a clear error if the constant is ever mis-edited). -fn label() -> std::io::Result { - ServiceLabel::from_str(SERVICE_LABEL) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e.to_string())) -} - -/// Acquire the native service manager, set to user-level where the platform -/// supports it (Linux/macOS), else system-level (Windows). Returns the manager -/// plus whether it is operating at user level (for messaging). -fn manager() -> std::io::Result<(Box, bool)> { - let mut mgr = ::native()?; - let mut user_level = false; - if PREFERS_USER_LEVEL && mgr.set_level(ServiceLevel::User).is_ok() { - user_level = true; - } - Ok((mgr, user_level)) -} - -/// Absolute path to the currently-running `dig-node` executable, so the -/// installed service points at THIS binary (not a PATH lookup that might resolve -/// to a different/absent copy). -fn current_exe() -> std::io::Result { - std::env::current_exe() -} - -/// On Windows, is this process running elevated (Administrator)? Used to fail -/// `install`/`uninstall` early with a helpful message instead of a cryptic SCM -/// access-denied. Always `true` off Windows (those paths are user-level). -#[cfg(windows)] -fn is_elevated() -> bool { - // Probe by attempting to open the SCM with all-access; only an elevated token - // can. Shelling to `net session` is the classic check; doing it via `sc` query - // would not distinguish. Use a lightweight `net session` invocation. - std::process::Command::new("net") - .arg("session") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .map(|s| s.success()) - .unwrap_or(false) -} -#[cfg(not(windows))] -fn is_elevated() -> bool { - true -} - -// These three items back the Windows-only recovery path ([`configure_windows_recovery`] -// + the note in [`install`]), but are deliberately kept platform-INDEPENDENT so the -// pure argument-building is unit-tested on EVERY CI runner (the coverage/test job runs -// on Linux). Their only non-test consumer is `#[cfg(windows)]`, so off Windows a -// non-test build sees them as unused — silence that one targeted case rather than +// These recovery-action items back the Windows-only crash-restart path +// ([`configure_windows_recovery`] + the note in [`install`]), but are deliberately kept +// platform-INDEPENDENT so the pure argument-building is unit-tested on EVERY CI runner (the +// coverage/test job runs on Linux). Their only non-test consumer is `#[cfg(windows)]`, so off +// Windows a non-test build sees them as unused — silence that one targeted case rather than // gate them (which would drop the Linux CI coverage of the builder). /// `sc.exe failure` recovery-action config: reset the failure counter after one @@ -125,14 +134,14 @@ fn recovery_action_args(service_name: &str) -> Vec { /// Register Windows SCM recovery actions (restart-on-crash) for the installed /// service. `service-manager`'s `sc.rs` backend only shells out to `sc create` -/// (§`install`) — it never configures `SERVICE_CONFIG_FAILURE_ACTIONS`, and the -/// pinned `windows-service` 0.7 crate exposes no `ChangeServiceConfig2` binding +/// (§`SystemServiceBackend::create`) — it never configures `SERVICE_CONFIG_FAILURE_ACTIONS`, +/// and the pinned `windows-service` 0.7 crate exposes no `ChangeServiceConfig2` binding /// either, so Windows services do NOT restart on crash unless this is set /// explicitly (unlike systemd/launchd, which `service-manager` already covers by -/// default). Call ONLY after a successful `mgr.install`; the caller treats a +/// default). Call ONLY after a successful [`reinstall`]; the caller treats a /// failure here as non-fatal (see [`install`]). #[cfg(windows)] -fn configure_windows_recovery(service_name: &str) -> std::io::Result<()> { +fn configure_windows_recovery(service_name: &str) -> io::Result<()> { let args = recovery_action_args(service_name); let output = std::process::Command::new("sc.exe") .args(&args) @@ -148,34 +157,128 @@ fn configure_windows_recovery(service_name: &str) -> std::io::Result<()> { } else { msg }; - Err(std::io::Error::other(msg)) + Err(io::Error::other(msg)) } } -/// Install dig-node as an auto-starting OS service that runs -/// `dig-node run` on the configured loopback port. The service's environment -/// carries the resolved port/host/upstream so it serves identically to a manual -/// `run`. On Windows, also configures SCM recovery actions (restart-on-crash) — -/// see [`configure_windows_recovery`] — so a crashed service comes back up the -/// same way systemd (`Restart=on-failure`) and launchd (`KeepAlive`) already do -/// for Linux/macOS via `service-manager`'s own defaults. -pub fn install(config: &Config) -> std::io::Result { - if cfg!(windows) && !is_elevated() { - return Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - "dig-node: installing a Windows service requires an elevated \ - (Administrator) console. Re-run this in a terminal opened with \ - \"Run as administrator\".", - )); +// --------------------------------------------------------------------------------------------- +// The clean-reinstall contract: a pure plan + a backend trait + the stop/delete/wait/create +// orchestration, unit-tested end-to-end with a recording mock (no real OS service involved). +// --------------------------------------------------------------------------------------------- + +/// What to register: the service identity + the program the SCM/launchd/systemd runs, plus the +/// environment that reproduces the resolved [`Config`] so the installed service serves +/// identically to a manual `dig-node run`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct InstallPlan { + /// The reverse-DNS service id ([`SERVICE_LABEL`]). + pub label: String, + /// The Windows display name ([`SERVICE_DISPLAY_NAME`]). + pub display_name: String, + /// Absolute path to the program the service runs (this `dig-node` binary). + pub program: PathBuf, + /// Arguments passed to `program` (`run-service` on Windows, else `run`). + pub args: Vec, + /// Environment variables baked into the service so it resolves the SAME config the + /// installing invocation did (the service does not inherit the installer's shell env). + pub environment: Vec<(String, String)>, + /// Whether the service auto-starts on boot/login (registration flag — distinct from being + /// started NOW; see the module doc's "`install` never auto-starts" note). + pub autostart: bool, +} + +/// The OS-service backend: the four primitive operations the clean-reinstall composes. Behind a +/// trait so [`reinstall`]'s ORDER (stop → delete → wait → create) is unit-tested with a +/// recording mock and CI never registers a real service. The real implementation is +/// [`SystemServiceBackend`]. +pub trait ServiceBackend { + /// Is the service currently registered with the OS service manager? + fn is_installed(&self) -> io::Result; + /// Stop the running service (best-effort at the call site: a not-running service is not an + /// error the caller must fail on). + fn stop(&self) -> io::Result<()>; + /// Deregister (delete) the service from the OS service manager. + fn delete(&self) -> io::Result<()>; + /// Register (create) the service from `plan`, including the display name on Windows. + fn create(&self, plan: &InstallPlan) -> io::Result<()>; +} + +/// What [`reinstall`] did, for machine-readable + human output. `existed` records whether a +/// prior registration was found (⇒ the stop/delete/wait clean-recreate ran); a fresh install +/// leaves `existed`/`stopped`/`deleted` false. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct ReinstallReport { + /// A prior registration existed, so the clean-recreate path ran. + pub existed: bool, + /// The existing service was stopped before deletion. + pub stopped: bool, + /// The existing service was deleted (deregistered). + pub deleted: bool, + /// The service was (re)created. + pub created: bool, +} + +/// **Clean-reinstall.** If the service ALREADY EXISTS: stop it (best-effort), delete +/// (deregister) it, wait for the removal to take effect, THEN (re)create it with the display +/// name — a clean recreate, NEVER a reconfigure-in-place. When no prior registration exists it +/// simply creates. Deliberately does NOT start the service either way — see the module doc. +/// +/// This ordering is the fix for Windows `CreateService 1073 "the specified service already +/// exists"`: by deleting before creating, `create` never targets an existing service. +pub fn reinstall( + backend: &B, + plan: &InstallPlan, +) -> io::Result { + let mut report = ReinstallReport::default(); + + if backend.is_installed()? { + report.existed = true; + // Stop is best-effort: a registered-but-already-stopped service errors on stop, and + // that must not block the delete + recreate that follows. + if backend.stop().is_ok() { + report.stopped = true; + } + backend.delete()?; + report.deleted = true; + wait_for_removal(backend)?; } - let (mgr, user_level) = manager()?; - let program = current_exe()?; - let svc_label = label()?; + backend.create(plan)?; + report.created = true; + Ok(report) +} - // Pass the effective config to the service as env vars so the running service - // matches what `install` was told (the service process does not inherit the - // installing shell's environment). +/// Poll [`ServiceBackend::is_installed`] until the service is gone, bounded by +/// [`REMOVAL_POLL_ATTEMPTS`]. Checks BEFORE sleeping, so a backend that removes synchronously +/// (the test mock, and systemd/launchd) returns immediately with no delay; only a lingering +/// Windows deletion actually waits. Errors with `TimedOut` if the service is still present +/// after the window, so a caller never blindly recreates onto a still-existing service (1073). +fn wait_for_removal(backend: &B) -> io::Result<()> { + for _ in 0..REMOVAL_POLL_ATTEMPTS { + if !backend.is_installed()? { + return Ok(()); + } + std::thread::sleep(REMOVAL_POLL_INTERVAL); + } + Err(io::Error::new( + io::ErrorKind::TimedOut, + format!( + "dig-node: service \"{SERVICE_LABEL}\" was deleted but is still present after \ + waiting for removal; cannot cleanly recreate it (a handle may be held open — \ + close the Services console and retry)" + ), + )) +} + +/// Build the [`InstallPlan`] for `program` from a resolved [`Config`]. PURE (given the program +/// path), so the identity + args + baked environment are unit-tested without touching the OS. +/// The installed service runs `run-service` on Windows (the SCM protocol entrypoint) and `run` +/// elsewhere (systemd/launchd exec the foreground process directly). +pub fn build_plan(config: &Config, program: PathBuf) -> InstallPlan { + let entry_arg = if cfg!(windows) { "run-service" } else { "run" }; + + // Bake the resolved config into the service environment so it serves identically to the + // invocation that installed it (a service does not inherit the installing shell's env). let mut environment = vec![ ("DIG_NODE_PORT".to_string(), config.port.to_string()), ("DIG_RPC_UPSTREAM".to_string(), config.upstream.clone()), @@ -188,6 +291,337 @@ pub fn install(config: &Config) -> std::io::Result { crate::state::RUN_CONTEXT_SERVICE.to_string(), ), ]; + // Only record DIG_NODE_HOST when the operator gave an EXPLICIT override + // (#288): omitting it lets the installed service resolve the same default the + // CLI would — bind BOTH loopback families (127.0.0.1 AND [::1], §5.2) — + // instead of freezing today's IPv4-only default into the service's + // environment forever. An operator who set DIG_NODE_HOST before `dig-node + // install` still gets that exact override carried into the service. + if let Some(host) = config.host { + environment.push(("DIG_NODE_HOST".to_string(), host.to_string())); + } + // Only record DIG_NODE_CACHE when an explicit dir was set: omitting it lets the + // service resolve dig-node's shared canonical default — the SAME dir the DIG + // Browser's in-process node uses — so the two share ONE cache (#96). Recording + // a path here pins the service to it, so an operator pointing the service at a + // dedicated cache must set the SAME path for the browser to keep sharing. + if let Some(dir) = crate::config::cache_dir_env_value(config.cache_dir.as_deref()) { + environment.push(("DIG_NODE_CACHE".to_string(), dir)); + } + + InstallPlan { + label: SERVICE_LABEL.to_string(), + display_name: SERVICE_DISPLAY_NAME.to_string(), + program, + args: vec![OsString::from(entry_arg)], + environment, + autostart: true, + } +} + +/// Build the `sc.exe config displayname= ""` argument list that overrides the +/// Windows service display name after `service-manager`'s `sc create` (which sets it to the +/// service id). PURE (no process spawn) so the argument construction is unit-testable without +/// invoking `sc.exe`. +#[cfg_attr(not(windows), allow(dead_code))] +fn display_name_config_args(service_name: &str, display_name: &str) -> Vec { + vec![ + "config".to_string(), + service_name.to_string(), + "displayname=".to_string(), + display_name.to_string(), + ] +} + +/// Parse the `DISPLAY_NAME` field out of `sc.exe qc ` output (the read-back verify for +/// [`SystemServiceBackend::create`]'s display-name override). PURE (no process spawn) so the +/// parsing is unit-tested without invoking `sc.exe`. Typical `sc qc` output: +/// +/// ```text +/// SERVICE_NAME: net.dignetwork.dig-node +/// TYPE : 10 WIN32_OWN_PROCESS +/// ... +/// DISPLAY_NAME : DIG NETWORK: NODE +/// ``` +/// +/// Splits on the FIRST `:` only, so a display name that itself contains a colon (this one does: +/// "DIG NETWORK: NODE") is not truncated. +#[cfg_attr(not(windows), allow(dead_code))] +fn parse_sc_qc_display_name(output: &str) -> Option<&str> { + output.lines().find_map(|line| { + let (key, value) = line.split_once(':')?; + key.trim() + .eq_ignore_ascii_case("DISPLAY_NAME") + .then(|| value.trim()) + }) +} + +// --------------------------------------------------------------------------------------------- +// The real, OS-backed backend + the CLI-facing install/uninstall/start/stop/status commands. +// --------------------------------------------------------------------------------------------- + +/// Build the parsed service label (infallible for our constant, but the crate +/// returns a Result, so surface a clear error if the constant is ever mis-edited). +fn label() -> io::Result { + ServiceLabel::from_str(SERVICE_LABEL) + .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e.to_string())) +} + +/// Absolute path to the currently-running `dig-node` executable, so the +/// installed service points at THIS binary (not a PATH lookup that might resolve +/// to a different/absent copy). +fn current_exe() -> io::Result { + std::env::current_exe() +} + +/// On Windows, is this process running elevated (Administrator)? Used to fail +/// `install`/`uninstall` early with a helpful message instead of a cryptic SCM +/// access-denied. Always `true` off Windows (those paths are user-level). +#[cfg(windows)] +fn is_elevated() -> bool { + // Probe by attempting to open the SCM with all-access; only an elevated token + // can. Shelling to `net session` is the classic check; doing it via `sc` query + // would not distinguish. Use a lightweight `net session` invocation. + std::process::Command::new("net") + .arg("session") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) +} +#[cfg(not(windows))] +fn is_elevated() -> bool { + true +} + +/// The real [`ServiceBackend`]: the native OS service manager (user-level on Linux/macOS, +/// system-level on Windows) plus the OS existence probe and the Windows display-name override +/// + read-back verify. +pub struct SystemServiceBackend { + label: ServiceLabel, + manager: Box, + /// Whether the manager is operating at user level (Linux/macOS) — surfaced for messaging. + user_level: bool, + /// Windows-only: whether the post-create `sc qc` read-back confirmed the display name was + /// actually applied. `None` off Windows (nothing to verify) or before a `create` has run. + display_name_verified: Cell>, +} + +impl SystemServiceBackend { + /// Acquire the native service manager, set to user-level where the platform supports it. + pub fn new() -> io::Result { + let mut manager = ::native()?; + let mut user_level = false; + if PREFERS_USER_LEVEL && manager.set_level(ServiceLevel::User).is_ok() { + user_level = true; + } + Ok(Self { + label: label()?, + manager, + user_level, + display_name_verified: Cell::new(None), + }) + } + + /// Whether this backend installs at user level (no elevation) vs system level. + pub fn user_level(&self) -> bool { + self.user_level + } + + /// Windows: whether the `sc qc` read-back confirmed the display-name override took effect. + /// `None` off Windows, or if [`ServiceBackend::create`] has not run yet. + pub fn display_name_verified(&self) -> Option { + self.display_name_verified.get() + } + + /// Start the registered service. + fn start(&self) -> io::Result<()> { + self.manager.start(ServiceStartCtx { + label: self.label.clone(), + }) + } +} + +impl ServiceBackend for SystemServiceBackend { + fn is_installed(&self) -> io::Result { + Ok(query_installed(&os_native_service_name(&self.label))) + } + + fn stop(&self) -> io::Result<()> { + self.manager.stop(ServiceStopCtx { + label: self.label.clone(), + }) + } + + fn delete(&self) -> io::Result<()> { + self.manager.uninstall(ServiceUninstallCtx { + label: self.label.clone(), + }) + } + + fn create(&self, plan: &InstallPlan) -> io::Result<()> { + self.manager.install(ServiceInstallCtx { + label: self.label.clone(), + program: plan.program.clone(), + args: plan.args.clone(), + contents: None, + username: None, + working_directory: None, + environment: Some(plan.environment.clone()), + autostart: plan.autostart, + })?; + // service-manager's `sc create` sets the display name to the service id; override it + // with the human-friendly name, then read it back with `sc qc` to confirm the override + // actually took (rather than trusting a silent `sc config` exit code). Both steps are + // best-effort: a failure leaves the service installed + working, just showing the id + // (or an unconfirmed display) in the Services console. + #[cfg(windows)] + { + let qualified = self.label.to_qualified_name(); + set_windows_display_name(&qualified, &plan.display_name); + let verified = query_windows_display_name(&qualified) + .ok() + .flatten() + .is_some_and(|actual| actual == plan.display_name); + self.display_name_verified.set(Some(verified)); + } + Ok(()) + } +} + +/// The identifier [`query_installed`] must probe the OS with — the SAME identifier +/// `service-manager`'s own backend registers the service under, which is **NOT uniformly +/// [`ServiceLabel::to_qualified_name`]**: `service-manager`'s Windows (`sc.rs`) and launchd +/// (`launchd.rs`) backends register under `to_qualified_name()` (the reverse-DNS +/// `net.dignetwork.dig-node`), but its **systemd** backend (`systemd.rs`) derives the unit file +/// name from `to_script_name()` instead — `{organization}-{application}` (`dignetwork-dig-node`, +/// dropping the `net` qualifier entirely). Probing systemd with the qualified name looks for a +/// unit that never exists, so `is_installed` always reports `false` there, silently defeating +/// the whole clean-reinstall contract (caught by the `service-smoke` CI job on `ubuntu-latest`: +/// the "install a second time" step landed `reinstalled:false` instead of `true`). +fn os_native_service_name(label: &ServiceLabel) -> String { + if cfg!(all(unix, not(target_os = "macos"))) { + label.to_script_name() + } else { + label.to_qualified_name() + } +} + +/// Probe whether a service named `service_name` is registered, per OS. Best-effort: a probe +/// that cannot run (tool missing) reports `false` so the clean-reinstall proceeds to create. +#[cfg(windows)] +fn query_installed(service_name: &str) -> bool { + // `sc query ` exits 0 when the service exists, 1060 (does-not-exist) otherwise. + std::process::Command::new("sc.exe") + .args(["query", service_name]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) +} + +/// macOS launchd existence probe: `launchctl print /