diff --git a/blueprint/build-plan.md b/blueprint/build-plan.md index ac33a06..98c43c6 100644 --- a/blueprint/build-plan.md +++ b/blueprint/build-plan.md @@ -34,4 +34,3 @@ - [x] 16b. **Project facts probe** - additive optional `ProjectGroup` fields (explicit contract decision): project description (package.json/Cargo.toml, README first line fallback), dev/build/start scripts, key deps, workspaces, node version, plus last commit age and dirty/clean from local git. Description renders inline in the Projects tab headers; full facts live in a project drawer (decided 2026-07-08: slide-over like the inspect drawer, no routing/detail page - the app has no router and the drawer pattern is established). - [x] 16c. **Project actions** - open in editor (new endpoint), copy cd command, and stop-all-services-in-project built on the feature 12 confirmation contract. Editor decided 2026-07-09: config `editor` key, `code` default. Shipped with two found-in-verification fixes: tilde expansion on `/api/open`/`/api/reveal`, stop-all dialog at App level. - [x] 17. **Signed Windows releases** - Authenticode-sign `portdoc.exe` in the release pipeline so Smart App Control and SmartScreen accept installs without overrides (v0.1.0 finding: SAC hard-blocks the unsigned binary on fresh Win11). Decided 2026-07-10: Azure Artifact Signing, Basic tier ($9.99/mo, 5k signatures), individual validation as "Brad Traversy". Integration is the proven unzip/sign/re-zip step on the Windows build leg (azure/artifact-signing-action@v2, OIDC login, RFC3161 timestamp) with `allow-dirty = ["ci"]`, swapped for cargo-dist's native `azure-windows-sign` when PR #2396 merges. Blocked on Brad's one-time Azure identity validation (1-20 business days); ships as v0.1.1. macOS signing/notarization deliberately deferred (curl/brew paths dodge Gatekeeper; needs the $99/yr Apple Developer membership and rcodesign - decide later). -- [ ] 18. **Cross-platform restart service** - add a confirmed Restart action beside single-service Stop controls on Linux, macOS, and Windows. Include Windows normal and forced process termination parity, revalidate the live service, capture its executable, argument vector, and working directory on the server before stopping it, wait for the old listener to release, accept a matching supervisor-created replacement, otherwise relaunch without a shell, and verify the same port returns. Disable restart when exact launch metadata is unavailable; PortDoc itself, Docker rows, unknown owners, and batch restart stay out of scope. diff --git a/blueprint/context/current-feature.md b/blueprint/context/current-feature.md index 5cfa3ad..c0aaad1 100644 --- a/blueprint/context/current-feature.md +++ b/blueprint/context/current-feature.md @@ -1,222 +1,8 @@ -# Feature: Restart service +# Current Feature -**From build-plan:** feature 18 -**Status:** in progress +> **Generated file.** Holds the one feature or fix being built right now. Run +> `/feature ` to spec a build-plan feature, or `/fix ""` for +> an ad-hoc fix. Build one thing at a time; `/complete` archives it (to +> `blueprint/history/features/` or `blueprint/history/fixes/`) and resets this file. -## Goal - -Add a Restart action beside PortDoc's existing single-service Stop actions. A -restart must stop the currently verified owner, then either recognize that a -supervisor already replaced it or safely relaunch the same executable and -arguments from the same working directory. - -This is a best-effort developer convenience, not a universal process manager. -The action is available only when PortDoc can capture enough server-side launch -metadata to reproduce the process without guessing. - -## In scope - -- Add Restart beside the existing single-service Stop actions in service rows, - the Services table, the inspect drawer, and stale-service callouts. -- Show a confirmation dialog with the current process, PID, port, command, and - working directory before anything is stopped. -- Re-probe and verify `service_id` plus PID immediately before acting. -- Capture the executable, argument vector, and working directory on the server - before sending a signal. -- Send the normal stop request first and verify the claimed PID releases the - port. -- If a different PID takes the same port, compare its executable, arguments, - and working directory with the captured launch identity. Report a matching - process as a successful supervisor restart. Report a non-matching process as - a port replacement. Never launch a duplicate in either case. -- Otherwise spawn the captured executable directly with its captured arguments - and working directory, without a shell. -- Add server-derived `restartable` and `restart_blocked_reason` fields to each - service so the UI never guesses eligibility from the display command. Keep - executable paths and argument vectors out of the JSON response. -- Poll for the same port to return and report the replacement PID when readable. -- Offer Force restart only after the original process ignores the normal stop, - using the same second-confirmation rule as Force kill. -- Keep existing error states visible, including changed service, permission - denied, missing launch metadata, spawn failure, and a replacement that does - not return to the expected port. -- Add Windows normal and forced termination behind the existing action - boundary so Stop and Restart use the same safety contract on every platform. -- Support Linux, macOS, and Windows with the same visible behavior. - -## Out of scope - -- Restarting PortDoc itself. -- Docker or Compose restart. Killing `docker-proxy` is not container control. -- Restarting services without a readable PID, executable, argument vector, or - working directory. -- Accepting a command, path, arguments, or working directory from the browser. -- Reconstructing a command by parsing the display string or running `sh -c`. -- Copying the original process environment. The relaunched process inherits - PortDoc's environment and may still load project-local `.env` files normally. -- Restart all for a project, background service management, saved launch - recipes, log capture, terminal attachment, or automatic restart policies. - -## Build loop - -Build one step at a time, never the whole feature at once. - -1. Plan mode lays out the step before any code. -2. The AI implements just that step. -3. It shows the diff, not full files; you read it and understand it. -4. You approve, then choose whether to commit a checkpoint or roll straight on. - Checkpoints are optional; `/complete` makes the feature-level commit. - -Never accept a step you have not read. If a diff is too big to review, split the -step. - -## Build steps - -- [x] **Step 1 - Capture and relaunch a process safely** - Extend the Linux, - macOS, and Windows probes with a non-serialized launch description containing - the executable, original argument vector, and working directory. Derive restart - eligibility and a blocked reason without exposing the launch description. Add - action helpers that spawn the executable directly, use null standard input, - inherit PortDoc's output destinations, prevent zombie children, and classify - whether the expected port returned under a matching or unrelated new PID. - *Done when:* focused Rust tests prove argument boundaries are preserved without - a shell, missing launch fields are rejected, eligibility reasons are stable, - and replacement polling distinguishes matching, unrelated, and absent - listeners. -- [x] **Step 2 - Add Windows stop parity** - Implement normal and forced Windows - termination behind the existing action boundary. Invoke `taskkill` directly, - never through a shell: `/PID ` first and `/F /PID ` only after the - second confirmation. Do not use `/T`, because PortDoc targets only the verified - listener PID. Replace Unix-specific UI wording with platform-neutral stop and - force language. *Done when:* Windows-focused tests cover argument construction, - a stopped child, a forced child, a missing PID, and command failure, while the - existing Linux and macOS signal tests remain green. -- [x] **Step 3 - Add the verified restart API** - Add `POST /api/restart` with - the same service and PID revalidation used by `/api/stop`. Capture launch - metadata before signaling, use the normal stop and release polling, detect a - supervisor-created replacement before spawning, then launch and verify the - expected port. Preserve the second-confirmation force path. *Done when:* - request-validation and restart-state tests cover stale identity, self-refusal, - matching supervisor replacement, unrelated port replacement, failed release, - spawn failure, successful replacement, and a spawned process that never - listens. -- [x] **Step 4 - Build the restart dialog and client contract** - Add the typed - restart request and result contract, an App-level restart context, and a - confirmation dialog that shows the exact target, working progress, force - escalation, success, unrelated port replacement, and actionable errors. Use - the server-derived eligibility fields for disabled states. *Done when:* the - frontend builds, no command data is sent by the client, and every backend - outcome has an explicit UI state. -- [x] **Step 5 - Place and verify Restart actions** - Put Restart beside - single-service Stop controls in service rows, the Services table, the inspect - drawer, and stale-service callouts. Use the server-derived eligibility fields - to disable unavailable actions with a clear reason. - *Done when:* `cargo test`, `cargo clippy`, `npm run lint`, and - `npm run build` pass, and browser evidence shows one successful restart plus - one disabled or failed case without console errors. - -## Files / areas - -- `src/probe/mod.rs` -- `src/probe/linux.rs` -- `src/probe/macos.rs` -- `src/probe/windows.rs` -- `src/action.rs` -- `src/snapshot.rs` -- `src/adapter.rs` -- `web/src/lib/types.ts` -- `src/main.rs` -- `web/src/App.tsx` -- `web/src/lib/derive.ts` -- `web/src/lib/restart.ts` (new) -- `web/src/components/RestartDialog.tsx` (new) -- `web/src/components/ServiceRow.tsx` -- `web/src/components/ServicesTable.tsx` -- `web/src/components/InspectDrawer.tsx` -- `web/src/components/Callouts.tsx` - -## Data / contracts - -- `POST /api/restart` -- Request: - - ```json - { - "service_id": "svc-3000-node", - "pid": 1234, - "force": false - } - ``` - -- Success response: - - ```json - { - "outcome": "restarted", - "pid": 5678 - } - ``` - -- `outcome` is one of: - - `restarted` - PortDoc launched a replacement and observed it on the port. - - `supervisor_restarted` - a matching replacement PID claimed the port after - the stop, so PortDoc did not launch a duplicate. - - `still_listening` - the original PID ignored the requested signal. - - `port_replaced` - a different process claimed the port but its launch - identity does not match the stopped service. PortDoc did not launch a - duplicate. - - `not_listening` - the replacement was launched but did not claim the - expected port within the bounded wait. -- `pid` is optional and contains the replacement listener PID when readable. -- Error responses use the existing `{ "error": "message" }` shape and suitable - `400`, `403`, `409`, or `500` status codes. -- The browser never supplies executable, arguments, command text, or cwd. -- `Service` gains two additive fields: - - `restartable` (boolean) - true only when the current platform and captured - launch description support restart. - - `restart_blocked_reason` (optional string) - protected self, Docker-managed, - unknown owner, incomplete launch metadata, or unsupported platform. -- Raw executable, argument, and launch metadata remain internal and are not - added to `DevSnapshot` JSON. - -## Testing - -- Rust logic is covered by `cargo test` in the same step that introduces it. -- Test direct spawning with arguments containing spaces and shell characters to - prove no shell interpretation occurs. -- Test service and PID revalidation before any signal. -- Test normal release, force escalation, matching supervisor replacement, spawn - failure, unrelated port replacement, successful return on the expected port, - and bounded no-listener timeout. -- Run the Rust test matrix on `ubuntu-latest`, `macos-latest`, and - `windows-latest` before the feature is complete. -- Manually verify one successful restart and one blocked or failed restart on - Linux, macOS, and Windows before release. Brad starts the test servers on each - machine. -- Keep real process and socket fixtures short-lived and bounded so failures - cannot hang the suite. -- The frontend has no test runner. Verify it with `npm run lint`, - `npm run build`, and browser evidence against a server Brad starts. - -## Notes for the AI - -- Preserve the existing `/api/stop` behavior and reuse its validation and signal - rules rather than creating a weaker parallel path. -- Treat the server-side re-probe as authoritative. Client-side eligibility is - only a convenience and must not replace backend checks. -- Never parse the display command back into arguments. -- Never execute restart data through a shell. -- On Windows, call `taskkill` with structured arguments through - `std::process::Command`. Never use a shell and never add `/T` to the request. - A normal Windows stop is best-effort; if the PID remains, the existing second - confirmation offers the forced path. -- Capture launch metadata before signaling because the process may disappear - immediately. -- Re-probe immediately before spawning. If another PID already owns the port, - compare its launch identity with the captured one. Report - `supervisor_restarted` only for a match, otherwise report `port_replaced`. Do - not create a duplicate in either case. -- A spawned process that does not return to the expected port is a visible - `not_listening` outcome, not a silent success. -- Do not touch the pre-existing uncommitted edits in `AGENTS.md`, - `src/adapter.rs`, or `src/config.rs`. +_Nothing in progress. Run `/feature` or `/fix` to start._ diff --git a/blueprint/history/fixes/remove-unsafe-restart-action.md b/blueprint/history/fixes/remove-unsafe-restart-action.md new file mode 100644 index 0000000..78abb49 --- /dev/null +++ b/blueprint/history/fixes/remove-unsafe-restart-action.md @@ -0,0 +1,53 @@ +# Fix: Remove unsafe restart action + +**Type:** Fix (not a build-plan item) +**Status:** complete + +## The problem + +Manual Linux validation of `v0.1.2-rc.1` showed that Restart is not safe for a +supervised development server. PortDoc targeted the process listening on the +port instead of the process supervisor, left the original watcher tree behind, +and launched a non-equivalent replacement attached to PortDoc's terminal. + +The current implementation cannot reliably reconstruct ownership or supervisor +behavior for arbitrary development servers across Linux, macOS, and Windows. +It can therefore stop the wrong layer of a process tree and make PortDoc the +replacement process's accidental supervisor. + +## The fix + +Revert the Restart feature introduced by merge commit `575b819`. Remove its API, +process relaunch behavior, probe metadata, snapshot fields, and frontend actions +on every supported platform. + +Preserve the existing safe Stop behavior, all release and installer work, and +Windows signing. Keep `v0.1.2-rc.1` as an immutable failed prerelease rather than +rewriting published history. + +A future Restart design requires an explicit, reviewable supervisor or project +launch contract. That redesign is not part of this rollback. + +## Build steps + +- [x] **Step 1 - Revert Restart without disturbing other shipped work** - Reverse + the production and UI changes from merge commit `575b819`, keeping this fix + spec while resolving the Blueprint file touched by the original merge. + Preserve `/api/stop`, the existing Stop and Force stop controls, cargo-dist, + installers, Windows signing, and the separate `v0.1.2-rc.1` version changes. + *Done when:* PortDoc exposes no Restart action or `/api/restart` endpoint, no + longer captures or spawns replacement launch data, and the remaining diff + contains no unrelated rollback. + +## Verify + +- Run `cargo fmt --check`. +- Run `cargo test`. +- Run `cargo clippy -- -D warnings`. +- Run `npm run lint` in `web/`. +- Run `npm run build` in `web/`. +- Confirm the frontend contains Stop controls but no Restart controls. +- Confirm `/api/stop` and its force path remain unchanged. +- Confirm the pull request CI matrix passes on Ubuntu, macOS, and Windows. +- Do not push, publish, retag, or create a replacement release as part of this + fix. diff --git a/src/action.rs b/src/action.rs index d558be5..4e02057 100644 --- a/src/action.rs +++ b/src/action.rs @@ -1,14 +1,11 @@ -//! Process actions: signal delivery, release polling, and safe relaunch. +//! Stopping services (feature 12): signal delivery and release polling. //! The safety guards (self-refusal, verify handshake) live in the stop //! endpoint; this module only knows pids and signals. +#[cfg(windows)] use std::process::{Command, Stdio}; -use std::sync::mpsc; -use std::thread; use std::time::Duration; -use crate::probe::LaunchInfo; - #[derive(Debug, thiserror::Error)] pub enum StopError { // Only the unix terminate constructs these two. @@ -28,84 +25,6 @@ pub enum StopError { Unsupported, } -#[derive(Debug, thiserror::Error)] -pub enum RestartError { - #[error("failed to create child reaper: {0}")] - Reaper(#[source] std::io::Error), - #[error("failed to launch replacement: {0}")] - Spawn(#[source] std::io::Error), - #[error("child reaper stopped before receiving replacement")] - ReaperUnavailable, -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ReplacementState { - Matching(u32), - Unrelated(Option), - Absent, -} - -fn replacement_command(launch: &LaunchInfo) -> Command { - let mut command = Command::new(&launch.executable); - command - .args(launch.arguments()) - .current_dir(&launch.cwd) - .stdin(Stdio::null()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()); - command -} - -pub fn spawn_replacement(launch: &LaunchInfo) -> Result { - let (sender, receiver) = mpsc::sync_channel::(1); - thread::Builder::new() - .name("portdoc-child-reaper".into()) - .spawn(move || { - if let Ok(mut child) = receiver.recv() { - let _ = child.wait(); - } - }) - .map_err(RestartError::Reaper)?; - - let child = replacement_command(launch) - .spawn() - .map_err(RestartError::Spawn)?; - let pid = child.id(); - if let Err(error) = sender.send(child) { - let mut child = error.0; - let _ = child.kill(); - let _ = child.wait(); - return Err(RestartError::ReaperUnavailable); - } - Ok(pid) -} - -pub fn classify_replacement<'a>( - expected: &LaunchInfo, - candidates: impl IntoIterator, Option<&'a LaunchInfo>)>, -) -> ReplacementState { - let mut unrelated: Option> = None; - for (pid, launch) in candidates { - if let Some(pid) = pid - && launch.is_some_and(|candidate| same_launch_identity(expected, candidate)) - { - return ReplacementState::Matching(pid); - } - if unrelated.is_none() { - unrelated = Some(pid); - } - } - unrelated - .map(ReplacementState::Unrelated) - .unwrap_or(ReplacementState::Absent) -} - -fn same_launch_identity(expected: &LaunchInfo, candidate: &LaunchInfo) -> bool { - expected.executable == candidate.executable - && expected.arguments() == candidate.arguments() - && expected.cwd == candidate.cwd -} - #[cfg(unix)] pub fn terminate(pid: u32, force: bool) -> Result<(), StopError> { let signal = if force { libc::SIGKILL } else { libc::SIGTERM }; @@ -229,90 +148,6 @@ mod tests { #[cfg(any(unix, windows))] use std::process::{Child, Command}; - fn test_launch(argv: &[&str]) -> LaunchInfo { - LaunchInfo::from_parts( - Some(std::env::current_exe().expect("current executable")), - argv.iter().map(OsString::from).collect(), - Some(std::env::current_dir().expect("current directory")), - ) - .expect("complete launch info") - } - - #[cfg(unix)] - fn short_launch() -> LaunchInfo { - LaunchInfo::from_parts( - Some("/bin/sh".into()), - ["sh", "-c", "exit 0"].map(OsString::from).to_vec(), - Some(std::env::current_dir().expect("current directory")), - ) - .expect("complete shell launch") - } - - #[cfg(windows)] - fn short_launch() -> LaunchInfo { - let executable = std::env::var_os("COMSPEC") - .map(Into::into) - .expect("COMSPEC should point to cmd.exe"); - LaunchInfo::from_parts( - Some(executable), - ["cmd.exe", "/C", "exit 0"].map(OsString::from).to_vec(), - Some(std::env::current_dir().expect("current directory")), - ) - .expect("complete cmd launch") - } - - #[test] - fn replacement_command_preserves_argument_boundaries() { - let launch = test_launch(&["portdoc-test", "value with spaces", "; touch nope"]); - let command = replacement_command(&launch); - assert_eq!(command.get_program(), launch.executable.as_os_str()); - assert_eq!( - command.get_args().collect::>(), - launch - .arguments() - .iter() - .map(OsString::as_os_str) - .collect::>() - ); - assert_eq!(command.get_current_dir(), Some(launch.cwd.as_path())); - } - - #[test] - #[cfg(any(unix, windows))] - fn replacement_is_spawned_and_handed_to_reaper() { - let pid = spawn_replacement(&short_launch()).expect("replacement should spawn"); - assert!(pid > 0); - } - - #[test] - fn replacement_classification_distinguishes_matching_unrelated_and_absent() { - let expected = test_launch(&["server", "--port", "3000"]); - let normalized_argv0 = test_launch(&["/canonical/server", "--port", "3000"]); - let unrelated = test_launch(&["other", "--port", "4000"]); - assert_eq!( - classify_replacement( - &expected, - [ - (Some(10), Some(&unrelated)), - (Some(11), Some(&normalized_argv0)) - ] - ), - ReplacementState::Matching(11) - ); - assert_eq!( - classify_replacement(&expected, [(Some(12), Some(&unrelated))]), - ReplacementState::Unrelated(Some(12)) - ); - assert_eq!( - classify_replacement(&expected, std::iter::empty()), - ReplacementState::Absent - ); - assert_eq!( - classify_replacement(&expected, [(None, None)]), - ReplacementState::Unrelated(None) - ); - } - /// Bounded reap so a failed signal cannot hang the suite. #[cfg(any(unix, windows))] fn exits_within(child: &mut Child, tries: u32) -> bool { diff --git a/src/adapter.rs b/src/adapter.rs index fb1e214..1d7e158 100644 --- a/src/adapter.rs +++ b/src/adapter.rs @@ -9,9 +9,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use crate::facts::ProjectFacts; use crate::label::{ProjectLabels, detect_framework, http_looking, is_dev_server, project_labels}; -use crate::probe::{ - LaunchInfo, ListeningSocket, ProbeError, ProbeOutput, ProcessInfo, platform_probe, -}; +use crate::probe::{ListeningSocket, ProbeError, ProbeOutput, ProcessInfo, platform_probe}; use crate::project::{Marker, detect_root, fs_marker}; use crate::snapshot::{DevSnapshot, Exposure, ProjectGroup, Service, StaleHint}; @@ -302,9 +300,6 @@ fn service_from(merged: MergedSocket, id: String) -> Service { let process = merged.process; let name = process.as_ref().and_then(|p| p.name.as_deref()); let exposure = exposure(&merged.addrs, name); - let launch = process.as_ref().and_then(|p| p.launch.clone()); - let (restartable, restart_blocked_reason) = - restart_eligibility(merged.pid, name, &exposure, launch.as_ref()); let framework = detect_framework(name, process.as_ref().and_then(|p| p.command.as_deref())); let url = http_looking(merged.port, name, framework.as_deref()) .then(|| url(&merged.addrs, merged.port)); @@ -326,37 +321,9 @@ fn service_from(merged: MergedSocket, id: String) -> Service { url, started_age: started_secs.map(humanize_age), stale, - restartable, - restart_blocked_reason, - launch, } } -fn restart_eligibility( - pid: Option, - name: Option<&str>, - exposure: &Exposure, - launch: Option<&LaunchInfo>, -) -> (bool, Option) { - let is_portdoc = name.is_some_and(|value| { - value.eq_ignore_ascii_case("portdoc") || value.eq_ignore_ascii_case("portdoc.exe") - }); - let reason = if pid == Some(std::process::id()) || is_portdoc { - Some("PortDoc will not restart itself") - } else if matches!(exposure, Exposure::Docker) { - Some("Use Docker to restart this service") - } else if pid.is_none() { - Some("No owner process to restart") - } else if launch.is_none() { - Some("Executable, arguments, or working directory are unavailable") - } else if !cfg!(any(unix, windows)) { - Some("Restart is not supported on this platform yet") - } else { - None - }; - (reason.is_none(), reason.map(str::to_string)) -} - const STALE_AFTER_SECS: u64 = 3 * 24 * 60 * 60; /// Only known dev servers get accused, and only with the provable fact @@ -372,11 +339,9 @@ fn stale_hint(framework: Option<&str>, started_secs_ago: Option) -> Option< #[cfg(test)] mod tests { use super::*; - use crate::probe::{LaunchInfo, Protocol}; + use crate::probe::Protocol; fn proc_info(pid: u32, name: &str) -> ProcessInfo { - let executable = std::env::current_exe().expect("current executable"); - let cwd = std::env::current_dir().expect("current directory"); ProcessInfo { pid, name: Some(name.into()), @@ -384,11 +349,6 @@ mod tests { cwd: Some("/home/brad/Code/app".into()), user: Some("brad".into()), started_secs_ago: Some(240), - launch: LaunchInfo::from_parts( - Some(executable), - [name, "--serve"].map(Into::into).to_vec(), - Some(cwd), - ), } } @@ -511,63 +471,6 @@ mod tests { assert_eq!(svc.id, "svc-5432-postgres"); } - #[test] - fn restart_eligibility_is_explicit_and_keeps_launch_data_private() { - let mut process = proc_info(42, "node"); - let launch = process.launch.clone(); - let (restartable, blocked) = - restart_eligibility(Some(42), Some("node"), &Exposure::Local, launch.as_ref()); - assert_eq!(restartable, cfg!(any(unix, windows))); - assert_eq!(blocked.is_none(), cfg!(any(unix, windows))); - - process.launch = None; - let service = service_from( - MergedSocket { - port: 3000, - pid: Some(42), - addrs: vec!["127.0.0.1".parse().expect("test addr")], - process: Some(process), - }, - "svc-3000-node".into(), - ); - assert!(!service.restartable); - assert_eq!( - service.restart_blocked_reason.as_deref(), - Some("Executable, arguments, or working directory are unavailable") - ); - let json = serde_json::to_value(&service).expect("service should serialize"); - assert_eq!(json["restartable"], false); - assert!(json.get("restart_blocked_reason").is_some()); - assert!(json.get("launch").is_none()); - assert!(json.get("executable").is_none()); - assert!(json.get("argv").is_none()); - } - - #[test] - fn restart_eligibility_refuses_self_unknown_owners_and_docker() { - let launch = proc_info(42, "node").launch; - let cases = [ - ( - Some(42), - Some("portdoc.exe"), - Exposure::Local, - "PortDoc will not restart itself", - ), - (None, None, Exposure::Unknown, "No owner process to restart"), - ( - Some(42), - Some("docker-proxy"), - Exposure::Docker, - "Use Docker to restart this service", - ), - ]; - for (pid, name, exposure, expected_reason) in cases { - let (restartable, reason) = restart_eligibility(pid, name, &exposure, launch.as_ref()); - assert!(!restartable); - assert_eq!(reason.as_deref(), Some(expected_reason)); - } - } - fn proc_at(pid: u32, name: &str, cwd: &str) -> ProcessInfo { ProcessInfo { cwd: Some(cwd.into()), diff --git a/src/docker.rs b/src/docker.rs index d04eacf..bf79bae 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -148,9 +148,6 @@ mod tests { url: None, started_age: None, stale: None, - restartable: false, - restart_blocked_reason: Some("test fixture".into()), - launch: None, } } diff --git a/src/hint.rs b/src/hint.rs index 839bd32..551f8aa 100644 --- a/src/hint.rs +++ b/src/hint.rs @@ -72,9 +72,6 @@ mod tests { url: None, started_age: None, stale: None, - restartable: false, - restart_blocked_reason: Some("test fixture".into()), - launch: None, } } diff --git a/src/main.rs b/src/main.rs index 5ceb94a..c562ba6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,7 +9,6 @@ mod hint; mod label; mod probe; mod project; -mod restart; mod snapshot; use std::net::SocketAddr; @@ -79,7 +78,6 @@ async fn main() { .route("/api/config", get(api_config)) .route("/api/ignore", post(api_ignore)) .route("/api/stop", post(api_stop)) - .route("/api/restart", post(api_restart)) .route("/api/reveal", post(api_reveal)) .route("/api/open", post(api_open)) .fallback(static_handler); @@ -248,17 +246,6 @@ async fn api_stop(Json(request): Json) -> Response { } } -async fn api_restart(Json(request): Json) -> Response { - match tokio::task::spawn_blocking(move || restart::execute(request)).await { - Ok(Ok(result)) => Json(result).into_response(), - Ok(Err(failure)) => api_error(failure.status, failure.message), - Err(error) => api_error( - StatusCode::INTERNAL_SERVER_ERROR, - format!("restart task failed: {error}"), - ), - } -} - /// The safety contract: no signal is ever sent to a pid that does not, /// right now, own the claimed service. fn stop_service(request: StopRequest) -> Response { @@ -275,10 +262,31 @@ fn stop_service(request: StopRequest) -> Response { ); } }; - let service = match restart::verify_target(&snapshot, &request.service_id, request.pid) { - Ok(service) => service, - Err(failure) => return api_error(failure.status, failure.message), + let Some(service) = snapshot + .services + .iter() + .find(|s| s.id == request.service_id) + else { + return api_error( + StatusCode::CONFLICT, + "service not found - refresh and retry".into(), + ); }; + match service.pid { + None => { + return api_error( + StatusCode::BAD_REQUEST, + "service has no known owner pid".into(), + ); + } + Some(pid) if pid != request.pid => { + return api_error( + StatusCode::CONFLICT, + "service changed - refresh and retry".into(), + ); + } + Some(_) => {} + } let port = service.port; match action::terminate(request.pid, request.force) { diff --git a/src/probe/linux.rs b/src/probe/linux.rs index 067c551..b2c6f4a 100644 --- a/src/probe/linux.rs +++ b/src/probe/linux.rs @@ -1,12 +1,10 @@ use std::collections::HashMap; -use std::ffi::OsString; -use std::os::unix::ffi::OsStringExt; use procfs::Current; use procfs::net::TcpState; use procfs::process::FDTarget; -use super::{LaunchInfo, ListeningSocket, Probe, ProbeError, ProbeOutput, ProcessInfo, Protocol}; +use super::{ListeningSocket, Probe, ProbeError, ProbeOutput, ProcessInfo, Protocol}; /// Linux probe backed by /proc: listening TCP sockets from /proc/net/tcp{,6}. pub struct LinuxProbe; @@ -77,12 +75,6 @@ fn process_info(pid: u32, host: &HostContext) -> Option { let process = procfs::process::Process::new(pid as i32).ok()?; let stat = process.stat().ok(); let cmdline = process.cmdline().ok().filter(|c| !c.is_empty()); - let cwd = process.cwd().ok(); - let launch = LaunchInfo::from_parts( - process.exe().ok(), - launch_argv(pid).unwrap_or_default(), - cwd.clone(), - ); Some(ProcessInfo { pid, @@ -91,7 +83,7 @@ fn process_info(pid: u32, host: &HostContext) -> Option { cmdline.as_ref().and_then(|c| c.first()).map(String::as_str), ), command: cmdline.as_ref().map(|c| c.join(" ")), - cwd, + cwd: process.cwd().ok(), user: process .uid() .ok() @@ -100,26 +92,9 @@ fn process_info(pid: u32, host: &HostContext) -> Option { (Some(s), Some(uptime)) => Some(secs_ago(uptime, s.starttime, host.ticks_per_second)), _ => None, }, - launch, }) } -fn launch_argv(pid: u32) -> Option> { - let bytes = std::fs::read(format!("/proc/{pid}/cmdline")).ok()?; - Some(parse_launch_argv(&bytes)) -} - -fn parse_launch_argv(bytes: &[u8]) -> Vec { - let mut parts: Vec<&[u8]> = bytes.split(|byte| *byte == 0).collect(); - if parts.last().is_some_and(|part| part.is_empty()) { - parts.pop(); - } - parts - .into_iter() - .map(|part| OsString::from_vec(part.to_vec())) - .collect() -} - /// The kernel caps comm at 15 bytes and threads may rename it (node's /// "MainThread"); prefer the process title when it is clearly the better /// truth, otherwise keep comm. @@ -207,7 +182,6 @@ fn proc_err(path: &str, err: procfs::ProcError) -> ProbeError { #[cfg(test)] mod tests { use super::*; - use std::os::unix::ffi::OsStrExt; #[test] fn probe_sees_own_listener() { @@ -265,15 +239,6 @@ mod tests { ); } - #[test] - fn raw_launch_argv_preserves_empty_and_non_utf8_arguments() { - let parsed = parse_launch_argv(b"node\0\0\xff\0"); - assert_eq!(parsed.len(), 3); - assert_eq!(parsed[0].as_bytes(), b"node"); - assert_eq!(parsed[1].as_bytes(), b""); - assert_eq!(parsed[2].as_bytes(), b"\xff"); - } - #[test] fn best_name_expands_truncated_comm_from_the_title() { assert_eq!( diff --git a/src/probe/macos.rs b/src/probe/macos.rs index 5bdfd24..950c05a 100644 --- a/src/probe/macos.rs +++ b/src/probe/macos.rs @@ -1,11 +1,10 @@ use std::collections::HashMap; -use std::ffi::OsString; use std::time::{SystemTime, UNIX_EPOCH}; use netstat2::{AddressFamilyFlags, ProtocolFlags, ProtocolSocketInfo, TcpState}; use sysinfo::{Pid, ProcessRefreshKind, ProcessesToUpdate, System, UpdateKind, Users}; -use super::{LaunchInfo, ListeningSocket, Probe, ProbeError, ProbeOutput, ProcessInfo, Protocol}; +use super::{ListeningSocket, Probe, ProbeError, ProbeOutput, ProcessInfo, Protocol}; /// macOS probe backed by netstat2 for the socket table (kernel pcb list via /// sysctl, visible for all users without root) and sysinfo for process @@ -44,7 +43,6 @@ fn listening_sockets() -> Result, ProbeError> { false, ProcessRefreshKind::nothing() .with_cmd(UpdateKind::Always) - .with_exe(UpdateKind::Always) .with_cwd(UpdateKind::Always) .with_user(UpdateKind::Always), ); @@ -106,30 +104,23 @@ fn raw_listeners() -> Result, ProbeError> { /// Best-effort process metadata; any unreadable piece degrades to None. fn process_info(pid: u32, system: &System, users: &Users, now: u64) -> Option { let process = system.process(Pid::from_u32(pid))?; - let mut argv = process.cmd().to_vec(); - trim_env_tail(&mut argv); - let cmd: Vec = argv + let mut cmd: Vec = process + .cmd() .iter() .map(|part| part.to_string_lossy().into_owned()) .collect(); - let cwd = process.cwd().map(std::path::Path::to_path_buf); - let launch = LaunchInfo::from_parts( - process.exe().map(std::path::Path::to_path_buf), - argv, - cwd.clone(), - ); + trim_env_tail(&mut cmd); Some(ProcessInfo { pid, name: expand_name(process.name().to_str(), cmd.first().map(String::as_str)), command: (!cmd.is_empty()).then(|| cmd.join(" ")), - cwd, + cwd: process.cwd().map(std::path::Path::to_path_buf), user: process .user_id() .and_then(|uid| users.get_user_by_id(uid)) .map(|user| user.name().to_string()), started_secs_ago: secs_ago(now, process.start_time()), - launch, }) } @@ -159,10 +150,8 @@ fn name_token(first_arg: &str) -> &str { /// KERN_PROCARGS2 argv sysinfo reads spill trailing environment entries /// ("redis-server 127.0.0.1:6379 XPC_FLAGS=1"); drop them from the tail. /// argv[0] always survives. -fn trim_env_tail(cmd: &mut Vec) { - while cmd.len() > 1 - && is_env_assignment(cmd.last().expect("len checked").to_string_lossy().as_ref()) - { +fn trim_env_tail(cmd: &mut Vec) { + while cmd.len() > 1 && is_env_assignment(cmd.last().expect("len checked")) { cmd.pop(); } } @@ -282,18 +271,18 @@ mod tests { #[test] fn trim_env_tail_drops_leaked_environment_entries_only() { - let mut cmd: Vec = [ + let mut cmd: Vec = [ "redis-server 127.0.0.1:6379", "XPC_FLAGS=1", "PATH=/usr/bin", ] - .map(OsString::from) + .map(String::from) .to_vec(); trim_env_tail(&mut cmd); - assert_eq!(cmd, vec![OsString::from("redis-server 127.0.0.1:6379")]); + assert_eq!(cmd, vec!["redis-server 127.0.0.1:6379"]); - let mut cmd: Vec = ["node", "server.js", "--port=3000"] - .map(OsString::from) + let mut cmd: Vec = ["node", "server.js", "--port=3000"] + .map(String::from) .to_vec(); trim_env_tail(&mut cmd); assert_eq!( @@ -302,13 +291,9 @@ mod tests { "lowercase --port=3000 is an argument, not env" ); - let mut cmd: Vec = ["FOO=1"].map(OsString::from).to_vec(); + let mut cmd: Vec = ["FOO=1"].map(String::from).to_vec(); trim_env_tail(&mut cmd); - assert_eq!( - cmd, - vec![OsString::from("FOO=1")], - "argv[0] always survives" - ); + assert_eq!(cmd, vec!["FOO=1"], "argv[0] always survives"); } #[test] diff --git a/src/probe/mod.rs b/src/probe/mod.rs index b8d0fcb..25e97dc 100644 --- a/src/probe/mod.rs +++ b/src/probe/mod.rs @@ -11,7 +11,6 @@ mod macos; #[cfg(target_os = "windows")] mod windows; -use std::ffi::OsString; use std::net::IpAddr; use std::path::PathBuf; @@ -39,36 +38,6 @@ pub struct ListeningSocket { pub user: Option, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct LaunchInfo { - pub executable: PathBuf, - pub argv: Vec, - pub cwd: PathBuf, -} - -impl LaunchInfo { - pub fn from_parts( - executable: Option, - argv: Vec, - cwd: Option, - ) -> Option { - let executable = executable.filter(|path| path.is_absolute())?; - let cwd = cwd.filter(|path| path.is_absolute())?; - if argv.is_empty() { - return None; - } - Some(Self { - executable, - argv, - cwd, - }) - } - - pub fn arguments(&self) -> &[OsString] { - &self.argv[1..] - } -} - /// Everything but `pid` is optional: unknown owners are a first-class case. #[derive(Debug, Clone)] pub struct ProcessInfo { @@ -80,7 +49,6 @@ pub struct ProcessInfo { pub cwd: Option, pub user: Option, pub started_secs_ago: Option, - pub launch: Option, } #[derive(Debug, Default)] @@ -196,35 +164,4 @@ mod tests { "probing is not supported on this platform" ); } - - #[test] - fn launch_info_requires_exact_absolute_parts() { - let executable = std::env::current_exe().expect("current executable"); - let cwd = std::env::current_dir().expect("current directory"); - let argv = vec![OsString::from("node"), OsString::from("server.js")]; - assert!(LaunchInfo::from_parts(None, argv.clone(), Some(cwd.clone())).is_none()); - assert!( - LaunchInfo::from_parts(Some("node".into()), argv.clone(), Some(cwd.clone())).is_none() - ); - assert!(LaunchInfo::from_parts(Some(executable.clone()), Vec::new(), Some(cwd)).is_none()); - assert!( - LaunchInfo::from_parts(Some(executable), argv, Some("relative-cwd".into())).is_none() - ); - } - - #[test] - fn launch_info_preserves_argument_boundaries() { - let executable = std::env::current_exe().expect("current executable"); - let cwd = std::env::current_dir().expect("current directory"); - let launch = LaunchInfo::from_parts( - Some(executable), - ["node", "server with spaces.js", "; touch /tmp/nope"] - .map(OsString::from) - .to_vec(), - Some(cwd), - ) - .expect("complete launch info"); - assert_eq!(launch.arguments(), &launch.argv[1..]); - assert_eq!(launch.arguments().len(), 2); - } } diff --git a/src/probe/windows.rs b/src/probe/windows.rs index 8896ec5..ae8a021 100644 --- a/src/probe/windows.rs +++ b/src/probe/windows.rs @@ -4,7 +4,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use netstat2::{AddressFamilyFlags, ProtocolFlags, ProtocolSocketInfo, TcpState}; use sysinfo::{Pid, ProcessRefreshKind, ProcessesToUpdate, System, UpdateKind, Users}; -use super::{LaunchInfo, ListeningSocket, Probe, ProbeError, ProbeOutput, ProcessInfo, Protocol}; +use super::{ListeningSocket, Probe, ProbeError, ProbeOutput, ProcessInfo, Protocol}; /// Windows probe backed by netstat2 for the socket table /// (GetExtendedTcpTable under the hood, which carries the owning pid for @@ -43,7 +43,6 @@ fn listening_sockets() -> Result, ProbeError> { false, ProcessRefreshKind::nothing() .with_cmd(UpdateKind::Always) - .with_exe(UpdateKind::Always) .with_cwd(UpdateKind::Always) .with_user(UpdateKind::Always), ); @@ -108,17 +107,11 @@ fn raw_listeners() -> Result, ProbeError> { /// labeling strips that downstream). fn process_info(pid: u32, system: &System, users: &Users, now: u64) -> Option { let process = system.process(Pid::from_u32(pid))?; - let argv = process.cmd().to_vec(); - let cmd: Vec = argv + let cmd: Vec = process + .cmd() .iter() .map(|part| part.to_string_lossy().into_owned()) .collect(); - let cwd = process.cwd().map(std::path::Path::to_path_buf); - let launch = LaunchInfo::from_parts( - process.exe().map(std::path::Path::to_path_buf), - argv, - cwd.clone(), - ); Some(ProcessInfo { pid, @@ -128,13 +121,12 @@ fn process_info(pid: u32, system: &System, users: &Users, now: u64) -> Option, -} - -#[derive(Debug, PartialEq, Eq)] -pub struct RestartFailure { - pub status: StatusCode, - pub message: String, -} - -impl RestartFailure { - fn new(status: StatusCode, message: impl Into) -> Self { - Self { - status, - message: message.into(), - } - } - - fn internal(message: impl Into) -> Self { - Self::new(StatusCode::INTERNAL_SERVER_ERROR, message) - } -} - -pub fn execute(request: RestartRequest) -> Result { - execute_with(request, &mut LiveRuntime) -} - -pub(crate) fn verify_target<'a>( - snapshot: &'a DevSnapshot, - service_id: &str, - pid: u32, -) -> Result<&'a Service, RestartFailure> { - let Some(service) = snapshot - .services - .iter() - .find(|service| service.id == service_id) - else { - return Err(RestartFailure::new( - StatusCode::CONFLICT, - "service not found - refresh and retry", - )); - }; - match service.pid { - None => Err(RestartFailure::new( - StatusCode::BAD_REQUEST, - "service has no known owner pid", - )), - Some(current) if current != pid => Err(RestartFailure::new( - StatusCode::CONFLICT, - "service changed - refresh and retry", - )), - Some(_) => Ok(service), - } -} - -fn execute_with( - request: RestartRequest, - runtime: &mut impl RestartRuntime, -) -> Result { - if request.pid == std::process::id() { - return Err(RestartFailure::new( - StatusCode::FORBIDDEN, - "PortDoc will not restart itself", - )); - } - - let snapshot = runtime - .snapshot() - .map_err(|message| RestartFailure::internal(format!("probe failed: {message}")))?; - let service = verify_target(&snapshot, &request.service_id, request.pid)?; - if !service.restartable { - let message = service - .restart_blocked_reason - .clone() - .unwrap_or_else(|| "service cannot be restarted safely".into()); - let status = if message.starts_with("PortDoc will not") { - StatusCode::FORBIDDEN - } else { - StatusCode::BAD_REQUEST - }; - return Err(RestartFailure::new(status, message)); - } - let launch = service.launch.clone().ok_or_else(|| { - RestartFailure::new( - StatusCode::BAD_REQUEST, - "Executable, arguments, or working directory are unavailable", - ) - })?; - let port = service.port; - - let released = match runtime.terminate(request.pid, request.force) { - Ok(()) | Err(TerminateFailure::Missing) => poll_original_release( - runtime, - port, - request.pid, - RELEASE_ATTEMPTS, - RELEASE_INTERVAL, - )?, - Err(TerminateFailure::NotPermitted(message)) => { - return Err(RestartFailure::new(StatusCode::FORBIDDEN, message)); - } - Err(TerminateFailure::Other(message)) => { - return Err(RestartFailure::internal(message)); - } - }; - if !released { - return Ok(RestartResponse { - outcome: RestartOutcome::StillListening, - pid: None, - }); - } - - match poll_replacement( - runtime, - port, - &launch, - SUPERVISOR_ATTEMPTS, - REPLACEMENT_INTERVAL, - )? { - ReplacementState::Matching(pid) => { - return Ok(RestartResponse { - outcome: RestartOutcome::SupervisorRestarted, - pid: Some(pid), - }); - } - ReplacementState::Unrelated(pid) => { - return Ok(RestartResponse { - outcome: RestartOutcome::PortReplaced, - pid, - }); - } - ReplacementState::Absent => {} - } - - runtime.spawn(&launch).map_err(RestartFailure::internal)?; - - match poll_replacement(runtime, port, &launch, START_ATTEMPTS, REPLACEMENT_INTERVAL)? { - ReplacementState::Matching(pid) => Ok(RestartResponse { - outcome: RestartOutcome::Restarted, - pid: Some(pid), - }), - ReplacementState::Unrelated(pid) => Ok(RestartResponse { - outcome: RestartOutcome::PortReplaced, - pid, - }), - ReplacementState::Absent => Ok(RestartResponse { - outcome: RestartOutcome::NotListening, - pid: None, - }), - } -} - -enum TerminateFailure { - Missing, - NotPermitted(String), - Other(String), -} - -trait RestartRuntime { - fn snapshot(&mut self) -> Result; - fn terminate(&mut self, pid: u32, force: bool) -> Result<(), TerminateFailure>; - fn probe(&mut self) -> Result; - fn spawn(&mut self, launch: &LaunchInfo) -> Result; - fn sleep(&mut self, duration: Duration); -} - -struct LiveRuntime; - -impl RestartRuntime for LiveRuntime { - fn snapshot(&mut self) -> Result { - adapter::live_snapshot().map_err(|error| error.to_string()) - } - - fn terminate(&mut self, pid: u32, force: bool) -> Result<(), TerminateFailure> { - match action::terminate(pid, force) { - Ok(()) => Ok(()), - Err(action::StopError::NoSuchProcess) => Err(TerminateFailure::Missing), - Err(error @ action::StopError::NotPermitted) => { - Err(TerminateFailure::NotPermitted(error.to_string())) - } - Err(error) => Err(TerminateFailure::Other(error.to_string())), - } - } - - fn probe(&mut self) -> Result { - let probe = platform_probe().ok_or_else(|| "probing is not supported".to_string())?; - probe.probe().map_err(|error| error.to_string()) - } - - fn spawn(&mut self, launch: &LaunchInfo) -> Result { - action::spawn_replacement(launch).map_err(|error| error.to_string()) - } - - fn sleep(&mut self, duration: Duration) { - std::thread::sleep(duration); - } -} - -fn poll_original_release( - runtime: &mut impl RestartRuntime, - port: u16, - pid: u32, - attempts: u32, - interval: Duration, -) -> Result { - for attempt in 0..attempts { - let output = runtime - .probe() - .map_err(|message| RestartFailure::internal(format!("probe failed: {message}")))?; - let still_owned = output - .sockets - .iter() - .any(|socket| socket.port == port && socket.pid == Some(pid)); - if !still_owned { - return Ok(true); - } - if attempt + 1 < attempts { - runtime.sleep(interval); - } - } - Ok(false) -} - -fn poll_replacement( - runtime: &mut impl RestartRuntime, - port: u16, - expected: &LaunchInfo, - attempts: u32, - interval: Duration, -) -> Result { - for attempt in 0..attempts { - let output = runtime - .probe() - .map_err(|message| RestartFailure::internal(format!("probe failed: {message}")))?; - let state = replacement_from_probe(&output, port, expected); - if state != ReplacementState::Absent { - return Ok(state); - } - if attempt + 1 < attempts { - runtime.sleep(interval); - } - } - Ok(ReplacementState::Absent) -} - -fn replacement_from_probe( - output: &ProbeOutput, - port: u16, - expected: &LaunchInfo, -) -> ReplacementState { - action::classify_replacement( - expected, - output - .sockets - .iter() - .filter(|socket| socket.port == port) - .map(|socket| { - ( - socket.pid, - socket - .process - .as_ref() - .and_then(|process| process.launch.as_ref()), - ) - }), - ) -} - -#[cfg(test)] -mod tests { - use std::collections::VecDeque; - use std::ffi::OsString; - use std::net::{IpAddr, Ipv4Addr}; - - use super::*; - use crate::probe::{ListeningSocket, ProcessInfo, Protocol}; - use crate::snapshot::Exposure; - - struct MockRuntime { - snapshot: Option>, - terminate: Option>, - probes: VecDeque>, - spawn: Option>, - snapshot_calls: usize, - terminate_calls: Vec<(u32, bool)>, - probe_calls: usize, - spawn_calls: usize, - sleep_calls: usize, - } - - impl RestartRuntime for MockRuntime { - fn snapshot(&mut self) -> Result { - self.snapshot_calls += 1; - self.snapshot - .take() - .unwrap_or_else(|| Err("unexpected snapshot call".into())) - } - - fn terminate(&mut self, pid: u32, force: bool) -> Result<(), TerminateFailure> { - self.terminate_calls.push((pid, force)); - self.terminate.take().unwrap_or(Ok(())) - } - - fn probe(&mut self) -> Result { - self.probe_calls += 1; - self.probes - .pop_front() - .unwrap_or_else(|| Ok(ProbeOutput::default())) - } - - fn spawn(&mut self, _launch: &LaunchInfo) -> Result { - self.spawn_calls += 1; - self.spawn.take().unwrap_or(Ok(9000)) - } - - fn sleep(&mut self, _duration: Duration) { - self.sleep_calls += 1; - } - } - - fn launch(label: &str) -> LaunchInfo { - LaunchInfo::from_parts( - Some(std::env::current_exe().expect("current executable")), - [label, "--serve"].map(OsString::from).to_vec(), - Some(std::env::current_dir().expect("current directory")), - ) - .expect("complete launch info") - } - - fn service(pid: Option, launch: Option) -> Service { - Service { - id: "svc-3000-node".into(), - port: 3000, - pid, - process_name: Some("node".into()), - command: Some("node --serve".into()), - cwd: Some("/project".into()), - user: Some("brad".into()), - project_id: None, - framework: Some("Node.js".into()), - exposure: Exposure::Local, - url: Some("http://localhost:3000".into()), - started_age: Some("1m".into()), - stale: None, - restartable: launch.is_some(), - restart_blocked_reason: launch - .is_none() - .then(|| "Executable, arguments, or working directory are unavailable".into()), - launch, - } - } - - fn snapshot(service: Service) -> DevSnapshot { - DevSnapshot { - generated_at: 1, - services: vec![service], - projects: Vec::new(), - conflicts: Vec::new(), - docker_hints: Vec::new(), - } - } - - fn socket(pid: Option, launch: Option) -> ListeningSocket { - ListeningSocket { - protocol: Protocol::Tcp, - local_addr: IpAddr::V4(Ipv4Addr::LOCALHOST), - port: 3000, - pid, - process: pid.map(|pid| ProcessInfo { - pid, - name: Some("node".into()), - command: Some("node --serve".into()), - cwd: Some(std::env::current_dir().expect("current directory")), - user: Some("brad".into()), - started_secs_ago: Some(1), - launch, - }), - uid: None, - user: None, - } - } - - fn output(socket: ListeningSocket) -> ProbeOutput { - ProbeOutput { - sockets: vec![socket], - } - } - - fn runtime(service: Service) -> MockRuntime { - MockRuntime { - snapshot: Some(Ok(snapshot(service))), - terminate: Some(Ok(())), - probes: VecDeque::new(), - spawn: Some(Ok(9000)), - snapshot_calls: 0, - terminate_calls: Vec::new(), - probe_calls: 0, - spawn_calls: 0, - sleep_calls: 0, - } - } - - fn request(pid: u32, force: bool) -> RestartRequest { - RestartRequest { - service_id: "svc-3000-node".into(), - pid, - force, - } - } - - #[test] - fn request_rejects_browser_supplied_launch_data() { - let json = serde_json::json!({ - "service_id": "svc-3000-node", - "pid": 10, - "force": false, - "executable": "/tmp/anything" - }); - assert!(serde_json::from_value::(json).is_err()); - } - - #[test] - fn target_validation_rejects_missing_changed_and_unknown_owners() { - let launch = launch("node"); - let current = snapshot(service(Some(10), Some(launch.clone()))); - let missing = verify_target(¤t, "missing", 10) - .err() - .expect("missing service"); - assert_eq!(missing.status, StatusCode::CONFLICT); - - let changed = verify_target(¤t, "svc-3000-node", 11) - .err() - .expect("changed pid"); - assert_eq!(changed.status, StatusCode::CONFLICT); - - let unknown = snapshot(service(None, Some(launch))); - let unknown = verify_target(&unknown, "svc-3000-node", 10) - .err() - .expect("unknown owner"); - assert_eq!(unknown.status, StatusCode::BAD_REQUEST); - } - - #[test] - fn self_refusal_happens_before_probe_or_signal() { - let launch = launch("portdoc"); - let mut runtime = runtime(service(Some(std::process::id()), Some(launch))); - let failure = execute_with(request(std::process::id(), false), &mut runtime) - .expect_err("PortDoc must not restart itself"); - assert_eq!(failure.status, StatusCode::FORBIDDEN); - assert_eq!(runtime.snapshot_calls, 0); - assert!(runtime.terminate_calls.is_empty()); - } - - #[test] - fn matching_supervisor_replacement_prevents_duplicate_spawn() { - let expected = launch("node"); - let mut runtime = runtime(service(Some(10), Some(expected.clone()))); - runtime.probes.extend([ - Ok(ProbeOutput::default()), - Ok(output(socket(Some(20), Some(expected)))), - ]); - - let result = execute_with(request(10, false), &mut runtime).expect("restart result"); - assert_eq!(result.outcome, RestartOutcome::SupervisorRestarted); - assert_eq!(result.pid, Some(20)); - assert_eq!(runtime.spawn_calls, 0); - } - - #[test] - fn reused_pid_after_proven_release_still_prevents_duplicate_spawn() { - let expected = launch("node"); - let mut runtime = runtime(service(Some(10), Some(expected.clone()))); - runtime.probes.extend([ - Ok(ProbeOutput::default()), - Ok(output(socket(Some(10), Some(expected)))), - ]); - - let result = execute_with(request(10, false), &mut runtime).expect("restart result"); - assert_eq!(result.outcome, RestartOutcome::SupervisorRestarted); - assert_eq!(result.pid, Some(10)); - assert_eq!(runtime.spawn_calls, 0); - } - - #[test] - fn unrelated_port_replacement_prevents_duplicate_spawn() { - let expected = launch("node"); - let mut unrelated = launch("other"); - unrelated.argv[1] = "--other".into(); - let mut runtime = runtime(service(Some(10), Some(expected))); - runtime.probes.extend([ - Ok(ProbeOutput::default()), - Ok(output(socket(Some(20), Some(unrelated)))), - ]); - - let result = execute_with(request(10, false), &mut runtime).expect("restart result"); - assert_eq!(result.outcome, RestartOutcome::PortReplaced); - assert_eq!(result.pid, Some(20)); - assert_eq!(runtime.spawn_calls, 0); - } - - #[test] - fn failed_release_returns_force_escalation_without_spawning() { - let expected = launch("node"); - let mut runtime = runtime(service(Some(10), Some(expected.clone()))); - for _ in 0..RELEASE_ATTEMPTS { - runtime - .probes - .push_back(Ok(output(socket(Some(10), Some(expected.clone()))))); - } - - let result = execute_with(request(10, false), &mut runtime).expect("restart result"); - assert_eq!(result.outcome, RestartOutcome::StillListening); - assert_eq!(result.pid, None); - assert_eq!(runtime.spawn_calls, 0); - assert_eq!(runtime.probe_calls, RELEASE_ATTEMPTS as usize); - } - - #[test] - fn force_request_is_forwarded_to_the_verified_termination() { - let expected = launch("node"); - let mut runtime = runtime(service(Some(10), Some(expected))); - runtime.terminate = Some(Err(TerminateFailure::NotPermitted("denied".into()))); - - let failure = execute_with(request(10, true), &mut runtime).expect_err("permission error"); - assert_eq!(failure.status, StatusCode::FORBIDDEN); - assert_eq!(runtime.terminate_calls, vec![(10, true)]); - assert_eq!(runtime.probe_calls, 0); - } - - #[test] - fn spawn_failure_is_visible_after_supervisor_window() { - let expected = launch("node"); - let mut runtime = runtime(service(Some(10), Some(expected))); - runtime.spawn = Some(Err("binary missing".into())); - - let failure = execute_with(request(10, false), &mut runtime).expect_err("spawn failure"); - assert_eq!(failure.status, StatusCode::INTERNAL_SERVER_ERROR); - assert!(failure.message.contains("binary missing")); - assert_eq!(runtime.probe_calls, 1 + SUPERVISOR_ATTEMPTS as usize); - assert_eq!(runtime.spawn_calls, 1); - } - - #[test] - fn spawned_replacement_returning_to_port_is_success() { - let expected = launch("node"); - let mut replacement = expected.clone(); - replacement.argv[0] = "/canonical/node".into(); - let mut runtime = runtime(service(Some(10), Some(expected.clone()))); - for _ in 0..=SUPERVISOR_ATTEMPTS { - runtime.probes.push_back(Ok(ProbeOutput::default())); - } - runtime - .probes - .push_back(Ok(output(socket(Some(30), Some(replacement))))); - - let result = execute_with(request(10, false), &mut runtime).expect("restart result"); - assert_eq!(result.outcome, RestartOutcome::Restarted); - assert_eq!(result.pid, Some(30)); - assert_eq!(runtime.spawn_calls, 1); - assert_eq!( - serde_json::to_value(&result).expect("response json"), - serde_json::json!({ "outcome": "restarted", "pid": 30 }) - ); - } - - #[test] - fn spawned_process_that_never_listens_returns_bounded_outcome() { - let expected = launch("node"); - let mut runtime = runtime(service(Some(10), Some(expected))); - - let result = execute_with(request(10, false), &mut runtime).expect("restart result"); - assert_eq!(result.outcome, RestartOutcome::NotListening); - assert_eq!(result.pid, None); - assert_eq!(runtime.spawn_calls, 1); - assert_eq!( - runtime.probe_calls, - 1 + SUPERVISOR_ATTEMPTS as usize + START_ATTEMPTS as usize - ); - } - - #[test] - fn unreadable_pid_on_returned_port_is_reported_without_spawning() { - let expected = launch("node"); - let mut runtime = runtime(service(Some(10), Some(expected))); - runtime - .probes - .extend([Ok(ProbeOutput::default()), Ok(output(socket(None, None)))]); - - let result = execute_with(request(10, false), &mut runtime).expect("restart result"); - assert_eq!(result.outcome, RestartOutcome::PortReplaced); - assert_eq!(result.pid, None); - assert_eq!(runtime.spawn_calls, 0); - } -} diff --git a/src/snapshot.rs b/src/snapshot.rs index a07f8a5..39b53cb 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -4,8 +4,6 @@ use serde::Serialize; -use crate::probe::LaunchInfo; - #[derive(Serialize)] pub struct DevSnapshot { /// Unix epoch milliseconds when the snapshot was built. @@ -41,12 +39,6 @@ pub struct Service { pub started_age: Option, #[serde(skip_serializing_if = "Option::is_none")] pub stale: Option, - pub restartable: bool, - #[serde(skip_serializing_if = "Option::is_none")] - pub restart_blocked_reason: Option, - /// Exact launch data stays inside the server and never enters snapshot JSON. - #[serde(skip)] - pub launch: Option, } #[derive(Serialize)] diff --git a/web/src/App.tsx b/web/src/App.tsx index 8383e97..a8067bd 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -2,7 +2,6 @@ import { useState } from 'react' import { TriangleAlert } from 'lucide-react' import { useSnapshot } from './lib/useSnapshot' import { StopAllContext, StopContext, type StopAllRequest } from './lib/stop' -import { RestartContext } from './lib/restart' import { ConfigContext, useConfigState } from './lib/config' import { InspectContext, type InspectTarget } from './lib/inspect' import type { ProjectGroup, Service } from './lib/types' @@ -15,7 +14,6 @@ import { ProjectGroups } from './components/ProjectGroups' import { ServicesTable } from './components/ServicesTable' import { StopDialog } from './components/StopDialog' import { StopAllDialog } from './components/StopAllDialog' -import { RestartDialog } from './components/RestartDialog' import { InspectDrawer } from './components/InspectDrawer' import { ProjectDrawer } from './components/ProjectDrawer' import { Button } from './components/ui/button' @@ -25,7 +23,6 @@ export default function App() { const [query, setQuery] = useState('') const [stopTarget, setStopTarget] = useState(null) const [stopAllTarget, setStopAllTarget] = useState(null) - const [restartTarget, setRestartTarget] = useState(null) const [inspect, setInspect] = useState(null) const [projectTarget, setProjectTarget] = useState(null) const { snapshot, error, loading, fetchedAt, refresh } = useSnapshot() @@ -49,7 +46,6 @@ export default function App() { return ( - @@ -112,18 +108,10 @@ export default function App() { }} /> )} - {restartTarget && ( - setRestartTarget(null)} - onChanged={() => void refresh()} - /> - )} - ) } diff --git a/web/src/components/Callouts.tsx b/web/src/components/Callouts.tsx index 11a9477..69ec697 100644 --- a/web/src/components/Callouts.tsx +++ b/web/src/components/Callouts.tsx @@ -1,14 +1,12 @@ import { useState } from 'react' -import { Clock, RotateCw } from 'lucide-react' +import { Clock } from 'lucide-react' import type { DevSnapshot } from '../lib/types' -import { canStop, restartBlockedReason, staleServices, stopBlockedReason } from '../lib/derive' -import { useRequestRestart } from '../lib/restart' +import { canStop, staleServices, stopBlockedReason } from '../lib/derive' import { useRequestStop } from '../lib/stop' import { useConfig } from '../lib/config' import { Button } from './ui/button' export function Callouts({ snapshot }: { snapshot: DevSnapshot }) { - const requestRestart = useRequestRestart() const requestStop = useRequestStop() const { ignored } = useConfig() return ( @@ -28,15 +26,6 @@ export function Callouts({ snapshot }: { snapshot: DevSnapshot }) { - - - )} - - -
-
- {field('Port', `:${service.port}`)} - {service.pid !== undefined && field('PID', String(service.pid))} - {service.command && field('Command', service.command)} - {service.cwd && field('Path', service.cwd)} -
- - {blockedReason && phase === 'confirm' && ( - {blockedReason} - )} - {!blockedReason && phase === 'confirm' && ( -

- PortDoc will request a normal stop, verify the port, wait briefly for a supervisor, - and relaunch only if the port stays free. -

- )} - {phase === 'working' && ( -

Stopping, checking the port, and starting the replacement.

- )} - {phase === 'escalate' && ( - - The process ignored the normal stop and is still listening. Force restart ends it - immediately without cleanup, then attempts the same verified relaunch. - - )} - {phase === 'result' && result && } - {phase === 'error' && {message}} -
- -
- {phase === 'confirm' && ( - <> - - - - )} - {phase === 'working' && ( - - )} - {phase === 'escalate' && ( - <> - - - - )} - {(phase === 'result' || phase === 'error') && ( - - )} -
- - - ) -} - -function dialogTitle(phase: Phase, service: Service): string { - if (phase === 'escalate') return 'Force restart?' - if (phase === 'result') return 'Restart result' - if (phase === 'error') return 'Restart failed' - return `Restart ${service.process_name ?? 'service'}?` -} - -function Status({ - variant, - children, -}: { - variant: 'warn' | 'danger' - children: ReactNode -}) { - return ( -

- - {children} -

- ) -} - -function ResultMessage({ result, port }: { result: RestartResult; port: number }) { - switch (result.outcome) { - case 'restarted': - return ( - - PortDoc started a replacement on :{port} - {result.pid !== undefined ? ` with PID ${result.pid}` : ''}. - - ) - case 'supervisor_restarted': - return ( - - A supervisor started the matching replacement - {result.pid !== undefined ? ` with PID ${result.pid}` : ''}. PortDoc did not launch a - duplicate. - - ) - case 'still_listening': - return The process is still listening after the force stop. - case 'port_replaced': - return ( - - Another process claimed :{port} - {result.pid !== undefined ? ` with PID ${result.pid}` : ''}. PortDoc did not launch a - duplicate. - - ) - case 'not_listening': - return ( - - The replacement launched but did not return to :{port} within the verification window. - - ) - } -} - -function Success({ children }: { children: ReactNode }) { - return ( -

- - {children} -

- ) -} diff --git a/web/src/components/ServiceRow.tsx b/web/src/components/ServiceRow.tsx index 8913b0a..617f4c8 100644 --- a/web/src/components/ServiceRow.tsx +++ b/web/src/components/ServiceRow.tsx @@ -1,14 +1,6 @@ -import { EllipsisVertical, ExternalLink, RotateCw, Shield, Square } from 'lucide-react' +import { EllipsisVertical, ExternalLink, Shield, Square } from 'lucide-react' import type { DockerHint, Service } from '../lib/types' -import { - canStop, - displayName, - isSelf, - restartBlockedReason, - stopBlockedReason, - wellKnownHint, -} from '../lib/derive' -import { useRequestRestart } from '../lib/restart' +import { canStop, displayName, isSelf, stopBlockedReason, wellKnownHint } from '../lib/derive' import { useRequestStop } from '../lib/stop' import { useInspect } from '../lib/inspect' import { Badge } from './ui/badge' @@ -38,7 +30,6 @@ function subLine(service: Service, dockerHint?: DockerHint): string | undefined export function ServiceRow({ service, conflicted, dockerHint }: ServiceRowProps) { const self = isSelf(service) const sub = subLine(service, dockerHint) - const requestRestart = useRequestRestart() const requestStop = useRequestStop() const inspect = useInspect() const stoppable = canStop(service) @@ -80,16 +71,6 @@ export function ServiceRow({ service, conflicted, dockerHint }: ServiceRowProps) Open - -