fix: some issues related to sudo supervisor

Cargo.toml

@@ -105,7 +105,7 @@ listeners = "0.5"
 [target.'cfg(unix)'.dependencies]
 exec = "0.3"
 libc = "0.2"
-nix = { version = "0.31", features = ["signal", "process"] }
+nix = { version = "0.31", features = ["signal", "process", "user"] }
 
 [build-dependencies]
 toml = "1.0"

docs/guides/port-management.md

@@ -4,8 +4,6 @@ Pitchfork provides smart port management and an optional reverse proxy that give
 
 ## Port Assignment
 
-### Expected Ports
-
 Configure the ports your daemon expects to use:
 
 ```toml
@@ -14,44 +12,26 @@ run = "node server.js"
 expected_port = [3000]
 ```
 
-Pitchfork will:
-1. Check if port 3000 is available before starting
-2. Inject `PORT=3000` into the daemon's environment
-3. Fail with a clear error if the port is already in use
+Pitchfork checks if the port is available before starting, injects `PORT=3000` into the daemon's environment, and fails with a clear error if the port is already in use.
 
 ### Auto Port Bumping
 
-When a port is occupied, pitchfork can automatically find the next available port:
+When a port is occupied, enable `auto_bump_port` to automatically find the next available port:
 
 ```toml
 [daemons.api]
 run = "node server.js"
 expected_port = [3000]
 auto_bump_port = true
+port_bump_attempts = 10   # default: 3
 ```
 
-With `auto_bump_port = true`, pitchfork tries 3000, 3001, 3002, ... until it finds a free port. The daemon receives the actual port via `$PORT`.
-
-Control how many attempts are made:
-
-```toml
-[daemons.api]
-run = "node server.js"
-expected_port = [3000]
-auto_bump_port = true
-port_bump_attempts = 10   # try up to 10 ports (default: 3)
-```
-
-Or via environment variable:
-```bash
-PITCHFORK_PORT_BUMP_ATTEMPTS=10 pitchfork start api
-```
+The daemon receives the actual allocated port via `$PORT`.
 
 ### Active Port Tracking
 
-After a daemon starts, pitchfork detects which port the process is actually listening on. This detected port is the source of truth for the reverse proxy — it's what gets routed when you access the proxy URL.
+After a daemon starts, pitchfork detects the port the process is actually listening on. This detected port is the source of truth for the reverse proxy.
 
----
 
 ## Reverse Proxy
 
@@ -133,12 +113,6 @@ frontend = { dir = "/home/user/my-app", daemon = "dev" }
 docs = { dir = "/home/user/docs-site" }  # defaults daemon = "docs"
 ```
 
-**Why global config only?**
-- **Single source of truth** — no sync step needed, no risk of stale state
-- **Cross-directory resolution** — slugs work from any directory
-- **Explicit and auditable** — one file shows all your proxied services
-- **Auto-start ready** — the proxy knows the dir and can load project config automatically
-
 ### URL format
 
 Proxy URLs use this shape:
@@ -168,8 +142,6 @@ pitchfork proxy remove api
 pitchfork proxy status
 ```
 
----
-
 ## Standard Ports (80/443)
 
 To use standard HTTP/HTTPS ports without the port number in URLs:
@@ -203,7 +175,6 @@ https = false
 Binding to ports below 1024 (including 80 and 443) requires the supervisor to be started with `sudo`. The proxy will fail to start if it cannot bind to the configured port.
 :::
 
----
 
 ## HTTPS Support
 
@@ -267,8 +238,6 @@ tls_cert = "/path/to/_wildcard.localhost+2.pem"
 tls_key = "/path/to/_wildcard.localhost+2-key.pem"
 ```
 
----
-
 ## Custom TLD
 
 Use a custom TLD instead of `localhost`:
@@ -300,7 +269,6 @@ sudo brew services start dnsmasq
 Later we will add built-in support for custom TLDs without manual DNS configuration.
 :::
 
----
 
 ## Proxy Commands
 
@@ -339,6 +307,8 @@ The entire auto-start operation — including waiting for the daemon's readiness
 auto_start_timeout = "60s"
 ```
 
+---
+
 ## Viewing Proxy URLs
 
 Proxy URLs are shown in CLI output when the proxy is enabled and the daemon has a registered slug:

docs/reference/file-locations.md

@@ -2,6 +2,18 @@
 
 Where pitchfork stores its files.
 
+## Directory Resolution
+
+Pitchfork resolves key directories as follows:
+
+| Directory | Resolution Order |
+|-----------|-----------------|
+| **Home** | `SUDO_USER`'s home (when euid=0) → `dirs::home_dir()` → `/tmp` |
+| **Config** | `PITCHFORK_CONFIG_DIR` env → `~/.config/pitchfork` |
+| **State** | `PITCHFORK_STATE_DIR` env → (sudo) `~/.local/state/pitchfork` · (non-sudo) `dirs::state_dir()/pitchfork` → `~/.local/state/pitchfork` |
+
+> **Note:** Under `sudo` (euid=0), the home directory (`~`) is resolved from `SUDO_USER` via the system password database, and `dirs::state_dir()` is bypassed to ensure all paths stay consistent with non-sudo invocations. On macOS `dirs::state_dir()` returns `None`, so the fallback `~/.local/state` is always used.
+
 ## Configuration Files
 
 Pitchfork supports configuration files in multiple locations. Files are merged in order, with later files overriding earlier ones.

src/cli/proxy.rs

@@ -118,8 +118,8 @@ fn install_cert(cert_path: &std::path::Path) -> Result<()> {
     use std::process::Command;
 
     // Resolve the login keychain path for the current user.
-    let home = std::env::var("HOME").map_err(|_| miette::miette!("$HOME is not set"))?;
-    let keychain = format!("{home}/Library/Keychains/login.keychain-db");
+    let home = &*crate::env::HOME_DIR;
+    let keychain = format!("{}/Library/Keychains/login.keychain-db", home.display());
 
     // Install into the current user's login keychain — no sudo required.
     // Must specify -k explicitly; without it macOS targets the admin domain

src/cli/supervisor/mod.rs

@@ -1,11 +1,24 @@
 use crate::Result;
+use crate::daemon_id::DaemonId;
+use crate::env;
 use crate::procs::PROCS;
+use crate::state_file::StateFile;
 
 mod run;
 mod start;
 mod status;
 mod stop;
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum KillOrStopOutcome {
+    /// Process was actively killed.
+    Killed,
+    /// PID was in the state file but the process was already dead.
+    AlreadyDead,
+    /// Existing process is running and --force was not passed.
+    StillRunning,
+}
+
 /// Start, stop, and check the status of the pitchfork supervisor daemon
 #[derive(Debug, clap::Args)]
 #[clap(visible_alias = "sup", verbatim_doc_comment)]
@@ -33,21 +46,41 @@ impl Supervisor {
     }
 }
 
-/// if --force is passed, will kill existing process
-/// Returns false if existing pid is running and --force was not passed (so we should cancel starting the daemon)
-pub async fn kill_or_stop(existing_pid: u32, force: bool) -> Result<bool> {
+/// If `force` is true, kills the existing process.
+/// Returns `KillOrStopOutcome::StillRunning` when the process is alive and `force` is false.
+///
+/// This is a low-level helper — callers are responsible for user-facing messages.
+pub async fn kill_or_stop(existing_pid: u32, force: bool) -> Result<KillOrStopOutcome> {
     if PROCS.is_running(existing_pid) {
         if force {
             debug!("killing pid {existing_pid}");
-            PROCS.kill_async(existing_pid).await?;
-            Ok(true)
+            match PROCS.kill_async(existing_pid).await {
+                Ok(true) => Ok(KillOrStopOutcome::Killed),
+                Ok(false) => Ok(KillOrStopOutcome::AlreadyDead),
+                Err(e) => Err(miette::miette!("{e}. Try rerun with sudo.")),
+            }
         } else {
-            warn!(
-                "pitchfork supervisor is already running with pid {existing_pid}. Kill it with `--force`"
-            );
-            Ok(false)
+            Ok(KillOrStopOutcome::StillRunning)
         }
     } else {
-        Ok(true)
+        Ok(KillOrStopOutcome::AlreadyDead)
     }
 }
+
+pub fn existing_supervisor_pid() -> Result<Option<u32>> {
+    let sf = StateFile::read(&*env::PITCHFORK_STATE_FILE)?;
+    Ok(sf
+        .daemons
+        .get(&DaemonId::pitchfork())
+        .and_then(|daemon| daemon.pid))
+}
+
+pub async fn resolve_existing_supervisor(force: bool) -> Result<(Option<u32>, KillOrStopOutcome)> {
+    let existing_pid = existing_supervisor_pid()?;
+    let outcome = if let Some(pid) = existing_pid {
+        kill_or_stop(pid, force).await?
+    } else {
+        KillOrStopOutcome::AlreadyDead
+    };
+    Ok((existing_pid, outcome))
+}

src/cli/supervisor/run.rs

@@ -1,8 +1,5 @@
 use crate::Result;
-use crate::cli::supervisor::kill_or_stop;
-use crate::daemon_id::DaemonId;
-use crate::env;
-use crate::state_file::StateFile;
+use crate::cli::supervisor::{KillOrStopOutcome, resolve_existing_supervisor};
 use crate::supervisor::SUPERVISOR;
 
 /// Runs the internal pitchfork daemon in the foreground
@@ -27,12 +24,20 @@ pub struct Run {
 
 impl Run {
     pub async fn run(&self) -> Result<()> {
-        let pid_file = StateFile::read(&*env::PITCHFORK_STATE_FILE)?;
-        if let Some(d) = pid_file.daemons.get(&DaemonId::pitchfork())
-            && let Some(pid) = d.pid
-            && !(kill_or_stop(pid, self.force).await?)
-        {
-            return Ok(());
+        let (existing_pid, outcome) = resolve_existing_supervisor(self.force).await?;
+        match outcome {
+            KillOrStopOutcome::StillRunning => {
+                let pid = existing_pid.expect("StillRunning implies a pid exists");
+                warn!(
+                    "Pitchfork supervisor is already running with pid {pid}. Use `--force` to replace it."
+                );
+                return Ok(());
+            }
+            KillOrStopOutcome::Killed => {
+                let pid = existing_pid.expect("Killed implies a pid exists");
+                info!("Killed existing supervisor with pid {pid}");
+            }
+            KillOrStopOutcome::AlreadyDead => {}
         }
 
         SUPERVISOR

src/cli/supervisor/start.rs

@@ -1,11 +1,9 @@
 use crate::Result;
-use crate::cli::supervisor::kill_or_stop;
-use crate::daemon_id::DaemonId;
+use crate::cli::supervisor::{KillOrStopOutcome, resolve_existing_supervisor};
 use crate::ipc::client::IpcClient;
 use crate::procs::PROCS;
 use crate::settings::settings;
-use crate::state_file::StateFile;
-use crate::{env, supervisor};
+use crate::supervisor;
 
 /// Starts the internal pitchfork daemon in the background
 #[derive(Debug, clap::Args)]
@@ -18,34 +16,42 @@ pub struct Start {
 
 impl Start {
     pub async fn run(&self) -> Result<()> {
-        if self.force {
-            let sf = StateFile::read(&*env::PITCHFORK_STATE_FILE)?;
-            if let Some(d) = sf.daemons.get(&DaemonId::pitchfork())
-                && let Some(pid) = d.pid
-            {
-                if !kill_or_stop(pid, true).await? {
-                    return Ok(());
-                }
+        let (existing_pid, outcome) = resolve_existing_supervisor(self.force).await?;
+
+        match outcome {
+            KillOrStopOutcome::StillRunning => {
+                // --force was not passed and the supervisor is already running.
+                let pid = existing_pid.expect("StillRunning implies a pid exists");
+                warn!(
+                    "Pitchfork supervisor is already running with pid {pid}. Use `--force` to restart it."
+                );
+                return Ok(());
+            }
+            KillOrStopOutcome::Killed => {
+                let pid = existing_pid.expect("Killed implies a pid exists");
                 // Wait briefly for the old process to fully exit
                 for _ in 0..20 {
                     if !PROCS.is_running(pid) {
                         break;
                     }
                     tokio::time::sleep(std::time::Duration::from_millis(100)).await;
                 }
-                // Start a fresh supervisor in the background
-                supervisor::start_in_background()?;
+                info!("Killed existing supervisor with pid {pid}");
             }
+            KillOrStopOutcome::AlreadyDead => {}
         }
-        IpcClient::connect(true).await?;
+
+        // Start a fresh supervisor in the background.
+        supervisor::start_in_background()?;
+        // Use autostart=false since we just spawned the supervisor above.
+        // Passing true would cause connect() to call start_if_not_running(),
+        // which races with the freshly spawned process writing its state file
+        // and may spawn a second supervisor.
+        IpcClient::connect(false).await?;
         info!("Supervisor started");
 
         let s = settings();
         if s.proxy.enable && s.proxy.https {
-            // Only prompt to trust the cert if it hasn't been generated yet.
-            // Once the cert exists the user has already been through the trust
-            // flow (or is using a custom cert), so repeating the hint on every
-            // `supervisor start` would be noisy.
             let cert_path = if s.proxy.tls_cert.is_empty() {
                 crate::env::PITCHFORK_STATE_DIR.join("proxy").join("ca.pem")
             } else {

src/cli/supervisor/status.rs

@@ -9,9 +9,7 @@ pub struct Status {}
 impl Status {
     pub async fn run(&self) -> Result<()> {
         IpcClient::connect(false).await?;
-        // NOTE: info! routes to stderr (via eprintln! in Logger::log), not stdout.
-        // Use println! for user-facing messages that should appear on stdout.
-        println!("Pitchfork daemon is running");
+        info!("Pitchfork daemon is running");
         Ok(())
     }
 }

src/cli/supervisor/stop.rs

@@ -1,6 +1,8 @@
 use crate::Result;
-use crate::cli::supervisor::kill_or_stop;
+use crate::cli::supervisor::KillOrStopOutcome;
+use crate::cli::supervisor::resolve_existing_supervisor;
 use crate::daemon_id::DaemonId;
+use crate::env;
 use crate::state_file::StateFile;
 
 /// Stops the internal pitchfork daemon running in the background
@@ -10,16 +12,27 @@ pub struct Stop {}
 
 impl Stop {
     pub async fn run(&self) -> Result<()> {
-        let pid_file = StateFile::get();
-        if let Some(d) = pid_file.daemons.get(&DaemonId::pitchfork())
-            && let Some(pid) = d.pid
-        {
-            info!("Stopping pitchfork daemon with pid {pid}");
-            if kill_or_stop(pid, true).await? {
-                return Ok(());
+        let (existing_pid, outcome) = resolve_existing_supervisor(true).await?;
+        let Some(pid) = existing_pid else {
+            warn!("Pitchfork daemon is not running");
+            return Ok(());
+        };
+        match outcome {
+            KillOrStopOutcome::Killed => {
+                info!("Stopped pitchfork daemon with pid {pid}");
+            }
+            KillOrStopOutcome::AlreadyDead => {
+                // Clean up the stale entry so subsequent commands don't see it.
+                if let Ok(mut sf) = StateFile::read(&*env::PITCHFORK_STATE_FILE) {
+                    sf.daemons.remove(&DaemonId::pitchfork());
+                    let _ = sf.write();
+                }
+                warn!("Pitchfork daemon with pid {pid} was already dead (cleaned up stale state)");
+            }
+            KillOrStopOutcome::StillRunning => {
+                unreachable!("stop always passes force=true")
             }
         }
-        warn!("Pitchfork daemon is not running");
         Ok(())
     }
 }