fix: some issues related to sudo supervisor

[gaojunran]

When launching supervisor with sudo(I've never done this before until the https proxy feature), there exists some issues to be fixed.

[gemini-code-assist]

Code Review

This pull request simplifies the port management documentation and enhances process termination logic. Key changes include updating the supervisor to provide more descriptive error messages when permission is denied and refactoring process killing functions to return Result for better error propagation. Documentation was streamlined by removing several sections, though feedback suggests retaining the "Why global config only?" explanation for better context. Additionally, the process group termination check in src/procs.rs may need to be more robust to ensure all child processes have exited.

I am having trouble creating individual review comments. Click here to see my feedback.

docs/guides/port-management.md (136-140)

The removal of the "Why global config only?" section removes valuable context that explains the design decisions behind Pitchfork's configuration model. This information helps users understand the benefits of the current approach, such as having a single source of truth and cross-directory resolution. Consider retaining this section to maintain documentation depth.

src/procs.rs (135)

Checking only the process group leader's status (is_terminated_or_zombie) might be insufficient to confirm that the entire process group has terminated. If the leader becomes a zombie but children are still running, this will return Ok(true) prematurely. While this is existing logic, consider if a more robust check (e.g., verifying if any processes remain in the PGID) is needed for process groups.

[greptile-apps]

Greptile Summary

This PR fixes several issues when running the pitchfork supervisor under sudo: it correctly resolves the original user's home directory via SUDO_USER (guarded by an euid==0 check), fixes PITCHFORK_STATE_DIR to use HOME_DIR directly under sudo, adds root-UID guards to the IPC socket chown and parse_sudo_ids paths, and refactors the kill_or_stop API into a cleaner KillOrStopOutcome enum with a shared resolve_existing_supervisor helper. The supervisor start flow is also improved to call start_in_background() directly and IpcClient::connect(false) to avoid a double-spawn race.

Confidence Score: 5/5

Safe to merge; the one code issue is a theoretical compile failure on unsupported platforms that won't affect any real target.

All functional changes are well-reasoned and correctly implemented. The only finding is a P2 platform-portability issue (#[cfg(windows)] instead of #[cfg(not(unix))]) that does not affect any platform pitchfork actually targets. Prior review concerns (SUDO_USER without euid check, info! vs println! in status, hint specificity) are addressed or partially improved.

src/procs.rs — the #[cfg(windows)] narrowing should be reverted to #[cfg(not(unix))].

Important Files Changed

Filename	Overview
src/env.rs	Adds euid==0 guard before honouring SUDO_USER, and fixes PITCHFORK_STATE_DIR under sudo to use HOME_DIR directly — both changes are correct and address a prior review concern.
src/procs.rs	kill()/kill_process_group() now return Result for EPERM detection; uses libc directly instead of sysinfo Signal. #[cfg(not(unix))] was narrowed to #[cfg(windows)], causing a compile-error gap on non-unix/non-windows platforms.
src/cli/supervisor/mod.rs	Introduces KillOrStopOutcome enum and resolve_existing_supervisor helper; clean refactor that consolidates pid-from-state-file + kill logic in one place.
src/cli/supervisor/start.rs	Switches from IpcClient::connect(true) autostart to explicit start_in_background() + connect(false) to avoid a double-spawn race; logic is correct for all three KillOrStopOutcome arms.
src/cli/supervisor/stop.rs	Adds AlreadyDead cleanup path that removes the stale daemon entry from the state file; best-effort (errors silently ignored) which is appropriate here.
src/ipc/server.rs	Adds euid==0 guard before reading SUDO_UID/SUDO_GID for chown, matching the pattern applied to env.rs and supervisor/mod.rs.
src/supervisor/mod.rs	parse_sudo_ids now guards with euid==0 before returning sudo UID/GID, preventing incorrect chown in non-sudo environments with stale SUDO_UID/SUDO_GID.
src/cli/proxy.rs	install_cert now uses HOME_DIR (which already resolves the correct user home under sudo) instead of reading $HOME directly — straightforward fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pitchfork supervisor start/stop/run] --> B[resolve_existing_supervisor force]
    B --> C{existing_pid in state file?}
    C -- No --> D[AlreadyDead]
    C -- Yes --> E[kill_or_stop pid force]
    E --> F{PROCS.is_running?}
    F -- No --> D
    F -- Yes, force=false --> G[StillRunning]
    F -- Yes, force=true --> H[kill_async pid]
    H -- Ok true --> I[Killed]
    H -- Ok false --> D
    H -- Err EPERM --> J[Error: rerun with sudo]
    G -->|start/run| K[warn + return]
    I -->|start| L[wait for exit loop]
    D -->|start| M[start_in_background]
    L --> M
    M --> N[IpcClient::connect autostart=false]
    N --> O[Supervisor started]
    I -->|stop| P[info: Stopped]
    D -->|stop| Q[StateFile cleanup + warn stale]

Loading

_{Reviews (13): Last reviewed commit: "fix: sudo supervisor" | Re-trigger Greptile}

[gaojunran]

@jdx ready for review!