From baff35ea2d1e93b349e4b4ec031163dacf8c7b28 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 11 Oct 2026 16:24:46 -0400 Subject: [PATCH 1/4] refactor(daemon): remove pid-file runtime adoption PV 0.2.6 moved every runtime onto monitors and kept the pid-file code only to stop runtimes an older release left running. Once every install runs 0.2.6, that code has nothing left to do, so it goes: - the legacy stop and its pid-file identity checks (argv, script and executable matching) in the supervisor - the cutover that every reconciliation and update job ran first - the pid-file branch of disable and uninstall, which now remove only JSON records that no monitor backs - the pid_path field of ProcessSpec and ManagedProcess, the pid path helpers in PvPaths, and adopt_recorded's unused pid argument - RuntimeMetadata's boot_session_id, process_start_identity and process_executable_identity fields. Old records still parse, since serde ignores the keys. A record without a monitor instance no longer counts as config proof. Releases after 0.2.6 must require 0.2.6, so `pv update` can't skip the cutover; DESIGN.md now says so. CONTRIBUTING.md describes fixtures under monitors and drops the script-identity tests. --- CONTRIBUTING.md | 50 +-- DESIGN.md | 4 +- crates/cli/tests/daemon.rs | 57 --- crates/cli/tests/support/runtime.rs | 3 +- crates/daemon/src/gateway.rs | 17 +- crates/daemon/src/health.rs | 7 +- crates/daemon/src/jobs.rs | 103 +----- crates/daemon/src/managed_resources/fake.rs | 1 - .../daemon/src/managed_resources/mailpit.rs | 1 - crates/daemon/src/managed_resources/mod.rs | 3 - crates/daemon/src/managed_resources/mysql.rs | 1 - .../daemon/src/managed_resources/postgres.rs | 1 - crates/daemon/src/managed_resources/redis.rs | 1 - .../managed_resources/runtime_contracts.rs | 12 +- crates/daemon/src/managed_resources/rustfs.rs | 1 - crates/daemon/src/managed_resources/tests.rs | 28 +- crates/daemon/src/runtime_stop.rs | 54 +-- crates/daemon/src/structured_log.rs | 11 - crates/daemon/src/supervisor.rs | 332 +----------------- .../supervisor/owned-python-runtime.py | 32 -- crates/daemon/tests/daemon_foundation.rs | 121 +------ crates/daemon/tests/gateway_reconciliation.rs | 246 ++++++------- ..._command_and_process_specs_are_stable.snap | 2 - ...rivate_environment_and_redacts_values.snap | 4 +- crates/daemon/tests/supervisor_foundation.rs | 54 +-- crates/state/src/paths.rs | 13 - crates/state/tests/state_foundation.rs | 12 - 27 files changed, 189 insertions(+), 982 deletions(-) delete mode 100644 crates/daemon/test-fixtures/supervisor/owned-python-runtime.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 22536affc..9d29a7035 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,7 +20,7 @@ Avoid `panic!`, `unreachable!`, `.unwrap()`, and `.expect()` in production code. For running tests, we recommend [nextest](https://nexte.st/). -Some tests require `python3` on `PATH`: the supervisor's script-identity tests, two supervisor process-group tests, and `pv-release`'s installer and smoke tests. They use only Python's standard library; no Python packages or virtual environment is required. +Some tests require `python3` on `PATH`: two supervisor process-group tests, and `pv-release`'s installer and smoke tests. They use only Python's standard library; no Python packages or virtual environment is required. To run a specific test by name: @@ -42,21 +42,24 @@ cargo insta test --accept --test-runner nextest -- ## Fixture Lifecycle -Tests that start long-running Managed Resource or Gateway fixtures must register -each intended process's exact PID-path/runtime-metadata-path pair -before startup or reconciliation. Capture assertions while the runtime is live, -then use explicit cleanup where the guard exposes it so cleanup failures fail -the test. If an operation has already failed, preserve that primary failure -while also reporting cleanup failures. Cleanup validates the recorded metadata -and process identity, stops the complete owned process group, waits for its -members and listeners to disappear, and removes the exact records only after -verified stop. - -Fixture guards must also clean up from `Drop` so normal return, early return, -panic unwind, and cancellation cannot bypass teardown. The fallback uses an -independent cleanup runtime and reports failures because `Drop` cannot return -them to the test. The Rust guard or supervisor owns whole-process-group -cleanup. +Tests that start long-running Managed Resource or Gateway runtimes run them +under the test monitor. Fixture setup calls `pv_fake::install_monitor(paths)`. +That links the daemon's `pv-monitor` example into the test's PV home as its +`pv` binary and gives the monitors the test process's lifeline. Capture +assertions while the runtime is live, then use explicit cleanup where the guard +exposes it so cleanup failures fail the test. If an operation has already +failed, preserve that primary failure while also reporting cleanup failures. +Cleanup goes through `stop_recorded_runtimes` or +`ProcessSupervisor::stop_subject`. The runtime's monitor stops its whole +process group and waits for the members to exit. PV removes the records only +after that verified stop. + +Fixture guards also clean up from `Drop` where they can, so normal return, +early return, panic unwind, and cancellation cannot bypass teardown. The +fallback uses an independent cleanup runtime and reports failures, because +`Drop` cannot return them to the test. Whenever the test process exits, even by +SIGKILL, its lifeline closes and every monitor it started stops its runtime and +exits. A failed test therefore leaves nothing running. Long-running fixtures that stand in for a runtime use `pv-fake`, not a script. `pv_fake::install(executable, persona)` places a native fake at the @@ -65,15 +68,13 @@ it dies, SIGKILL included, so it needs no parent watcher. It records its lifecycle in `.pv-fake.events.jsonl`. `cargo nextest run` builds it as the daemon's `pv-fake` example, but runs narrowed with `--lib` or `--test ` do not; filter with `-E` instead. A fake from an older build -refuses to start and says how to rebuild. Guard registration and explicit -cleanup still apply. To stop a fake at a known point, pass `FakeSettings` to +refuses to start and says how to rebuild. Explicit cleanup still applies. To stop a fake at a known point, pass `FakeSettings` to `pv_fake::install_with_settings` and wait on its events, such as `held` and `descendant_spawned`, instead of marker files. Steer a running Gateway fake with `pv_fake::write_gateway_control`, which rejects unknown keys. Shell scripts remain only where a test needs what the fake doesn't do: the -supervisor's script-identity tests, which need a script as the runtime, and -the process-group tests, which need a child that outlives its parent or ignores +process-group tests, which need a child that outlives its parent or ignores SIGTERM. A script that stays alive must give up on its own, so a test that dies before stopping it leaves nothing running for long, but not before the CI profile's 120-second test limit, so a stalled test can't pass because its @@ -102,10 +103,11 @@ cargo nextest store export-chrome-trace nextest-run.zip -o trace.json `NEXTEST_EXPERIMENTAL_RECORD=1 cargo nextest replay -R nextest-run.zip` replays the run in the terminal. -Persisted-runtime cleanup must never discover ownership through process-name -scans or signal a PID found only in a record. A persisted PID is actionable -only together with matching recorded metadata and a verified process identity. -A test may signal and reap a child it directly spawned and still owns. These +Runtime cleanup must never discover ownership through process-name scans or +signal a PID found only in a record. A runtime is stopped through its monitor. +If the monitor has died, recovery signals the runtime's process group only +while the runtime identity and boot session in the monitor's record still +match. A test may signal and reap a child it directly spawned and still owns. These guarantees do not cover machine or kernel failure, a test process that is stopped rather than killed (its fakes keep running), or descendants that deliberately escape their owned process group. diff --git a/DESIGN.md b/DESIGN.md index 9911bfff9..bd96e5be8 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -266,8 +266,6 @@ Before reconciliation starts a runtime, PV stops and releases any monitor alread `pv daemon:disable` and `pv uninstall` stop and release every monitor, then remove records that no monitor backs. Only a successful Postgres exit status proves that its escaped backends are gone. Postgres exits 0 only after its backends exit, so a held exit 0 counts whoever stopped it. If Postgres ended by a signal, including a force-kill, or with another exit code, or its monitor is unavailable during the current boot, PV keeps its monitor and records; `--force` does not bypass this proof. A monitor record from an earlier boot is removed without signalling anything it names. A monitor record PV cannot read, such as one from another monitor protocol version, makes the command fail and keeps the records. -Runtimes that a PV release from before monitors started have only a pid file and a JSON record with the process start identity and boot session. Every reconciliation or update job first stops each one after verifying that identity, removes its pid file, and keeps its JSON record so reconciliation can use its config proofs and restart it under a monitor. The startup job need not run first: `pv update` queues its own job as soon as the new daemon is up. A job logs a runtime it could not stop and continues. A record from an earlier kernel boot is removed without signalling its PID, which may have been reused. `pv daemon:disable` and `pv uninstall` stop these runtimes the same way and remove both files. A live recorded PID that fails verification makes the command fail and keeps the records. - After daemon restart, PV adopts running runtimes through their monitors when their spec and config match desired state, so their PIDs stay the same. Reconciliation restarts runtimes that are stale, mismatched, exited, or no longer desired. PV starts each supervised runtime in its own process group so it can stop the entire PV-owned process tree safely. The monitor runs each stop: the graceful signal, the grace period, then a force-kill of whatever remains in the group. @@ -736,7 +734,7 @@ The daemon writes a minimal startup failure marker at `~/.pv/run/daemon-startup- `pv update` does not create an extra `pv.db` backup before every PV application self-update. The embedded migration system creates a timestamped backup only when migrations are about to run. -PV application self-update restarts the daemon/control plane but does not stop currently running Gateway, Project-serving workers, or backing Managed Resource processes before swapping the PV binary. The updated daemon automatically schedules startup System reconciliation to apply current desired state, adopting running runtimes through their monitors. A monitor never re-executes its binary after startup, so pruning an older release does not affect it. +PV application self-update restarts the daemon/control plane but does not stop currently running Gateway, Project-serving workers, or backing Managed Resource processes before swapping the PV binary. The updated daemon automatically schedules startup System reconciliation to apply current desired state, adopting running runtimes through their monitors. A monitor never re-executes its binary after startup, so pruning an older release does not affect it. PV 0.2.6 is the only release that moves runtimes PV started before monitors, which it tracked by pid files, onto monitors. Later releases set `minimum_pv_version` to at least 0.2.6 so `pv update` cannot skip that step. App rollback is attempted after activation was attempted. Helper rollback is also attempted when helper replacement itself fails, before any incompatible app can be activated. If daemon startup, daemon health, or migration health fails after activation and rollback succeeds, stdout reports `PV application: update failed; rolled back to ` and stderr reports the original failure. A generic daemon health failure is reported as `daemon did not become healthy after update`; migration failure is reported as `database migration failed after update: `. diff --git a/crates/cli/tests/daemon.rs b/crates/cli/tests/daemon.rs index 27831d9a0..40623898a 100644 --- a/crates/cli/tests/daemon.rs +++ b/crates/cli/tests/daemon.rs @@ -367,21 +367,18 @@ fn daemon_disable_stops_live_runtimes_without_a_plist_or_database() -> anyhow::R worker.command = paths .resources() .join("frankenphp/8.5/fixture/bin/frankenphp"); - worker.pid_path = paths.worker_pid("8.5-test"); worker.metadata_path = paths.worker_runtime_metadata("8.5-test"); let mut resource = gateway.clone(); resource.name = "mailpit".to_owned(); resource.resource_name = "mailpit".to_owned(); resource.track = "1".to_owned(); resource.command = paths.resources().join("mailpit/1/fixture/bin/mailpit"); - resource.pid_path = paths.resource_pid("mailpit", "1"); resource.metadata_path = paths.resource_runtime_metadata("mailpit", "1"); let mut mysql = resource.clone(); mysql.name = "mysql".to_owned(); mysql.resource_name = "mysql".to_owned(); mysql.track = "8.4".to_owned(); mysql.command = paths.resources().join("mysql/8.4/fixture/bin/mysqld"); - mysql.pid_path = paths.resource_pid("mysql", "8.4"); mysql.metadata_path = paths.resource_runtime_metadata("mysql", "8.4"); let mut fixtures = Vec::new(); for spec in [gateway, worker, resource, mysql] { @@ -441,7 +438,6 @@ fn daemon_disable_waits_for_process_death_after_its_socket_is_gone() -> anyhow:: let mut daemon_spec = gateway_spec(&paths); daemon_spec.command = paths.bin().join("releases/fixture/pv"); daemon_spec.arguments = vec!["daemon:run".to_owned()]; - daemon_spec.pid_path = paths.run().join("fixture-daemon.pid"); daemon_spec.metadata_path = paths.run().join("fixture-daemon.json"); let mut daemon = RuntimeFixture::start(&paths, daemon_spec)?; let pid = daemon.pid()?; @@ -493,7 +489,6 @@ fn daemon_disable_keeps_dead_postgres_records_when_cleanup_is_unproven() -> anyh let mut spec = gateway_spec(&paths); spec.resource_name = "postgres".to_owned(); spec.track = "18".to_owned(); - spec.pid_path = paths.resource_pid("postgres", "18"); spec.metadata_path = paths.resource_runtime_metadata("postgres", "18"); let mut runtime = RuntimeFixture::start(&paths, spec)?; // Killed, Postgres exits by signal, which proves nothing about its backends. @@ -511,55 +506,6 @@ fn daemon_disable_keeps_dead_postgres_records_when_cleanup_is_unproven() -> anyh Ok(()) } -#[test] -fn daemon_disable_removes_previous_boot_postgres_records_without_signalling_reused_pid() --> anyhow::Result<()> { - let tempdir = tempdir()?; - let home = tempdir.path().join("home"); - let paths = PvPaths::for_home(&home); - let environment = TestEnvironment::new( - &home, - tempdir.path(), - &paths.active_pv_binary(), - &tempdir.path().join("absent.plist"), - ); - // A process outside PV now holds the pid that a previous boot's Postgres recorded. - let foreign = ChildGuard( - TestCommand::new("/bin/sleep") - .arg("30") - .process_group(0) - .spawn()?, - ); - let foreign_pid = foreign.0.id(); - let previous_boot = - platform::BootSessionId::try_from("00000000-0000-0000-0000-000000000001".to_owned())?; - assert_ne!(previous_boot, platform::current_boot_session_id()?); - let pid_path = paths.resource_pid("postgres", "18"); - let metadata_path = paths.resource_runtime_metadata("postgres", "18"); - state::fs::write_sensitive_file(&pid_path, &format!("{foreign_pid}\n"))?; - let metadata = json!({ - "name": "postgres", - "pid": foreign_pid, - "command": "/bin/sleep", - "arguments": ["30"], - "resource_name": "postgres", - "track": "18", - "log_path": paths.logs().join("postgres.log"), - "started_at": "2026-10-09T00:00:00Z", - "boot_session_id": previous_boot, - "process_start_identity": platform::inspect_process_start_identity(foreign_pid)?, - }); - state::fs::write_sensitive_file(&metadata_path, &serde_json::to_string(&metadata)?)?; - - let output = run_pv(&["daemon:disable"], &environment)?; - assert_eq!(output.exit_code, ExitCode::SUCCESS); - assert!(!state::fs::path_entry_exists(&pid_path)?); - assert!(!state::fs::path_entry_exists(&metadata_path)?); - assert!(platform::process_group_has_live_members(foreign_pid)?); - drop(foreign); - Ok(()) -} - #[test] fn daemon_disable_clears_a_previous_boot_postgres_monitor_without_signalling_reused_pid() -> anyhow::Result<()> { @@ -575,7 +521,6 @@ fn daemon_disable_clears_a_previous_boot_postgres_monitor_without_signalling_reu let mut spec = gateway_spec(&paths); spec.resource_name = "postgres".to_owned(); spec.track = "18".to_owned(); - spec.pid_path = paths.resource_pid("postgres", "18"); spec.metadata_path = paths.resource_runtime_metadata("postgres", "18"); let mut runtime = RuntimeFixture::start(&paths, spec)?; runtime.cleanup()?; @@ -639,7 +584,6 @@ fn daemon_replacement_waits_for_delayed_exit_before_bootstrap() -> anyhow::Resul let mut spec = gateway_spec(&paths); spec.command = paths.bin().join("releases/fixture/pv"); spec.arguments = vec!["daemon:run".to_owned()]; - spec.pid_path = paths.run().join("fixture-daemon.pid"); spec.metadata_path = paths.run().join("fixture-daemon.json"); let mut previous = RuntimeFixture::start(&paths, spec)?; let pid = previous.pid()?; @@ -714,7 +658,6 @@ fn daemon_replacement_refuses_to_bootstrap_while_previous_daemon_is_alive() -> a let mut spec = gateway_spec(&paths); spec.command = paths.bin().join("releases/fixture/pv"); spec.arguments = vec!["daemon:run".to_owned()]; - spec.pid_path = paths.run().join("fixture-daemon.pid"); spec.metadata_path = paths.run().join("fixture-daemon.json"); let mut daemon = RuntimeFixture::start(&paths, spec)?; let pid = daemon.pid()?; diff --git a/crates/cli/tests/support/runtime.rs b/crates/cli/tests/support/runtime.rs index ef770a4f3..1cb59e4b5 100644 --- a/crates/cli/tests/support/runtime.rs +++ b/crates/cli/tests/support/runtime.rs @@ -39,7 +39,7 @@ impl RuntimeFixture { fixture.process = Some(runtime.block_on(supervisor.start(fixture.spec.clone()))?); fixture.ownership = Some( supervisor - .adopt_recorded(&fixture.spec.pid_path, &fixture.spec.metadata_path)? + .adopt_recorded(&fixture.spec.metadata_path)? .ok_or_else(|| anyhow!("fixture did not publish ownership"))?, ); for _attempt in 0..200 { @@ -140,7 +140,6 @@ pub fn gateway_spec(paths: &PvPaths) -> ProcessSpec { config_path: paths.gateway_root_config(), config_fingerprint: None, log_path: paths.gateway_supervisor_log(), - pid_path: paths.gateway_pid(), metadata_path: paths.gateway_runtime_metadata(), resource_name: "caddy".to_owned(), track: "2".to_owned(), diff --git a/crates/daemon/src/gateway.rs b/crates/daemon/src/gateway.rs index f18b773b5..8a8a1fb05 100644 --- a/crates/daemon/src/gateway.rs +++ b/crates/daemon/src/gateway.rs @@ -2615,7 +2615,6 @@ pub fn gateway_process_spec(paths: &PvPaths, command: &CaddyCliCommand) -> Proce config_path: paths.gateway_root_config(), config_fingerprint: None, log_path: paths.gateway_supervisor_log(), - pid_path: paths.gateway_pid(), metadata_path: paths.gateway_runtime_metadata(), resource_name: "caddy".to_owned(), track: "2".to_owned(), @@ -2636,7 +2635,6 @@ pub fn worker_process_spec( config_path: paths.worker_root_config(&worker.runtime_key), config_fingerprint: None, log_path: paths.worker_log(&worker.runtime_key), - pid_path: paths.worker_pid(&worker.runtime_key), metadata_path: paths.worker_runtime_metadata(&worker.runtime_key), resource_name: "frankenphp".to_owned(), track: worker.runtime_key.clone(), @@ -4760,19 +4758,7 @@ async fn cleanup_fresh_runtime( } fn cleanup_fresh_runtime_files(spec: &ProcessSpec) -> Result<(), DaemonError> { - let pid_error = delete_optional_file(&spec.pid_path).err(); - let metadata_error = delete_optional_file(&spec.metadata_path).err(); - - match (pid_error, metadata_error) { - (None, None) => Ok(()), - (Some(error), None) | (None, Some(error)) => Err(error), - (Some(pid_error), Some(metadata_error)) => Err(DaemonError::UnexpectedProtocolResponse { - reason: format!( - "failed to remove runtime `{}` pid file: {pid_error}; failed to remove metadata file: {metadata_error}", - spec.name - ), - }), - } + delete_optional_file(&spec.metadata_path) } #[derive(Debug)] @@ -5343,7 +5329,6 @@ fn cleanup_stale_worker_runtime(paths: &PvPaths, runtime_key: &str) -> Result<() php_runtime_key: runtime_key.to_owned(), })?; - delete_optional_file(&paths.worker_pid(runtime_key))?; delete_optional_file(&paths.worker_admin_socket(runtime_key))?; delete_optional_dir(&paths.worker_config_dir(runtime_key))?; delete_optional_file(&paths.worker_runtime_metadata(runtime_key))?; diff --git a/crates/daemon/src/health.rs b/crates/daemon/src/health.rs index 7d7f33b03..8eab9ed3c 100644 --- a/crates/daemon/src/health.rs +++ b/crates/daemon/src/health.rs @@ -347,7 +347,6 @@ fn collect_runtime_health_probes( let scope = ReconciliationScope::resource("caddy", "2")?; let (current, error) = recorded_runtime_is_current( &supervisor, - &paths.gateway_pid(), &paths.gateway_runtime_metadata(), caddy.current_artifact_path.as_deref(), true, @@ -401,7 +400,6 @@ fn collect_runtime_health_probes( }); let (current, error) = recorded_runtime_is_current( &supervisor, - &paths.worker_pid(&runtime_key), &paths.worker_runtime_metadata(&runtime_key), Some(artifact_root), true, @@ -439,7 +437,6 @@ fn collect_runtime_health_probes( }; let (current, ownership_error) = recorded_runtime_is_current( &supervisor, - &paths.resource_pid(&track.resource_name, &track.track), &paths.resource_runtime_metadata(&track.resource_name, &track.track), track.current_artifact_path.as_deref(), false, @@ -535,7 +532,6 @@ fn collect_project_tls_health( fn recorded_runtime_is_current( supervisor: &ProcessSupervisor, - pid_path: &Utf8Path, metadata_path: &Utf8Path, artifact_root: Option<&Utf8Path>, requires_applied_config: bool, @@ -543,7 +539,7 @@ fn recorded_runtime_is_current( let Some(artifact_root) = artifact_root else { return (false, None); }; - match supervisor.adopt_recorded(pid_path, metadata_path) { + match supervisor.adopt_recorded(metadata_path) { Ok(Some(runtime)) => ( runtime.uses_current_artifact(artifact_root) && (!requires_applied_config || runtime.has_applied_desired_config()), @@ -1612,7 +1608,6 @@ mod tests { config_path: paths.worker_root_config(php_track), config_fingerprint: None, log_path: paths.worker_log(php_track), - pid_path: paths.worker_pid(php_track), metadata_path: paths.worker_runtime_metadata(php_track), resource_name: "frankenphp".to_owned(), track: php_track.to_owned(), diff --git a/crates/daemon/src/jobs.rs b/crates/daemon/src/jobs.rs index c34a2fe9b..9a41bea4e 100644 --- a/crates/daemon/src/jobs.rs +++ b/crates/daemon/src/jobs.rs @@ -29,7 +29,6 @@ use crate::reconciliation::{ EnqueueResult, QueuedReconciliation, ReconciliationJobTiming, ReconciliationQueue, ReconciliationScope, RunningReconciliation, }; -use crate::runtime_stop; use crate::structured_log::{self, PhaseOutcome, ReconciliationPhase, ReconciliationPhaseLog}; use protocol::{DaemonEvent, DaemonResponse, DaemonTransport, write_line}; use state::{ @@ -496,16 +495,6 @@ pub(crate) async fn run_startup_reconciliation_job( .await } -/// Stops the runtimes a release from before monitors left running, so they restart under -/// monitors. Every job does this before it reconciles, since the first job after an update need -/// not be the startup job. Once none remain, it only lists the run directory. Reconciliation -/// reports any runtime it couldn't stop, since it can't start a replacement over one. -async fn stop_legacy_runtimes_before_reconciliation(paths: &PvPaths) { - if let Err(error) = runtime_stop::stop_legacy_runtimes(paths).await { - structured_log::legacy_runtime_stop_failed(paths, &error.to_string()); - } -} - async fn wait_for_startup_reconciliation_turn( queued: QueuedReconciliation, shutdown: &mut oneshot::Receiver<()>, @@ -1667,7 +1656,6 @@ async fn complete_update_job_inner( if fallback_shutdown_requested(fallback_shutdown) { return Ok(ReconciliationOutcome::Cancelled); } - stop_legacy_runtimes_before_reconciliation(paths).await; let report = if runtime_catalog.is_none() { let update_paths = paths.clone(); let update_progress = progress.clone(); @@ -2359,7 +2347,6 @@ async fn complete_reconciliation_job_with_progress_outcome( timing.queue_wait(), &[], ); - stop_legacy_runtimes_before_reconciliation(paths).await; let obsolete_project = match (discard_obsolete_project, scope) { (true, ReconciliationScope::Project { id }) => { project_exists(paths, id.as_str()).map(|exists| !exists) @@ -3794,7 +3781,6 @@ mod tests { use std::io::{self, Write}; use std::net::{TcpListener, TcpStream}; use std::os::unix::fs::PermissionsExt; - use std::os::unix::process::CommandExt as _; use std::pin::Pin; use std::process; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -3917,10 +3903,7 @@ mod tests { .await?; let supervisor = ProcessSupervisor::new(paths.clone()); let worker = supervisor - .adopt_recorded( - &paths.worker_pid("8.4"), - &paths.worker_runtime_metadata("8.4"), - )? + .adopt_recorded(&paths.worker_runtime_metadata("8.4"))? .ok_or_else(|| anyhow::anyhow!("worker not running after setup"))?; worker.stop(Duration::from_secs(1)).await?; state::fs::write_sensitive_file(&config_path, "php: [\n")?; @@ -3932,10 +3915,7 @@ mod tests { None, ) .await; - let recovered = supervisor.adopt_recorded( - &paths.worker_pid("8.4"), - &paths.worker_runtime_metadata("8.4"), - )?; + let recovered = supervisor.adopt_recorded(&paths.worker_runtime_metadata("8.4"))?; let worker_recovered = recovered.is_some(); drop(recovered); let database = Database::open(&paths)?; @@ -4432,83 +4412,6 @@ mod tests { Ok(()) } - #[tokio::test] - async fn any_first_job_restarts_a_gateway_from_before_monitors_under_a_monitor() - -> anyhow::Result<()> { - let tempdir = tempdir()?; - let paths = PvPaths::for_home(tempdir.path().join("home")); - seed_installed_caddy(&paths)?; - let mut caddy_guard = SeededRuntimeGuard::new(paths.clone())?; - // A Gateway that a release from before monitors started, known by its pid file. - let legacy_caddy = tempdir.path().join("legacy-caddy-release/bin/caddy"); - pv_fake::install(&legacy_caddy, Persona::LongRunning)?; - let mut legacy = LegacyRuntime::spawn(&legacy_caddy)?; - let legacy_pid = legacy.0.id(); - state::fs::write_sensitive_file(&paths.gateway_pid(), &format!("{legacy_pid}\n"))?; - let legacy_record = json!({ - "name": "gateway", - "pid": legacy_pid, - "command": legacy_caddy, - "arguments": [], - "resource_name": "caddy", - "track": CADDY_TEST_TRACK, - "log_path": paths.gateway_supervisor_log(), - "started_at": "2026-10-09T00:00:00Z", - "boot_session_id": platform::current_boot_session_id()?, - "process_start_identity": platform::inspect_process_start_identity(legacy_pid)?, - }); - state::fs::write_sensitive_file( - &paths.gateway_runtime_metadata(), - &serde_json::to_string(&legacy_record)?, - )?; - - // An ordinary job, as `pv update` can queue ahead of the startup job. - let job = start_reconciliation_job(&paths, "system")?; - let result = super::complete_reconciliation_job( - &paths, - &job, - &ReconciliationScope::System, - None, - ReconciliationJobTiming::immediate(), - Some(GatewayPfRoutingState::Inactive), - ) - .await; - let legacy_status = legacy.0.try_wait(); - let record: serde_json::Value = serde_json::from_str(&state::fs::read_to_string( - &paths.gateway_runtime_metadata(), - )?)?; - caddy_guard.cleanup().await?; - - result?; - assert!(legacy_status?.is_some()); - assert!(!state::fs::path_entry_exists(&paths.gateway_pid())?); - assert_ne!(record["pid"], json!(legacy_pid)); - assert!(record["monitor_instance"].is_string()); - Ok(()) - } - - /// A runtime that a release from before monitors started outside the supervisor. - struct LegacyRuntime(process::Child); - - impl LegacyRuntime { - #[expect( - clippy::disallowed_types, - reason = "a runtime from before monitors was started outside the supervisor" - )] - fn spawn(executable: &Utf8Path) -> anyhow::Result { - Ok(Self( - process::Command::new(executable).process_group(0).spawn()?, - )) - } - } - - impl Drop for LegacyRuntime { - fn drop(&mut self) { - let _kill_result = self.0.kill(); - let _wait_result = self.0.wait(); - } - } - #[tokio::test] async fn targeted_resource_scope_records_exact_partial_and_success_coverage() -> anyhow::Result<()> { @@ -4983,7 +4886,7 @@ mod tests { observations[2] == unrelated_before, resource_cleanup_result.is_ok(), !state::fs::path_entry_exists( - &paths.resource_pid("mailpit", MAILPIT_TEST_TRACK), + &paths.resource_runtime_metadata("mailpit", MAILPIT_TEST_TRACK), )?, ], ); diff --git a/crates/daemon/src/managed_resources/fake.rs b/crates/daemon/src/managed_resources/fake.rs index 7bd065d62..40cee6ca5 100644 --- a/crates/daemon/src/managed_resources/fake.rs +++ b/crates/daemon/src/managed_resources/fake.rs @@ -98,7 +98,6 @@ impl ManagedResourceRuntimeAdapter for FakeMailpitRuntimeAdapter { config_path, config_fingerprint: None, log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), diff --git a/crates/daemon/src/managed_resources/mailpit.rs b/crates/daemon/src/managed_resources/mailpit.rs index f4dc27c93..e5b10c9d8 100644 --- a/crates/daemon/src/managed_resources/mailpit.rs +++ b/crates/daemon/src/managed_resources/mailpit.rs @@ -81,7 +81,6 @@ impl ManagedResourceRuntimeAdapter for MailpitRuntimeAdapter { config_path, config_fingerprint: None, log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), diff --git a/crates/daemon/src/managed_resources/mod.rs b/crates/daemon/src/managed_resources/mod.rs index 65003177d..53745cf64 100644 --- a/crates/daemon/src/managed_resources/mod.rs +++ b/crates/daemon/src/managed_resources/mod.rs @@ -2458,7 +2458,6 @@ async fn start_or_adopt_runtime( .stop_subject(&spec.metadata_path, stop_signal, RESOURCE_STOP_GRACE_PERIOD) .await? { - delete_optional_file(&spec.pid_path)?; delete_optional_file(&spec.metadata_path)?; } else if let ManagedResourceReadiness::TcpHttp(check) = &readiness && crate::supervisor::probe_readiness_once(check).await.is_ok() @@ -2579,7 +2578,6 @@ fn remaining_timeout(started_at: Instant, readiness_timeout: Duration) -> Option } fn cleanup_started_runtime_files(spec: &ProcessSpec) -> Result<(), DaemonError> { - delete_optional_file(&spec.pid_path)?; delete_optional_file(&spec.metadata_path)?; delete_optional_file(&spec.config_path)?; @@ -2704,7 +2702,6 @@ fn cleanup_resource_runtime_files_for_track( resource_name: &str, track: &str, ) -> Result<(), DaemonError> { - delete_optional_file(&paths.resource_pid(resource_name, track))?; delete_optional_file(&paths.resource_runtime_metadata(resource_name, track))?; delete_optional_file(&paths.resource_runtime_config(resource_name, track))?; diff --git a/crates/daemon/src/managed_resources/mysql.rs b/crates/daemon/src/managed_resources/mysql.rs index 3da4b3472..653704da6 100644 --- a/crates/daemon/src/managed_resources/mysql.rs +++ b/crates/daemon/src/managed_resources/mysql.rs @@ -175,7 +175,6 @@ impl ManagedResourceRuntimeAdapter for MysqlRuntimeAdapter { config_path, config_fingerprint: None, log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), diff --git a/crates/daemon/src/managed_resources/postgres.rs b/crates/daemon/src/managed_resources/postgres.rs index 617043b17..4c4b8a47d 100644 --- a/crates/daemon/src/managed_resources/postgres.rs +++ b/crates/daemon/src/managed_resources/postgres.rs @@ -99,7 +99,6 @@ impl ManagedResourceRuntimeAdapter for PostgresRuntimeAdapter { config_path: paths.resource_runtime_config(&context.resource_name, &context.track), config_fingerprint: Some(config_fingerprint(&config)), log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), diff --git a/crates/daemon/src/managed_resources/redis.rs b/crates/daemon/src/managed_resources/redis.rs index 4a780da6f..d3eb71ccf 100644 --- a/crates/daemon/src/managed_resources/redis.rs +++ b/crates/daemon/src/managed_resources/redis.rs @@ -72,7 +72,6 @@ set-proc-title no config_path, config_fingerprint: None, log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), diff --git a/crates/daemon/src/managed_resources/runtime_contracts.rs b/crates/daemon/src/managed_resources/runtime_contracts.rs index e5fc22623..d2ef6efcf 100644 --- a/crates/daemon/src/managed_resources/runtime_contracts.rs +++ b/crates/daemon/src/managed_resources/runtime_contracts.rs @@ -211,7 +211,7 @@ async fn real_postgres_forced_exit_keeps_recovery_records() -> Result<()> { let backend_pid = u32::try_from(backend_pid)?; let backend_identity = platform::inspect_process_start_identity(backend_pid)? .ok_or_else(|| anyhow!("connected backend had no process identity"))?; - let postmaster = supervisor.adopt_recorded(&spec.pid_path, &spec.metadata_path)? + let postmaster = supervisor.adopt_recorded( &spec.metadata_path)? .ok_or_else(|| anyhow!("Postgres ownership was not verified"))?; postmaster.kill_and_wait_for_test(Duration::from_secs(2))?; let stopped = crate::stop_recorded_runtimes(paths.clone()).await; @@ -531,11 +531,7 @@ async fn start_wait_and_stop( check: impl Future>, ) -> Result<()> { let spec = adapter.build_process_spec(paths, context)?; - let (log_path, pid_path, metadata_path) = ( - spec.log_path.clone(), - spec.pid_path.clone(), - spec.metadata_path.clone(), - ); + let (log_path, metadata_path) = (spec.log_path.clone(), spec.metadata_path.clone()); adapter.prepare_runtime(paths, context).await?; let supervisor = ProcessSupervisor::new(paths.clone()); let Some(pending) = start_or_adopt_runtime( @@ -562,10 +558,6 @@ async fn start_wait_and_stop( // Released only now, so whatever the check held stayed open through the stop. checked.with_context(|| runtime_log(&log_path))?; stopped?; - ensure!( - !state::fs::path_entry_exists(&pid_path)?, - "runtime PID record remains after stop" - ); ensure!( !state::fs::path_entry_exists(&metadata_path)?, "runtime metadata remains after stop" diff --git a/crates/daemon/src/managed_resources/rustfs.rs b/crates/daemon/src/managed_resources/rustfs.rs index b8da5f94e..39d1abce0 100644 --- a/crates/daemon/src/managed_resources/rustfs.rs +++ b/crates/daemon/src/managed_resources/rustfs.rs @@ -87,7 +87,6 @@ impl ManagedResourceRuntimeAdapter for RustfsRuntimeAdapter { config_path, config_fingerprint: None, log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), diff --git a/crates/daemon/src/managed_resources/tests.rs b/crates/daemon/src/managed_resources/tests.rs index 4707c172b..e8286b146 100644 --- a/crates/daemon/src/managed_resources/tests.rs +++ b/crates/daemon/src/managed_resources/tests.rs @@ -2649,10 +2649,9 @@ async fn assert_fallback_during_project_resource_readiness( let runtime_files_after_reconciliation = runtime_files_exist_for_resource(&paths, "mysql", FAKE_SQL_TRACK)?; let supervisor = ProcessSupervisor::new(paths.clone()); - if let Some(adopted) = supervisor.adopt_recorded( - &paths.resource_pid("mysql", FAKE_SQL_TRACK), - &paths.resource_runtime_metadata("mysql", FAKE_SQL_TRACK), - )? { + if let Some(adopted) = + supervisor.adopt_recorded(&paths.resource_runtime_metadata("mysql", FAKE_SQL_TRACK))? + { adopted.stop(Duration::from_secs(1)).await?; } super::cleanup_resource_runtime_files( @@ -5265,10 +5264,7 @@ async fn resource_readiness_wave_recovers_after_cancellation_and_stays_db_free() ); let pid = resource_runtime_metadata_pid(&paths, "mysql", track)?; let adopted = supervisor - .adopt_recorded( - &paths.resource_pid("mysql", track), - &paths.resource_runtime_metadata("mysql", track), - )? + .adopt_recorded(&paths.resource_runtime_metadata("mysql", track))? .ok_or_else(|| anyhow!("cancelled mysql {track} runtime was not live"))?; assert_eq!(u64::from(adopted.pid()), pid); cancelled_pids.insert(track, pid); @@ -6354,10 +6350,9 @@ async fn delete_rustfs_bucket(paths: &PvPaths, bucket: &str) -> Result<()> { } async fn stop_recorded_rustfs_runtime(paths: &PvPaths) -> Result<()> { - if let Some(process) = ProcessSupervisor::new(paths.clone()).adopt_recorded( - &paths.resource_pid("rustfs", RUSTFS_TRACK), - &paths.resource_runtime_metadata("rustfs", RUSTFS_TRACK), - )? { + if let Some(process) = ProcessSupervisor::new(paths.clone()) + .adopt_recorded(&paths.resource_runtime_metadata("rustfs", RUSTFS_TRACK))? + { process.stop(Duration::from_secs(1)).await?; } @@ -6365,10 +6360,9 @@ async fn stop_recorded_rustfs_runtime(paths: &PvPaths) -> Result<()> { } async fn stop_recorded_redis_runtime(paths: &PvPaths) -> Result<()> { - if let Some(process) = ProcessSupervisor::new(paths.clone()).adopt_recorded( - &paths.resource_pid("redis", REDIS_TRACK), - &paths.resource_runtime_metadata("redis", REDIS_TRACK), - )? { + if let Some(process) = ProcessSupervisor::new(paths.clone()) + .adopt_recorded(&paths.resource_runtime_metadata("redis", REDIS_TRACK))? + { process.stop(Duration::from_secs(1)).await?; } @@ -7682,7 +7676,6 @@ impl super::ManagedResourceRuntimeAdapter for AsyncSqlHookRuntimeAdapter { config_path, config_fingerprint: None, log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), @@ -7895,7 +7888,6 @@ impl super::ManagedResourceRuntimeAdapter for GatedSqlRuntimeAdapter { config_path, config_fingerprint: None, log_path: paths.resource_log(&context.resource_name, &context.track), - pid_path: paths.resource_pid(&context.resource_name, &context.track), metadata_path: paths.resource_runtime_metadata(&context.resource_name, &context.track), resource_name: context.resource_name.clone(), track: context.track.clone(), diff --git a/crates/daemon/src/runtime_stop.rs b/crates/daemon/src/runtime_stop.rs index e123cd3b7..909f8ac3d 100644 --- a/crates/daemon/src/runtime_stop.rs +++ b/crates/daemon/src/runtime_stop.rs @@ -170,41 +170,11 @@ pub async fn stop_recorded_runtimes(paths: PvPaths) -> Result<(), DaemonError> { failures.push(RuntimeReconciliationFailure::new(subject, error)); } } - for pid_path in records { - let result = if fs::path_entry_exists(&pid_path)? { - supervisor.stop_legacy_runtime(&pid_path, false).await - } else { - // Only proofs remain once a monitored runtime is gone; one still running keeps them. - remove_record_without_monitor(&paths, &pid_path.with_extension("json")) - }; - if let Err(error) = result { + // Only proofs remain once a monitored runtime is gone; one still running keeps them. + for metadata_path in records { + if let Err(error) = remove_record_without_monitor(&paths, &metadata_path) { failures.push(RuntimeReconciliationFailure::new( - pid_path.to_string(), - error, - )); - } - } - if failures.is_empty() { - Ok(()) - } else { - Err(DaemonError::RuntimeStopFailures { failures }) - } -} - -/// Stops the runtimes a PV release from before monitors left running, keeping their records' -/// config proofs, so they restart under monitors. Once none remain, it does nothing. -pub(crate) async fn stop_legacy_runtimes(paths: &PvPaths) -> Result<(), DaemonError> { - if !fs::path_entry_exists(paths.run())? { - return Ok(()); - } - let supervisor = ProcessSupervisor::new(paths.clone()); - let mut failures = Vec::new(); - for pid_path in runtime_records(paths)? { - if fs::path_entry_exists(&pid_path)? - && let Err(error) = supervisor.stop_legacy_runtime(&pid_path, true).await - { - failures.push(RuntimeReconciliationFailure::new( - pid_path.to_string(), + metadata_path.to_string(), error, )); } @@ -228,14 +198,12 @@ fn remove_record_without_monitor( Ok(()) } -/// The pid-file path of every runtime record, whether only its pid file, only its JSON record, -/// or both exist. +/// The path of every runtime's JSON record. fn runtime_records(paths: &PvPaths) -> Result, DaemonError> { let mut records = BTreeSet::new(); - for path in [paths.gateway_pid(), paths.gateway_runtime_metadata()] { - if fs::path_entry_exists(&path)? { - records.insert(path.with_extension("pid")); - } + let gateway = paths.gateway_runtime_metadata(); + if fs::path_entry_exists(&gateway)? { + records.insert(gateway); } collect_records(&paths.run().join("workers"), &mut records)?; let resources = paths.run().join("resources"); @@ -248,7 +216,7 @@ fn runtime_records(paths: &PvPaths) -> Result, DaemonError continue; } // MySQL keeps its initialization SQL beside the per-resource directories. - if fs::path_is_file(&resource)? && !matches!(resource.extension(), Some("pid" | "json")) { + if fs::path_is_file(&resource)? && resource.extension() != Some("json") { continue; } require_directory(&resource)?; @@ -267,11 +235,11 @@ fn collect_records( } require_directory(directory)?; for path in fs::read_dir_paths(directory)? { - if matches!(path.extension(), Some("pid" | "json")) { + if path.extension() == Some("json") { if !fs::path_is_file(&path)? { return Err(DaemonError::InvalidRuntimeRecord { path }); } - records.insert(path.with_extension("pid")); + records.insert(path); } } Ok(()) diff --git a/crates/daemon/src/structured_log.rs b/crates/daemon/src/structured_log.rs index 649c0ff6a..7125ece15 100644 --- a/crates/daemon/src/structured_log.rs +++ b/crates/daemon/src/structured_log.rs @@ -379,17 +379,6 @@ pub(crate) fn runtime_readiness_diagnostics( ); } -pub(crate) fn legacy_runtime_stop_failed(paths: &PvPaths, error: &str) { - append_best_effort( - paths, - "error", - "runtime", - "legacy_runtime_stop_failed", - "runtime from before monitors could not be stopped", - &[("error", error)], - ); -} - /// Records how a monitored runtime exited before its monitor is released and forgets it. pub(crate) fn runtime_exited(paths: &PvPaths, subject: &str, exit: &str) { append_best_effort( diff --git a/crates/daemon/src/supervisor.rs b/crates/daemon/src/supervisor.rs index 3562a0358..91e7607b5 100644 --- a/crates/daemon/src/supervisor.rs +++ b/crates/daemon/src/supervisor.rs @@ -7,10 +7,7 @@ use camino::{Utf8Path, Utf8PathBuf}; use futures_util::{Stream, StreamExt, stream}; use platform::PlatformCapability; #[cfg(target_os = "macos")] -use rustix::process::{ - Pid, Signal, WaitOptions, kill_process_group, test_kill_process, test_kill_process_group, - waitpid, -}; +use rustix::process::{Pid, Signal, WaitOptions, kill_process_group, waitpid}; use rustls::pki_types::ServerName; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; @@ -60,7 +57,6 @@ pub struct ProcessSpec { pub config_path: Utf8PathBuf, pub config_fingerprint: Option, pub log_path: Utf8PathBuf, - pub pid_path: Utf8PathBuf, pub metadata_path: Utf8PathBuf, pub resource_name: String, pub track: String, @@ -83,7 +79,6 @@ impl fmt::Debug for ProcessSpec { debug.field("config_fingerprint", config_fingerprint); } debug.field("log_path", &self.log_path); - debug.field("pid_path", &self.pid_path); debug.field("metadata_path", &self.metadata_path); debug.field("resource_name", &self.resource_name); debug.field("track", &self.track); @@ -120,7 +115,6 @@ pub struct ManagedProcess { subject: String, instance: String, log_path: Utf8PathBuf, - pid_path: Utf8PathBuf, metadata_path: Utf8PathBuf, } @@ -186,8 +180,7 @@ pub enum ReadinessCheck { }, } -/// A runtime's record beside its monitor: its spec identity and config proofs. A record PV wrote -/// before monitors names its process by pid file and identity instead of a monitor instance. +/// A runtime's record beside its monitor: its spec identity and config proofs. #[derive(Deserialize, Eq, PartialEq, Serialize)] struct RuntimeMetadata { name: String, @@ -217,18 +210,6 @@ struct RuntimeMetadata { staged_config_fingerprint: Option, log_path: String, started_at: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - boot_session_id: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - process_start_identity: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - process_executable_identity: Option, -} - -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] -struct ProcessExecutableIdentity { - executable: String, - argument_zero: String, } impl ProcessSupervisor { @@ -236,124 +217,6 @@ impl ProcessSupervisor { Self { paths } } - /// Stops a runtime that a PV release from before monitors started, proven by its pid file and - /// live identity. Then it removes the pid file and, unless `keep_metadata`, the record, whose - /// config proofs a restarted runtime may still need. - pub(crate) async fn stop_legacy_runtime( - &self, - pid_path: &Utf8Path, - keep_metadata: bool, - ) -> Result<(), DaemonError> { - let metadata_path = pid_path.with_extension("json"); - for path in [pid_path, metadata_path.as_path()] { - if !fs::path_is_file(path)? { - return Err(DaemonError::InvalidRuntimeRecord { - path: path.to_owned(), - }); - } - } - let metadata = read_runtime_metadata(&metadata_path)?.ok_or_else(|| { - DaemonError::InvalidRuntimeRecord { - path: metadata_path.clone(), - } - })?; - let pid = read_pid_file(pid_path)?.ok_or_else(|| DaemonError::InvalidRuntimeRecord { - path: pid_path.to_owned(), - })?; - let subject_matches = if pid_path == self.paths.gateway_pid() { - metadata.resource_name == "caddy" - } else if pid_path.parent() == Some(self.paths.run().join("workers").as_path()) { - metadata.resource_name == "frankenphp" - && pid_path == self.paths.worker_pid(&metadata.track) - } else { - !metadata.resource_name.is_empty() - && !metadata.track.is_empty() - && pid_path - == self - .paths - .resource_pid(&metadata.resource_name, &metadata.track) - }; - if metadata.pid != pid || metadata.process_start_identity.is_none() || !subject_matches { - return Err(DaemonError::InvalidRuntimeRecord { - path: metadata_path, - }); - } - if let Some(recorded_boot) = &metadata.boot_session_id - && recorded_boot != &platform::current_boot_session_id()? - { - // A previous kernel's processes cannot survive this boot. Do not inspect - // or signal the recorded PID: it may now belong to an unrelated process. - return remove_unchanged_runtime_records( - pid_path, - &metadata_path, - &metadata, - keep_metadata, - ); - } - if legacy_process_matches(pid, &metadata)? { - // Postgres backends can leave the postmaster's process group, so an empty - // group proves nothing about them. The postmaster exits successfully only - // after its backends exit. Watch before signalling, then recheck ownership. - let exit_watch = if metadata.resource_name == "postgres" { - let watch = platform::ProcessExitWatch::new(pid)?; - if !legacy_process_matches(pid, &metadata)? { - return Err(DaemonError::RuntimeProcessIdentityChanged { pid }); - } - Some(watch) - } else { - None - }; - let stop = stop_policy(&metadata.resource_name); - // An adopted leader can be reaped during the grace period, by launchd once the - // daemon that started it has exited. Do not signal a group whose recorded - // leader no longer matches. - stop_process_group(pid, stop.signal, stop_grace(stop), || { - legacy_process_matches(pid, &metadata) - }) - .await?; - if let Some(mut watch) = exit_watch { - let status = timeout(Duration::from_secs(1), async { - loop { - if let Some(status) = watch.try_exit_status()? { - return Ok::<_, DaemonError>(status); - } - sleep(READINESS_POLL_INTERVAL).await; - } - }) - .await; - let status = match status { - Ok(status) => status?, - Err(_elapsed) => { - return Err(DaemonError::RuntimeCleanupUnproven { - pid, - reason: format!( - "PV could not confirm that Postgres shut down cleanly, so it kept its records. {POSTGRES_RECOVERY}" - ), - }); - } - }; - if !status.success() { - return Err(DaemonError::RuntimeCleanupUnproven { - pid, - reason: format!( - "Postgres ended with {status}, so PV kept its records. {POSTGRES_RECOVERY}" - ), - }); - } - } - } else if !process_and_group_are_absent(pid)? { - return Err(DaemonError::RuntimeProcessIdentityChanged { pid }); - } else if metadata.resource_name == "postgres" { - return Err(DaemonError::RuntimeCleanupUnproven { - pid, - reason: format!( - "Postgres exited before PV could stop it, so PV kept its records. {POSTGRES_RECOVERY}" - ), - }); - } - remove_unchanged_runtime_records(pid_path, &metadata_path, &metadata, keep_metadata) - } - /// Starts `spec`'s runtime under a new monitor, run by the installed `pv`, and records it. pub async fn start(&self, spec: ProcessSpec) -> Result { require_process_containment()?; @@ -392,7 +255,6 @@ impl ProcessSupervisor { subject, instance, log_path: spec.log_path.clone(), - pid_path: spec.pid_path.clone(), metadata_path: spec.metadata_path.clone(), }; if let Err(error) = write_runtime_metadata(&spec, process.pid, &process.instance) { @@ -510,17 +372,15 @@ impl ProcessSupervisor { let Some(metadata) = read_runtime_metadata(&spec.metadata_path)? else { return Ok(None); }; - // A record that names neither a monitor nor a process identity is incomplete. - if metadata.matches(spec) - && (metadata.monitor_instance.is_some() || metadata.process_start_identity.is_some()) - { + // A record that names no monitor is incomplete. + if metadata.matches(spec) && metadata.monitor_instance.is_some() { return Ok(metadata.recorded_config_fingerprint()); } // The installed artifact changed under a live runtime: prove the prior recording // against its recorded spec so old applied fingerprints stay readable. Live // ownership decisions still use verify_ownership against the current spec. - let Some(adopted) = self.adopt_recorded(&spec.pid_path, &spec.metadata_path)? else { + let Some(adopted) = self.adopt_recorded(&spec.metadata_path)? else { return Ok(None); }; @@ -616,10 +476,9 @@ impl ProcessSupervisor { } /// The runtime recorded at `metadata_path`, if its monitor still runs it, whatever its - /// current spec. `_pid_path` names a record PV wrote before monitors. + /// current spec. pub fn adopt_recorded( &self, - _pid_path: &Utf8Path, metadata_path: &Utf8Path, ) -> Result, DaemonError> { require_process_containment()?; @@ -683,10 +542,6 @@ impl ManagedProcess { &self.log_path } - pub fn pid_path(&self) -> &Utf8Path { - &self.pid_path - } - pub fn metadata_path(&self) -> &Utf8Path { &self.metadata_path } @@ -1367,21 +1222,6 @@ pub(crate) fn exit_description(exit: Option) -> String { } } -/// Whether the process at `pid` is still the one a record from before monitors names. -fn legacy_process_matches(pid: u32, metadata: &RuntimeMetadata) -> Result { - let Some(start_identity) = metadata.process_start_identity else { - return Ok(false); - }; - - live_process_matches( - pid, - Utf8Path::new(&metadata.command), - &metadata.arguments, - start_identity, - metadata.process_executable_identity.as_ref(), - ) -} - fn write_runtime_metadata( spec: &ProcessSpec, pid: u32, @@ -1405,9 +1245,6 @@ fn write_runtime_metadata( staged_config_fingerprint: None, log_path: spec.log_path.to_string(), started_at, - boot_session_id: None, - process_start_identity: None, - process_executable_identity: None, }; let encoded = serde_json::to_string(&metadata)?; @@ -1416,38 +1253,6 @@ fn write_runtime_metadata( Ok(()) } -fn remove_unchanged_runtime_records( - pid_path: &Utf8Path, - metadata_path: &Utf8Path, - metadata: &RuntimeMetadata, - keep_metadata: bool, -) -> Result<(), DaemonError> { - // Admission is closed and the daemon has stopped. Still compare both records - // before removal so another instance's evidence cannot be removed by this stop. - if read_pid_file(pid_path)? != Some(metadata.pid) - || read_runtime_metadata(metadata_path)?.is_none_or(|current| current != *metadata) - { - return Err(DaemonError::RuntimeProcessIdentityChanged { pid: metadata.pid }); - } - fs::remove_file_if_exists(pid_path)?; - if !keep_metadata { - fs::remove_file_if_exists(metadata_path)?; - } - Ok(()) -} - -fn read_pid_file(path: &Utf8Path) -> Result, DaemonError> { - let Some(content) = read_optional_file(path)? else { - return Ok(None); - }; - let pid = content - .trim() - .parse::() - .map_err(|source| io::Error::new(io::ErrorKind::InvalidData, source))?; - - Ok(Some(pid)) -} - fn read_runtime_metadata(path: &Utf8Path) -> Result, DaemonError> { let Some(content) = read_optional_file(path)? else { return Ok(None); @@ -1471,23 +1276,6 @@ fn process_group_has_exited(pid: u32) -> Result { Ok(!platform::process_group_has_live_members(pid)?) } -#[cfg(target_os = "macos")] -fn process_and_group_are_absent(pid: u32) -> Result { - let process = process_group_pid(pid)?; - let process_absent = match test_kill_process(process) { - Err(rustix::io::Errno::SRCH) => true, - Ok(()) | Err(rustix::io::Errno::PERM) => false, - Err(source) => return Err(io::Error::from(source).into()), - }; - let group_absent = match test_kill_process_group(process) { - Err(rustix::io::Errno::SRCH) => true, - Ok(()) | Err(rustix::io::Errno::PERM) => false, - Err(source) => return Err(io::Error::from(source).into()), - }; - - Ok(process_absent && group_absent) -} - #[cfg(target_os = "macos")] pub(crate) fn reap_process_if_child(pid: u32) -> Result<(), DaemonError> { let process = process_group_pid(pid)?; @@ -1504,74 +1292,6 @@ fn process_group_has_exited(_pid: u32) -> Result { Ok(true) } -#[cfg(any(target_os = "linux", target_os = "windows"))] -fn process_and_group_are_absent(_pid: u32) -> Result { - require_process_containment()?; - - Ok(false) -} - -fn live_process_matches( - pid: u32, - command: &Utf8Path, - arguments: &[String], - process_start_identity: platform::ProcessStartIdentity, - process_executable_identity: Option<&ProcessExecutableIdentity>, -) -> Result { - let Some(process_identity) = platform::inspect_process_identity(pid)? else { - return Ok(false); - }; - if process_identity.start_identity != process_start_identity { - return Ok(false); - } - - Ok(process_identity_matches( - &process_identity, - command, - arguments, - process_executable_identity, - )) -} - -fn process_identity_matches( - process_identity: &platform::ProcessIdentity, - command: &Utf8Path, - arguments: &[String], - process_executable_identity: Option<&ProcessExecutableIdentity>, -) -> bool { - let direct_arguments_match = process_identity.arguments == arguments; - let direct_command_matches = executable_matches(process_identity, command); - let script_arguments_match = script_arguments_match(process_identity, command, arguments); - let script_identity_matches = script_arguments_match - && fs::read_to_string(command).is_ok_and(|source| source.starts_with("#!")) - && process_executable_identity.is_some_and(|expected| { - expected.executable == process_identity.executable.as_str() - && expected.argument_zero == process_identity.argument_zero - }); - - (direct_command_matches && direct_arguments_match) || script_identity_matches -} - -fn script_arguments_match( - process_identity: &platform::ProcessIdentity, - command: &Utf8Path, - arguments: &[String], -) -> bool { - process_identity - .arguments - .split_first() - .is_some_and(|(script, live_arguments)| { - script == command.as_str() && live_arguments == arguments - }) -} - -fn executable_matches(process_identity: &platform::ProcessIdentity, command: &Utf8Path) -> bool { - process_identity.executable == command - || (command == Utf8Path::new("/bin/sh") - && process_identity.executable == Utf8Path::new("/bin/bash") - && process_identity.argument_zero == command.as_str()) -} - #[cfg(target_os = "macos")] fn process_not_found(error: &io::Error) -> bool { error.kind() == io::ErrorKind::NotFound || error.raw_os_error() == Some(3) @@ -1729,7 +1449,7 @@ mod tests { use super::{ ProcessSpec, ProcessSupervisor, RecordedConfigFingerprint, monitor_subject, - process_group_has_exited, read_runtime_metadata, remove_unchanged_runtime_records, + process_group_has_exited, }; use crate::StopSignal; use crate::monitor::{self, MonitorStart, MonitorStop}; @@ -1740,40 +1460,6 @@ mod tests { /// a test, so a stalled test can't pass because its fixture exited on its own. const IDLE_SHELL_LOOP: &str = "i=0; while [ $i -lt 150 ]; do sleep 1; i=$((i + 1)); done"; - #[test] - fn shutdown_cleanup_preserves_runtime_records_changed_after_inspection() -> Result<()> { - for change_pid in [false, true] { - let tempdir = tempdir()?; - let pid_path = tempdir.path().join("runtime.pid"); - let metadata_path = tempdir.path().join("runtime.json"); - let mut metadata = serde_json::json!({ - "name": "postgres", "pid": 42, "command": "/owned/postgres", - "arguments": [], "log_path": "/owned/postgres.log", "started_at": "unused", - }); - state::fs::write_sensitive_file(&pid_path, "42\n")?; - state::fs::write_sensitive_file(&metadata_path, &serde_json::to_string(&metadata)?)?; - let inspected = read_runtime_metadata(&metadata_path)? - .ok_or_else(|| anyhow!("runtime metadata was missing"))?; - let current_pid = if change_pid { "43\n" } else { "42\n" }; - metadata["name"] = serde_json::json!("replacement"); - state::fs::write_sensitive_file(&pid_path, current_pid)?; - if !change_pid { - state::fs::write_sensitive_file( - &metadata_path, - &serde_json::to_string(&metadata)?, - )?; - } - let before = state::fs::read_to_string(&metadata_path)?; - assert!(matches!( - remove_unchanged_runtime_records(&pid_path, &metadata_path, &inspected, false), - Err(crate::DaemonError::RuntimeProcessIdentityChanged { pid: 42 }), - )); - assert_eq!(state::fs::read_to_string(&pid_path)?, current_pid); - assert_eq!(state::fs::read_to_string(&metadata_path)?, before); - } - Ok(()) - } - #[tokio::test] async fn start_replaces_a_monitor_left_without_a_record() -> Result<()> { let tempdir = tempdir()?; @@ -1881,7 +1567,6 @@ mod tests { &descendant_pid_path, ), ); - let pid_path = spec.pid_path.clone(); let metadata_path = spec.metadata_path.clone(); let supervisor = ProcessSupervisor::new(paths); let runtime = tokio::runtime::Builder::new_current_thread() @@ -1903,7 +1588,7 @@ mod tests { let cleanup_result: Result<()> = (|| { let process = supervisor - .adopt_recorded(&pid_path, &metadata_path)? + .adopt_recorded(&metadata_path)? .ok_or_else(|| anyhow!("recorded process was not adoptable"))?; process .kill_and_wait_for_test(Duration::from_secs(1)) @@ -1938,7 +1623,6 @@ mod tests { config_path: paths.config().join(format!("{name}.json")), config_fingerprint: None, log_path: paths.logs().join(format!("{name}.log")), - pid_path: paths.run().join(format!("{name}.pid")), metadata_path: paths.run().join(format!("{name}-metadata.json")), resource_name: name.to_string(), track: "test".to_string(), diff --git a/crates/daemon/test-fixtures/supervisor/owned-python-runtime.py b/crates/daemon/test-fixtures/supervisor/owned-python-runtime.py deleted file mode 100644 index 4161394aa..000000000 --- a/crates/daemon/test-fixtures/supervisor/owned-python-runtime.py +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env python3 -import os -import signal -import sys -import threading -import time - - -parent_pid = os.getppid() -if parent_pid == 1: - os._exit(0) - - -def monitor_parent(): - while True: - time.sleep(0.1) - if os.getppid() != parent_pid: - os._exit(0) - - -threading.Thread(target=monitor_parent, daemon=True).start() - - -def stop(_signum, _frame): - sys.exit(0) - - -if sys.argv[1:] != ["1025", "8025"]: - sys.exit(2) - -signal.signal(signal.SIGTERM, stop) -signal.pause() diff --git a/crates/daemon/tests/daemon_foundation.rs b/crates/daemon/tests/daemon_foundation.rs index aaa0a850c..921405f9b 100644 --- a/crates/daemon/tests/daemon_foundation.rs +++ b/crates/daemon/tests/daemon_foundation.rs @@ -29,7 +29,6 @@ use std::io::{self, ErrorKind, Write as _}; use std::net::{Ipv4Addr, SocketAddr, TcpListener as StdTcpListener, UdpSocket as StdUdpSocket}; #[cfg(unix)] use std::os::fd::OwnedFd; -use std::os::unix::process::CommandExt as _; use std::str::FromStr; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, mpsc}; @@ -38,12 +37,6 @@ use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; use tokio::net::{TcpStream, UdpSocket, UnixListener, UnixStream}; use tokio::time::{Instant as TokioInstant, sleep, timeout, timeout_at}; -#[expect( - clippy::disallowed_types, - reason = "a runtime from before monitors was started outside the supervisor" -)] -type TestCommand = std::process::Command; - const EXPECTED_DNS_TTL_SECONDS: u32 = 5; const JOB_STATUS_WAIT_TIMEOUT: Duration = Duration::from_secs(10); const JOB_STATUS_POLL_INTERVAL: Duration = Duration::from_millis(50); @@ -475,7 +468,6 @@ async fn seeded_gateway_drop_does_not_block_current_thread_runtime() -> Result<( state::fs::ensure_layout(&supervisor_paths)?; pv_fake::install_monitor(&supervisor_paths)?; let output_path = tempdir.path().join("nested.output"); - let nested_pid_path = supervisor_paths.run().join("nested-test.pid"); let nested_metadata_path = supervisor_paths.run().join("nested-test.json"); let nested_release_path = paths.run().join("nested-test.release"); let fallback_ready_path = paths.run().join("nested-fallback-ready"); @@ -503,7 +495,6 @@ async fn seeded_gateway_drop_does_not_block_current_thread_runtime() -> Result<( config_path: paths.home().to_owned(), config_fingerprint: None, log_path: output_path.clone(), - pid_path: nested_pid_path.clone(), metadata_path: nested_metadata_path.clone(), resource_name: "test".to_owned(), track: "fallback".to_owned(), @@ -512,7 +503,6 @@ async fn seeded_gateway_drop_does_not_block_current_thread_runtime() -> Result<( if let Err(error) = state::fs::write_sensitive_file(&nested_release_path, "release\n") { let cleanup_result = async { child.stop(Duration::from_millis(100)).await?; - state::fs::remove_file_if_exists(&nested_pid_path)?; state::fs::remove_file_if_exists(&nested_metadata_path)?; Ok(()) } @@ -526,7 +516,6 @@ async fn seeded_gateway_drop_does_not_block_current_thread_runtime() -> Result<( let cleanup_result = async { let child_cleanup = async { child.stop(Duration::from_millis(100)).await?; - state::fs::remove_file_if_exists(&nested_pid_path)?; state::fs::remove_file_if_exists(&nested_metadata_path)?; Ok(()) } @@ -554,7 +543,6 @@ async fn seeded_gateway_drop_does_not_block_current_thread_runtime() -> Result<( let mut cleanup_failures = Vec::new(); if let Err(error) = async { child.stop(Duration::from_millis(100)).await?; - state::fs::remove_file_if_exists(&nested_pid_path)?; state::fs::remove_file_if_exists(&nested_metadata_path)?; Ok::<(), anyhow::Error>(()) } @@ -582,7 +570,6 @@ async fn seeded_gateway_drop_does_not_block_current_thread_runtime() -> Result<( } child.stop(Duration::from_millis(100)).await?; - state::fs::remove_file_if_exists(&nested_pid_path)?; state::fs::remove_file_if_exists(&nested_metadata_path)?; let output = state::fs::read_to_string(&output_path)?; assert!( @@ -887,10 +874,7 @@ async fn fallback_shutdown_preserves_pending_matching_worker_reload() -> Result< let supervisor = daemon::ProcessSupervisor::new(paths.clone()); assert!( supervisor - .adopt_recorded( - &paths.worker_pid("8.4"), - &paths.worker_runtime_metadata("8.4") - )? + .adopt_recorded(&paths.worker_runtime_metadata("8.4"))? .is_some() ); state::fs::write_sensitive_file(&release_load, "release\n")?; @@ -970,10 +954,7 @@ async fn fallback_shutdown_cancels_watcher_reload_and_preserves_pending_worker() let supervisor = daemon::ProcessSupervisor::new(paths.clone()); assert!( supervisor - .adopt_recorded( - &paths.worker_pid("8.4"), - &paths.worker_runtime_metadata("8.4") - )? + .adopt_recorded(&paths.worker_runtime_metadata("8.4"))? .is_some() ); @@ -1057,11 +1038,7 @@ async fn matching_cancellation_preserves_pending_runtime_without_restore_proof() metadata["desired_config_fingerprint"] ); let supervisor = daemon::ProcessSupervisor::new(paths.clone()); - assert!( - supervisor - .adopt_recorded(&paths.worker_pid("8.4"), &metadata_path)? - .is_some() - ); + assert!(supervisor.adopt_recorded(&metadata_path)?.is_some()); state::fs::write_sensitive_file(&release_load, "release\n")?; gateway_guard.shutdown_and_cleanup().await?; @@ -1182,7 +1159,7 @@ async fn runtime_health_scanning_waits_for_startup_completion() -> Result<()> { sleep(Duration::from_millis(100)).await; let supervisor = daemon::ProcessSupervisor::new(paths.clone()); let gateway = supervisor - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? + .adopt_recorded(&paths.gateway_runtime_metadata())? .ok_or_else(|| anyhow!("Gateway was not running after startup"))?; let gateway_pid = gateway.pid(); gateway.stop(Duration::from_secs(1)).await?; @@ -1200,7 +1177,7 @@ async fn runtime_health_scanning_waits_for_startup_completion() -> Result<()> { tokio::time::resume(); wait_for_succeeded_job_scope(&paths, "resource:caddy:2").await?; let recovered_gateway = supervisor - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? + .adopt_recorded(&paths.gateway_runtime_metadata())? .ok_or_else(|| anyhow!("health recovery did not restart the Gateway"))?; let database = Database::open(&paths)?; let http_port = database @@ -1332,13 +1309,9 @@ async fn daemon_shutdown_joins_health_triggered_worker_recovery() -> Result<()> let (worker_fake, release_validation) = install_worker_validation_barrier(&paths, true)?; let supervisor = daemon::ProcessSupervisor::new(paths.clone()); let worker = supervisor - .adopt_recorded( - &paths.worker_pid("8.4"), - &paths.worker_runtime_metadata("8.4"), - )? + .adopt_recorded(&paths.worker_runtime_metadata("8.4"))? .ok_or_else(|| anyhow!("worker was not running before health recovery"))?; worker.stop(Duration::from_secs(1)).await?; - state::fs::remove_file_if_exists(&paths.worker_pid("8.4"))?; state::fs::remove_file_if_exists(&paths.worker_runtime_metadata("8.4"))?; tokio::time::pause(); @@ -1823,60 +1796,6 @@ async fn startup_reconciliation_logs_and_replaces_a_gateway_killed_while_the_dae Ok(()) } -#[tokio::test] -async fn startup_reconciliation_restarts_a_gateway_from_before_monitors_under_a_monitor() --> Result<()> { - let tempdir = tempdir()?; - let paths = PvPaths::for_home(tempdir.path().join("home")); - seed_foundation_caddy(&paths)?; - let mut gateway_guard = SeededGatewayGuard::new(paths.clone())?; - // A Gateway that a PV release from before monitors started, known by its pid file. - let legacy_caddy = tempdir.path().join("legacy-caddy-release/bin/caddy"); - pv_fake::install(&legacy_caddy, Persona::LongRunning)?; - let mut legacy = ChildGuard(TestCommand::new(&legacy_caddy).process_group(0).spawn()?); - let legacy_pid = legacy.0.id(); - state::fs::write_sensitive_file(&paths.gateway_pid(), &format!("{legacy_pid}\n"))?; - let legacy_record = json!({ - "name": "gateway", - "pid": legacy_pid, - "command": legacy_caddy, - "arguments": [], - "resource_name": "caddy", - "track": "2", - "log_path": paths.gateway_supervisor_log(), - "started_at": "2026-10-09T00:00:00Z", - "boot_session_id": platform::current_boot_session_id()?, - "process_start_identity": platform::inspect_process_start_identity(legacy_pid)?, - }); - state::fs::write_sensitive_file( - &paths.gateway_runtime_metadata(), - &serde_json::to_string(&legacy_record)?, - )?; - - let result = async { - let daemon = - daemon::RunningDaemon::start_without_managed_resource_adapters(paths.clone()).await?; - gateway_guard.attach_daemon(daemon); - wait_for_succeeded_job_id(&paths, "job_000001").await?; - let record: Value = serde_json::from_str(&state::fs::read_to_string( - &paths.gateway_runtime_metadata(), - )?)?; - - Ok::<_, anyhow::Error>(record) - } - .await; - let legacy_status = legacy.0.try_wait(); - let cleanup_result = gateway_guard.shutdown_and_cleanup().await; - let record = propagate_after_cleanup(result, cleanup_result)?; - - assert!(legacy_status?.is_some()); - assert!(!state::fs::path_entry_exists(&paths.gateway_pid())?); - assert_ne!(record["pid"], json!(legacy_pid)); - assert!(record["monitor_instance"].is_string()); - - Ok(()) -} - #[tokio::test] async fn repeated_system_requests_during_startup_create_one_trailing_job() -> Result<()> { let tempdir = tempdir()?; @@ -2158,9 +2077,7 @@ async fn update_job_refreshes_manifest_without_installed_tracks_and_persists_suc }) .await??; let supervisor = daemon::ProcessSupervisor::new(paths.clone()); - if let Some(gateway) = - supervisor.adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? - { + if let Some(gateway) = supervisor.adopt_recorded(&paths.gateway_runtime_metadata())? { gateway.stop(Duration::from_secs(1)).await?; } daemon.shutdown().await?; @@ -2706,10 +2623,7 @@ impl FoundationWorkerPortHandoff { let supervisor = daemon::ProcessSupervisor::new(paths.clone()); let recorded_pid = supervisor - .adopt_recorded( - &paths.worker_pid(php_track), - &paths.worker_runtime_metadata(php_track), - )? + .adopt_recorded(&paths.worker_runtime_metadata(php_track))? .ok_or_else(|| anyhow!("seeded FrankenPHP runtime was not adoptable"))? .pid(); daemon::wait_for_readiness( @@ -2721,10 +2635,7 @@ impl FoundationWorkerPortHandoff { ) .await?; let published_pid = supervisor - .adopt_recorded( - &paths.worker_pid(php_track), - &paths.worker_runtime_metadata(php_track), - )? + .adopt_recorded(&paths.worker_runtime_metadata(php_track))? .ok_or_else(|| anyhow!("seeded FrankenPHP runtime was not adoptable after readiness"))? .pid(); if published_pid != recorded_pid { @@ -2843,15 +2754,6 @@ impl Drop for SeededGatewayGuard { } } -struct ChildGuard(std::process::Child); - -impl Drop for ChildGuard { - fn drop(&mut self) { - let _kill_result = self.0.kill(); - let _wait_result = self.0.wait(); - } -} - fn report_seeded_gateway_cleanup_failure(paths: &PvPaths, message: &str) { let record = json!({ "level": "error", @@ -3404,10 +3306,7 @@ async fn daemon_health_automatically_recovers_killed_worker_with_invalid_project initial_daemon.shutdown().await?; let supervisor = daemon::ProcessSupervisor::new(paths.clone()); let initial_worker = supervisor - .adopt_recorded( - &paths.worker_pid("8.4"), - &paths.worker_runtime_metadata("8.4"), - )? + .adopt_recorded(&paths.worker_runtime_metadata("8.4"))? .ok_or_else(|| anyhow!("worker was not running before health recovery"))?; let initial_worker_pid = initial_worker.pid(); state::fs::write_sensitive_file(&config_path, "php: [\n")?; diff --git a/crates/daemon/tests/gateway_reconciliation.rs b/crates/daemon/tests/gateway_reconciliation.rs index fb6c79940..a58a52870 100644 --- a/crates/daemon/tests/gateway_reconciliation.rs +++ b/crates/daemon/tests/gateway_reconciliation.rs @@ -363,8 +363,8 @@ root: public )?; assert_worker_command(&paths, "8.4", &fake_frankenphp)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -565,9 +565,9 @@ async fn gateway_reconciliation_recovers_after_bounded_worker_wave_is_cancelled( assert!(metadata["staged_config_fingerprint"].is_null()); } - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; for runtime_key in runtime_keys { - stop_runtime_from_pid_file(&paths.worker_pid(&runtime_key)).await?; + stop_runtime(&paths.worker_runtime_metadata(&runtime_key)).await?; } runtime_guard.cleanup().await?; @@ -712,8 +712,8 @@ async fn matching_worker_recovers_after_post_load_readiness_is_cancelled() -> Re gateway_load_count + 1 ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid(track)).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata(track)).await?; runtime_guard.cleanup().await?; @@ -795,9 +795,9 @@ async fn runtime_move_accepts_prior_proof_after_artifact_change() -> Result<()> &paths.gateway_runtime_metadata() )?)?); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.5")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; + stop_runtime(&paths.worker_runtime_metadata("8.5")).await?; runtime_guard.cleanup().await?; @@ -1094,9 +1094,9 @@ async fn gateway_runtime_move_retains_source_until_gateway_commit() -> Result<() assert_ne!(committed_metadata["replacement_required"], true); assert!(committed_metadata["applied_config_fingerprint"].is_string()); assert!(committed_metadata["staged_config_fingerprint"].is_null()); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.5")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; + stop_runtime(&paths.worker_runtime_metadata("8.5")).await?; runtime_guard.cleanup().await?; @@ -1155,12 +1155,8 @@ async fn worker_failure_starts_missing_gateway_without_reloading_live_gateway() )?; link_project_record(&paths, &redis_project, "api.acme.test", Some(track))?; if !live_gateway { - stop_recorded_runtime_preserving_files( - &paths, - &paths.gateway_pid(), - &paths.gateway_runtime_metadata(), - ) - .await?; + stop_recorded_runtime_preserving_files(&paths, &paths.gateway_runtime_metadata()) + .await?; let tampered = format!( "{}# tampered Gateway root\n", fs::read_to_string(&paths.gateway_root_config())? @@ -1196,10 +1192,10 @@ async fn worker_failure_starts_missing_gateway_without_reloading_live_gateway() assert_eq!(fs::read_to_string(&source_fragment)?, source_content); fs::remove_file(&redis_failure_marker)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid(&base_runtime_key)).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata(&base_runtime_key)).await?; if paths.worker_runtime_metadata(&redis_runtime_key).exists() { - stop_runtime_from_pid_file(&paths.worker_pid(&redis_runtime_key)).await?; + stop_runtime(&paths.worker_runtime_metadata(&redis_runtime_key)).await?; } runtime_guard.cleanup().await?; } @@ -1356,12 +1352,7 @@ async fn failed_worker_readiness_does_not_cancel_siblings_or_reload_gateway() -> None }; let peer_content_after_failure = fs::read_to_string(&peer_fragment)?; - stop_recorded_runtime_preserving_files( - &paths, - &paths.gateway_pid(), - &paths.gateway_runtime_metadata(), - ) - .await?; + stop_recorded_runtime_preserving_files(&paths, &paths.gateway_runtime_metadata()).await?; let verified_gateway_root = fs::read_to_string(&paths.gateway_root_config())?; fs::write_sensitive_file( &paths.gateway_root_config(), @@ -1420,10 +1411,10 @@ async fn failed_worker_readiness_does_not_cancel_siblings_or_reload_gateway() -> required_runtime_metadata_pid(&paths.worker_runtime_metadata(&xdebug_runtime_key))?, xdebug_worker_pid ); - stop_runtime_from_pid_file(&paths.worker_pid(&redis_runtime_key)).await?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid(&base_runtime_key)).await?; - stop_runtime_from_pid_file(&paths.worker_pid(&xdebug_runtime_key)).await?; + stop_runtime(&paths.worker_runtime_metadata(&redis_runtime_key)).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata(&base_runtime_key)).await?; + stop_runtime(&paths.worker_runtime_metadata(&xdebug_runtime_key)).await?; assert!( matches!( @@ -1484,7 +1475,7 @@ async fn gateway_reconciliation_starts_gateway_without_linked_projects() -> Resu assert!(paths.gateway_runtime_metadata().exists()); assert!(!paths.worker_runtime_metadata("8.4").exists()); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; runtime_guard.cleanup().await?; @@ -1549,7 +1540,7 @@ async fn assert_uncertain_pf_state_preserves_gateway( Database::open(&paths)?.runtime_observed_states()?, )?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; runtime_guard.cleanup().await?; @@ -1595,7 +1586,7 @@ async fn fresh_gateway_ownership_probe_failure_cleans_runtime_before_rollback() { let supervisor = ProcessSupervisor::new(paths.clone()); let gateway = supervisor - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? + .adopt_recorded(&paths.gateway_runtime_metadata())? .ok_or_else(|| anyhow::anyhow!("fresh Gateway was not adoptable"))?; fs::write_sensitive_file(&paths.gateway_runtime_metadata(), "{")?; fs::write_sensitive_file(&admin_response_gate, "")?; @@ -1838,8 +1829,8 @@ env: let invalid_defaults_result = reconcile_gateway_runtimes(&paths).await; let runtime_states = Database::open(&paths)?.runtime_observed_states()?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; assert_eq!(second_gateway_pid, first_gateway_pid); assert_eq!(second_worker_pid, first_worker_pid); @@ -1972,8 +1963,8 @@ async fn unchanged_gateway_config_is_rehardened() -> Result<()> { }; let admin_loads = fake_admin_load_bodies(&root_config)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; assert_eq!(result?, GATEWAY_RECONCILIATION_SUMMARY); assert_eq!(second_gateway_pid, first_gateway_pid); @@ -2432,9 +2423,9 @@ env: .map(|state| state.status); assert_eq!(worker_83_status, Some(RuntimeObservedStatus::Running)); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; for track in ["8.3", "8.4", "8.5"] { - stop_runtime_from_pid_file(&paths.worker_pid(track)).await?; + stop_runtime(&paths.worker_runtime_metadata(track)).await?; } runtime_guard.cleanup().await?; @@ -2570,8 +2561,8 @@ async fn targeted_project_reconciliation_does_not_activate_tampered_unrelated_fr worker_loads ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -2715,8 +2706,8 @@ async fn targeted_project_reconciliation_promotes_split_peer_runtime_state() -> ); assert!(!paths.worker_runtime_metadata("8.5").exists()); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -2833,8 +2824,8 @@ async fn targeted_project_reconciliation_uses_verified_fragment_snapshot() -> Re peer_gateway_fragment ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -3029,7 +3020,7 @@ async fn targeted_project_reconciliation_preserves_old_route_until_new_worker_is wait_for_process_exit(new_worker_pid).await?; assert!(!paths.worker_runtime_metadata("8.5").exists()); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; runtime_guard.cleanup().await?; @@ -3077,7 +3068,7 @@ async fn gateway_reconciliation_takes_full_path_when_applied_fingerprint_is_miss let metadata: Value = serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; assert_eq!(second_gateway_pid, first_gateway_pid); assert_eq!( @@ -3150,7 +3141,7 @@ async fn gateway_reconciliation_validates_changed_or_missing_ca_key() -> Result< )); } - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; assert_eq!(unchanged_result?, GATEWAY_RECONCILIATION_SUMMARY); assert_eq!(unchanged_metadata["pid"], initial_metadata["pid"]); @@ -3280,7 +3271,7 @@ async fn gateway_reconciliation_keeps_pending_state_when_applied_fingerprint_com serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; wait_for_process_exit(first_gateway_pid).await?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; assert_ne!(replacement_gateway_pid, first_gateway_pid); assert_ne!(replacement_metadata["replacement_required"], true); @@ -3333,12 +3324,8 @@ root: public drop(database); reconcile_gateway_runtimes(&paths).await?; - let first_gateway_pid = - runtime_guard.capture(paths.gateway_pid(), paths.gateway_runtime_metadata())?; - let first_worker_pid = runtime_guard.capture( - paths.worker_pid("8.4"), - paths.worker_runtime_metadata("8.4"), - )?; + let first_gateway_pid = runtime_guard.capture(paths.gateway_runtime_metadata())?; + let first_worker_pid = runtime_guard.capture(paths.worker_runtime_metadata("8.4"))?; replace_runtime_metadata_identity(&paths.gateway_runtime_metadata(), "gateway", "core")?; replace_runtime_metadata_identity(&paths.worker_runtime_metadata("8.4"), "php-worker", "8.4")?; @@ -3361,8 +3348,8 @@ root: public assert_eq!(stable_gateway_pid, second_gateway_pid); assert_eq!(stable_worker_pid, second_worker_pid); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; Ok(()) @@ -3406,7 +3393,7 @@ async fn gateway_reconciliation_replaces_legacy_admin_off_process_before_admin_c let mut legacy_process = supervisor.start(legacy_spec).await?; assert!( supervisor - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata(),)? + .adopt_recorded(&paths.gateway_runtime_metadata(),)? .is_some(), "legacy fixture must be adoptable before reconciliation" ); @@ -3448,7 +3435,7 @@ async fn gateway_reconciliation_replaces_legacy_admin_off_process_before_admin_c .ok_or_else(|| anyhow::anyhow!("expected stable gateway metadata"))?; assert_eq!(stable_pid, replacement_pid); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; runtime_guard.cleanup().await?; @@ -3501,7 +3488,7 @@ root: public let first_metadata_json: serde_json::Value = serde_json::from_str(&first_metadata)?; let first_gateway_pid = metadata_pid(&first_metadata_json)?; let first_gateway = ProcessSupervisor::new(paths.clone()) - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? + .adopt_recorded(&paths.gateway_runtime_metadata())? .ok_or_else(|| anyhow::anyhow!("first Gateway was not adoptable"))?; assert_eq!(first_gateway.pid(), first_gateway_pid); first_gateway.stop(Duration::from_secs(1)).await?; @@ -3509,8 +3496,8 @@ root: public reconcile_gateway_runtimes(&paths).await?; let second_metadata = state::testing::read_to_string(&paths.gateway_runtime_metadata())?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; assert_ne!(first_metadata, second_metadata); @@ -3565,7 +3552,7 @@ root: public let first_metadata_json: serde_json::Value = serde_json::from_str(&first_metadata)?; let first_gateway_pid = metadata_pid(&first_metadata_json)?; assert_eq!( - runtime_guard.capture(paths.gateway_pid(), paths.gateway_runtime_metadata())?, + runtime_guard.capture(paths.gateway_runtime_metadata())?, first_gateway_pid ); // As a start cancelled after its monitor began: the runtime's monitor is PV's own, so its @@ -3648,10 +3635,10 @@ root: public https_server.abort(); drop(http_listener); if paths.worker_runtime_metadata("8.4").exists() { - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; } if paths.gateway_runtime_metadata().exists() { - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; } assert!( @@ -3841,9 +3828,8 @@ root: public .await; let daemon_log_result = state::fs::read_to_string(&paths.daemon_log()); - let surviving_worker_cleanup_result = - stop_runtime_from_pid_file(&paths.worker_pid("8.3")).await; - let gateway_cleanup_result = stop_runtime_from_pid_file(&paths.gateway_pid()).await; + let surviving_worker_cleanup_result = stop_runtime(&paths.worker_runtime_metadata("8.3")).await; + let gateway_cleanup_result = stop_runtime(&paths.gateway_runtime_metadata()).await; targeted_reconcile_result?; let daemon_log = daemon_log_result?; @@ -4029,8 +4015,8 @@ root: public edited_worker_fragment ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -4110,8 +4096,8 @@ root: public .exists() ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -4196,12 +4182,12 @@ root: public None => false, }; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; if worker_83_alive { - stop_runtime_from_pid_file(&paths.worker_pid("8.3")).await?; + stop_runtime(&paths.worker_runtime_metadata("8.3")).await?; } if worker_84_alive { - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; } assert!(worker_83_alive); @@ -4437,9 +4423,9 @@ env: state::ProjectEnvObservedStatus::Failed )); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.3")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; + stop_runtime(&paths.worker_runtime_metadata("8.3")).await?; runtime_guard.cleanup().await?; @@ -4518,8 +4504,8 @@ root: . ); assert_eq!(fs::read_to_string(&worker_fragment_path)?, worker_fragment); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -4631,9 +4617,9 @@ root: web assert!(!committed.contains("old.acme.test")); assert!(committed.contains("new.acme.test")); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid(&base_runtime_key)).await?; - stop_runtime_from_pid_file(&paths.worker_pid(&redis_runtime_key)).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata(&base_runtime_key)).await?; + stop_runtime(&paths.worker_runtime_metadata(&redis_runtime_key)).await?; runtime_guard.cleanup().await?; @@ -4807,11 +4793,11 @@ root: web .count(); assert_eq!(stale_ports, 0); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; for track in ["8.4", "8.5"] { - let pid_path = paths.worker_pid(track); - if pid_path.exists() { - stop_runtime_from_pid_file(&pid_path).await?; + let metadata_path = paths.worker_runtime_metadata(track); + if metadata_path.exists() { + stop_runtime(&metadata_path).await?; } } @@ -4925,9 +4911,9 @@ root: public let gateway_root = read_test_bytes(paths.gateway_root_config())?; let worker_root = read_test_bytes(paths.worker_root_config("8.4"))?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.3")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; + stop_runtime(&paths.worker_runtime_metadata("8.3")).await?; assert_eq!(first_gateway_pid, second_gateway_pid); assert_eq!(first_worker_pid, second_worker_pid); @@ -5068,8 +5054,8 @@ root: public let restored_metadata: Value = serde_json::from_str(&fs::read_to_string(&paths.worker_runtime_metadata("8.4"))?)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; assert!( matches!(&result, Err(DaemonError::ReadinessTimedOut { .. })), @@ -5190,7 +5176,7 @@ async fn gateway_reconciliation_rejection_keeps_old_runtime_and_disk_state() -> vec![Some(422)] ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; runtime_guard.cleanup().await?; @@ -5235,7 +5221,7 @@ async fn gateway_reconciliation_confirms_reverted_desired_config_without_reload( serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; let final_loads = fake_admin_load_bodies(&paths.gateway_root_config())?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; result?; assert_eq!(final_gateway_pid, initial_gateway_pid); @@ -5298,7 +5284,7 @@ async fn gateway_reconciliation_rejects_load_error_reported_with_success_status( let root_after = read_test_bytes(paths.gateway_root_config())?; let current_config = fake_admin_current_bytes(&paths.gateway_root_config())?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; assert!(matches!( result, @@ -5410,7 +5396,7 @@ async fn gateway_reconciliation_replaces_runtime_before_newer_desired_state() -> let replacement_metadata: Value = serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; assert_eq!(recovery_summary, "Gateway runtime reconciled"); assert_ne!(first_gateway_pid, second_gateway_pid); @@ -5501,7 +5487,7 @@ async fn gateway_reconciliation_preserves_accepted_config_for_pf_state( let metadata: Value = serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; assert_eq!(first_gateway_pid, second_gateway_pid); assert_eq!(first_gateway_pid, third_gateway_pid); @@ -5544,7 +5530,7 @@ async fn gateway_reconciliation_does_not_reload_after_runtime_exits_after_load() reconcile_gateway_runtimes(&paths).await?; let supervisor = ProcessSupervisor::new(paths.clone()); let captured_gateway = supervisor - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? + .adopt_recorded(&paths.gateway_runtime_metadata())? .ok_or_else(|| anyhow::anyhow!("Gateway was not adoptable before its forced exit"))?; let gateway_pid = captured_gateway.pid(); let previous_root = read_test_bytes(paths.gateway_root_config())?; @@ -5571,7 +5557,7 @@ async fn gateway_reconciliation_does_not_reload_after_runtime_exits_after_load() timeout(Duration::from_secs(5), async { loop { if supervisor - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? + .adopt_recorded(&paths.gateway_runtime_metadata())? .is_none() { return Ok::<_, anyhow::Error>(()); @@ -5590,7 +5576,6 @@ async fn gateway_reconciliation_does_not_reload_after_runtime_exits_after_load() .chain(&new_ports) .map(|port| TcpListener::bind(("127.0.0.1", *port))) .collect::>>()?; - fs::remove_file_if_exists(&paths.gateway_pid())?; fs::remove_file_if_exists(&paths.gateway_runtime_metadata())?; runtime_guard.cleanup().await } @@ -5653,7 +5638,7 @@ async fn gateway_reconciliation_reports_compound_restore_failure() -> Result<()> let metadata: Value = serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; let Err(DaemonError::CaddyAdmin(CaddyAdminError::RestoredConfigReloadFailed { original_error, @@ -5782,8 +5767,8 @@ root: public serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; let load_bodies = fake_admin_load_bodies(&paths.gateway_root_config())?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; let Err(DaemonError::CaddyAdmin(CaddyAdminError::RestoredConfigReloadFailed { restored_error, @@ -5856,7 +5841,7 @@ async fn gateway_reconciliation_reports_compound_restored_readiness_failure() -> let metadata: Value = serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; let Err(DaemonError::CaddyAdmin(CaddyAdminError::RestoredConfigReloadFailed { original_error, @@ -5931,7 +5916,7 @@ async fn gateway_reconciliation_rolls_back_config_when_runtime_readiness_fails() let previous_metadata: Value = serde_json::from_str(&fs::read_to_string(&paths.gateway_runtime_metadata())?)?; let first_gateway = ProcessSupervisor::new(paths.clone()) - .adopt_recorded(&paths.gateway_pid(), &paths.gateway_runtime_metadata())? + .adopt_recorded(&paths.gateway_runtime_metadata())? .ok_or_else(|| anyhow::anyhow!("first Gateway was not adoptable"))?; assert_eq!(first_gateway.pid(), first_gateway_pid); @@ -5956,10 +5941,9 @@ async fn gateway_reconciliation_rolls_back_config_when_runtime_readiness_fails() let first_gateway_is_alive = process_is_alive(first_gateway_pid)?; if first_gateway_is_alive { first_gateway.stop(Duration::from_secs(1)).await?; - fs::remove_file_if_exists(&paths.gateway_pid())?; fs::remove_file_if_exists(&paths.gateway_runtime_metadata())?; } else if paths.gateway_runtime_metadata().exists() { - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; } let Err(error) = result else { @@ -7198,9 +7182,9 @@ impl GatewayRuntimeGuard { } /// The pid of the runtime recorded at `metadata_path`. - fn capture(&mut self, _pid_path: Utf8PathBuf, metadata_path: Utf8PathBuf) -> Result { + fn capture(&mut self, metadata_path: Utf8PathBuf) -> Result { let process = ProcessSupervisor::new(self.paths.clone()) - .adopt_recorded(&metadata_path.with_extension("pid"), &metadata_path)? + .adopt_recorded(&metadata_path)? .ok_or_else(|| anyhow::anyhow!("runtime at {metadata_path} was not adoptable"))?; Ok(process.pid()) @@ -7261,38 +7245,32 @@ fn report_gateway_cleanup_failure(paths: &PvPaths, message: &str) { let _write_result = writeln!(std::io::stderr().lock(), "{record}"); } -/// Stops the runtime whose records sit at `path`, through its monitor, and removes its record. -async fn stop_runtime_from_pid_file(path: &Utf8Path) -> Result<()> { - let metadata_path = path.with_extension("json"); - let pv_root = path +/// Stops the runtime whose record is at `metadata_path`, through its monitor, and removes the +/// record. +async fn stop_runtime(metadata_path: &Utf8Path) -> Result<()> { + let pv_root = metadata_path .ancestors() .find(|ancestor| ancestor.file_name() == Some(".pv")) - .ok_or_else(|| anyhow::anyhow!("runtime path is outside a PV home: {path}"))?; + .ok_or_else(|| anyhow::anyhow!("runtime record is outside a PV home: {metadata_path}"))?; let home = pv_root .parent() .ok_or_else(|| anyhow::anyhow!("PV root has no home directory: {pv_root}"))?; ProcessSupervisor::new(PvPaths::for_home(home)) - .stop_subject( - &metadata_path, - StopSignal::Terminate, - Duration::from_secs(1), - ) + .stop_subject(metadata_path, StopSignal::Terminate, Duration::from_secs(1)) .await?; - fs::remove_file_if_exists(path)?; - fs::remove_file_if_exists(&metadata_path)?; + fs::remove_file_if_exists(metadata_path)?; Ok(()) } async fn stop_recorded_runtime_preserving_files( paths: &PvPaths, - pid_path: &Utf8Path, metadata_path: &Utf8Path, ) -> Result<()> { let supervisor = ProcessSupervisor::new(paths.clone()); let runtime = supervisor - .adopt_recorded(pid_path, metadata_path)? - .ok_or_else(|| anyhow::anyhow!("runtime at {pid_path} was not adoptable"))?; + .adopt_recorded(metadata_path)? + .ok_or_else(|| anyhow::anyhow!("runtime at {metadata_path} was not adoptable"))?; runtime.stop(Duration::from_secs(1)).await?; Ok(()) @@ -7529,7 +7507,7 @@ async fn resource_only_target_recovers_alive_unready_gateway_with_invalid_config .find(|state| state.subject == RuntimeSubject::Gateway) .map(|state| state.status); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; assert_ne!(recovered_gateway_pid, initial_gateway_pid); assert!(fs::read_to_string(&paths.gateway_root_config())?.contains("PV Gateway is running")); @@ -7596,8 +7574,8 @@ async fn targeted_inspection_promotes_only_recoverable_uncertainty() -> Result<( assert_eq!(summary, GATEWAY_RECONCILIATION_SUMMARY); assert!(paths.gateway_runtime_metadata().exists()); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; } @@ -7699,8 +7677,8 @@ async fn targeted_inspection_promotes_only_recoverable_uncertainty() -> Result<( gateway_pid ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; } @@ -7782,8 +7760,8 @@ async fn gateway_failure_preserves_primary_error_when_observation_write_fails() "expected the recording SQLite failure, got {cleanup:?}" ); - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; runtime_guard.cleanup().await?; @@ -7863,8 +7841,8 @@ async fn targeted_reconciliation_reports_alive_unready_worker() -> Result<()> { let still_owned = supervisor.verify_ownership(&spec)?.is_some(); let still_unready = TcpStream::connect(("127.0.0.1", ports[2])).await.is_err(); let final_pid = required_runtime_metadata_pid(&paths.worker_runtime_metadata("8.4"))?; - stop_runtime_from_pid_file(&paths.gateway_pid()).await?; - stop_runtime_from_pid_file(&paths.worker_pid("8.4")).await?; + stop_runtime(&paths.gateway_runtime_metadata()).await?; + stop_runtime(&paths.worker_runtime_metadata("8.4")).await?; let worker_status = Database::open(&paths)? .runtime_observed_states()? .into_iter() diff --git a/crates/daemon/tests/snapshots/gateway_reconciliation__caddy_cli_command_and_process_specs_are_stable.snap b/crates/daemon/tests/snapshots/gateway_reconciliation__caddy_cli_command_and_process_specs_are_stable.snap index 0e92bd28b..e0c24bd91 100644 --- a/crates/daemon/tests/snapshots/gateway_reconciliation__caddy_cli_command_and_process_specs_are_stable.snap +++ b/crates/daemon/tests/snapshots/gateway_reconciliation__caddy_cli_command_and_process_specs_are_stable.snap @@ -33,7 +33,6 @@ expression: snapshot }, config_path: "/home/.pv/config/gateway/Caddyfile", log_path: "/home/.pv/logs/gateway/supervisor.log", - pid_path: "/home/.pv/run/gateway.pid", metadata_path: "/home/.pv/run/gateway.json", resource_name: "caddy", track: "2", @@ -56,7 +55,6 @@ expression: snapshot }, config_path: "/home/.pv/config/workers/php-8.4/Caddyfile", log_path: "/home/.pv/logs/workers/php-8.4.log", - pid_path: "/home/.pv/run/workers/php-8.4.pid", metadata_path: "/home/.pv/run/workers/php-8.4.json", resource_name: "frankenphp", track: "8.4", diff --git a/crates/daemon/tests/snapshots/supervisor_foundation__process_spec_debug_omits_empty_private_environment_and_redacts_values.snap b/crates/daemon/tests/snapshots/supervisor_foundation__process_spec_debug_omits_empty_private_environment_and_redacts_values.snap index 395ff428c..e97bb6d2d 100644 --- a/crates/daemon/tests/snapshots/supervisor_foundation__process_spec_debug_omits_empty_private_environment_and_redacts_values.snap +++ b/crates/daemon/tests/snapshots/supervisor_foundation__process_spec_debug_omits_empty_private_environment_and_redacts_values.snap @@ -1,6 +1,6 @@ --- source: crates/daemon/tests/supervisor_foundation.rs -expression: (empty_private_env, private_env) +expression: "(empty_private_env, private_env)" --- ( ProcessSpec { @@ -12,7 +12,6 @@ expression: (empty_private_env, private_env) ], config_path: "/home/.pv/config/empty-private-env.json", log_path: "/home/.pv/logs/empty-private-env.log", - pid_path: "/home/.pv/run/empty-private-env.pid", metadata_path: "/home/.pv/run/empty-private-env.json", resource_name: "empty-private-env", track: "test", @@ -30,7 +29,6 @@ expression: (empty_private_env, private_env) }, config_path: "/home/.pv/config/private-env.json", log_path: "/home/.pv/logs/private-env.log", - pid_path: "/home/.pv/run/private-env.pid", metadata_path: "/home/.pv/run/private-env.json", resource_name: "private-env", track: "test", diff --git a/crates/daemon/tests/supervisor_foundation.rs b/crates/daemon/tests/supervisor_foundation.rs index 32b429c3e..8e37aba5e 100644 --- a/crates/daemon/tests/supervisor_foundation.rs +++ b/crates/daemon/tests/supervisor_foundation.rs @@ -30,11 +30,6 @@ use tokio_rustls::TlsAcceptor; )] type TestProcessCommand = std::process::Command; -#[cfg(target_os = "macos")] -const OWNED_PYTHON_RUNTIME_SCRIPT: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/test-fixtures/supervisor/owned-python-runtime.py" -)); /// Keeps a shell alive until it is stopped, but for 150 s at most, so a test that dies before /// stopping it leaves nothing running for long. That outlasts the CI profile's 120 s limit on a /// test, so a stalled test can't pass because its fixture exited on its own. @@ -594,7 +589,6 @@ async fn supervisor_stops_its_runtime_when_runtime_metadata_persistence_fails() pv_fake::install_monitor(&paths)?; let supervisor = ProcessSupervisor::new(paths.clone()); let metadata_parent_blocker = paths.run().join("metadata-parent"); - let pid_path = paths.run().join("metadata-failure.pid"); state::fs::write_sensitive_file(&metadata_parent_blocker, "not a directory")?; let result = supervisor @@ -606,7 +600,6 @@ async fn supervisor_stops_its_runtime_when_runtime_metadata_persistence_fails() config_path: paths.config().join("metadata-failure.json"), config_fingerprint: None, log_path: paths.logs().join("metadata-failure.log"), - pid_path: pid_path.clone(), metadata_path: metadata_parent_blocker.join("metadata.json"), resource_name: "metadata-failure".to_string(), track: "test".to_string(), @@ -681,7 +674,7 @@ async fn supervisor_verifies_and_adopts_owned_runtime_metadata() -> Result<()> { assert!(replacement.replacement_required()); assert!(supervisor.adopt(&spec)?.is_some()); let replacement = supervisor - .adopt_recorded(&spec.pid_path, &spec.metadata_path)? + .adopt_recorded(&spec.metadata_path)? .ok_or_else(|| anyhow!("replacement-required runtime was not adoptable by its record"))?; assert_eq!(replacement.pid(), process.pid()); assert!(supervisor.clear_replacement_required(&spec)?); @@ -705,45 +698,6 @@ async fn supervisor_verifies_and_adopts_owned_runtime_metadata() -> Result<()> { Ok(()) } -#[cfg(target_os = "macos")] -#[tokio::test] -async fn supervisor_verifies_owned_python_shebang_script() -> Result<()> { - let tempdir = tempdir()?; - let paths = PvPaths::for_home(tempdir.path().join("home")); - state::fs::ensure_layout(&paths)?; - pv_fake::install_monitor(&paths)?; - let runtime = paths.run().join("owned-python-runtime"); - state::fs::write_sensitive_file(&runtime, OWNED_PYTHON_RUNTIME_SCRIPT)?; - set_executable(&runtime)?; - - let supervisor = ProcessSupervisor::new(paths.clone()); - let spec = process_spec( - &paths, - "owned-python-runtime", - runtime, - vec!["1025".to_string(), "8025".to_string()], - ); - let process = supervisor.start(spec.clone()).await?; - let pid = process.pid(); - let ownership = timeout(Duration::from_secs(1), async { - loop { - if let Some(owned) = supervisor.verify_ownership(&spec)? { - return Ok::<_, daemon::DaemonError>(owned); - } - - sleep(Duration::from_millis(10)).await; - } - }) - .await; - - process.stop(Duration::from_secs(1)).await?; - let owned = ownership??; - - assert_eq!(owned.pid(), pid); - - Ok(()) -} - #[cfg(target_os = "macos")] #[tokio::test] async fn supervisor_owns_pv_fake_by_direct_identity_with_an_armed_lifeline() -> Result<()> { @@ -760,13 +714,10 @@ async fn supervisor_owns_pv_fake_by_direct_identity_with_an_armed_lifeline() -> let process = supervisor.start(spec.clone()).await?; let lifeline_armed = wait_for_fake_start(&fake).await; let owned = supervisor.verify_ownership(&spec); - let metadata = runtime_metadata(&spec.metadata_path); process.stop(Duration::from_secs(1)).await?; assert!(lifeline_armed?); assert!(owned?.is_some()); - // Script runtimes record an executable identity; a directly matched binary does not. - assert_eq!(metadata?.get("process_executable_identity"), None); Ok(()) } @@ -904,7 +855,7 @@ async fn adopted_stop_accepts_runtime_already_stopped_by_its_owner() -> Result<( ); let process = supervisor.start(spec.clone()).await?; let adopted = supervisor - .adopt_recorded(&spec.pid_path, &spec.metadata_path)? + .adopt_recorded(&spec.metadata_path)? .ok_or_else(|| anyhow!("runtime was not adoptable before owner cleanup"))?; process.stop(Duration::from_secs(1)).await?; @@ -1050,7 +1001,6 @@ fn process_spec( config_path: paths.config().join(format!("{name}.json")), config_fingerprint: None, log_path: paths.logs().join(format!("{name}.log")), - pid_path: paths.run().join(format!("{name}.pid")), metadata_path: paths.run().join(format!("{name}.json")), resource_name: name.to_string(), track: "test".to_string(), diff --git a/crates/state/src/paths.rs b/crates/state/src/paths.rs index 74f71e058..d93f8bf95 100644 --- a/crates/state/src/paths.rs +++ b/crates/state/src/paths.rs @@ -260,27 +260,14 @@ impl PvPaths { .join(format!("resources/{resource_name}/{track}.log")) } - pub fn gateway_pid(&self) -> Utf8PathBuf { - self.run().join("gateway.pid") - } - pub fn gateway_runtime_metadata(&self) -> Utf8PathBuf { self.run().join("gateway.json") } - pub fn worker_pid(&self, php_runtime: &str) -> Utf8PathBuf { - self.run().join(format!("workers/php-{php_runtime}.pid")) - } - pub fn worker_runtime_metadata(&self, php_runtime: &str) -> Utf8PathBuf { self.run().join(format!("workers/php-{php_runtime}.json")) } - pub fn resource_pid(&self, resource_name: &str, track: &str) -> Utf8PathBuf { - self.run() - .join(format!("resources/{resource_name}/{track}.pid")) - } - pub fn resource_runtime_metadata(&self, resource_name: &str, track: &str) -> Utf8PathBuf { self.run() .join(format!("resources/{resource_name}/{track}.json")) diff --git a/crates/state/tests/state_foundation.rs b/crates/state/tests/state_foundation.rs index 21fb06d62..abea5ddd9 100644 --- a/crates/state/tests/state_foundation.rs +++ b/crates/state/tests/state_foundation.rs @@ -103,10 +103,6 @@ fn pv_paths_include_gateway_and_worker_runtime_artifacts() { paths.worker_log("8.4").as_str(), "/Users/alice/.pv/logs/workers/php-8.4.log" ); - assert_eq!( - paths.gateway_pid().as_str(), - "/Users/alice/.pv/run/gateway.pid" - ); assert_eq!( paths.gateway_admin_socket().as_str(), "/Users/alice/.pv/run/gateway-admin.sock" @@ -123,10 +119,6 @@ fn pv_paths_include_gateway_and_worker_runtime_artifacts() { paths.gateway_runtime_metadata().as_str(), "/Users/alice/.pv/run/gateway.json" ); - assert_eq!( - paths.worker_pid("8.4").as_str(), - "/Users/alice/.pv/run/workers/php-8.4.pid" - ); assert_eq!( paths.worker_runtime_metadata("8.4").as_str(), "/Users/alice/.pv/run/workers/php-8.4.json" @@ -139,10 +131,6 @@ fn pv_paths_include_gateway_and_worker_runtime_artifacts() { paths.resource_log("mailpit", "1.0").as_str(), "/Users/alice/.pv/logs/resources/mailpit/1.0.log" ); - assert_eq!( - paths.resource_pid("mailpit", "1.0").as_str(), - "/Users/alice/.pv/run/resources/mailpit/1.0.pid" - ); assert_eq!( paths.resource_runtime_metadata("mailpit", "1.0").as_str(), "/Users/alice/.pv/run/resources/mailpit/1.0.json" From f810dd99cceca3e7244c959c0db9e57e1e03fd48 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 11 Oct 2026 16:46:13 -0400 Subject: [PATCH 2/4] fix(daemon): keep monitor-only group stops out of other platforms' builds With the pid-file stop gone, only the macOS monitor stops a process group, so stop_process_group, wait_for_process_group_exit and the Stop signal variant are macOS-only. --- crates/daemon/src/supervisor.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/daemon/src/supervisor.rs b/crates/daemon/src/supervisor.rs index 91e7607b5..90d2033f0 100644 --- a/crates/daemon/src/supervisor.rs +++ b/crates/daemon/src/supervisor.rs @@ -44,6 +44,7 @@ pub enum StopSignal { #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum ProcessSignal { + #[cfg(target_os = "macos")] Stop(StopSignal), Kill, } @@ -1038,6 +1039,7 @@ fn signal_process_group(_pid: u32, _signal: ProcessSignal) -> Result<(), DaemonE /// Signals `pid`'s process group with `signal`, then SIGKILL once `grace_period` passes, and /// waits until no live member remains. Escalates only while `leader_still_owned` proves the /// group still belongs to the runtime. +#[cfg(target_os = "macos")] pub(crate) async fn stop_process_group( pid: u32, signal: StopSignal, @@ -1070,6 +1072,7 @@ pub(crate) async fn stop_process_group( .into()) } +#[cfg(target_os = "macos")] pub(crate) async fn wait_for_process_group_exit( pid: u32, readiness_timeout: Duration, From e9a1bb64084da222ac1e5a80d7fa3d024053db12 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Mon, 12 Oct 2026 01:10:57 -0400 Subject: [PATCH 3/4] fix(release): default app releases to requiring PV 0.2.6 Only 0.2.6 moves runtimes from pid files onto monitors, so a later release that allows an older installed PV would let `pv update` skip that step. The release workflow's minimum_pv_version now defaults to 0.2.6 instead of 0.1.0, and the release skill says never to go below it. CONTRIBUTING.md no longer promises that a monitor always proves its runtime's group gone when a test's lifeline closes. --- .agents/skills/release-app-version/SKILL.md | 2 +- .github/workflows/app-release.yml | 4 ++-- CONTRIBUTING.md | 6 ++++-- DESIGN.md | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.agents/skills/release-app-version/SKILL.md b/.agents/skills/release-app-version/SKILL.md index 9bb45fdc4..c91dcd5cf 100644 --- a/.agents/skills/release-app-version/SKILL.md +++ b/.agents/skills/release-app-version/SKILL.md @@ -48,7 +48,7 @@ Use this workflow for PV application releases. This is separate from Managed Res - `PV App Publication`: git ref, `source_run_id` - whether this is build-only or build-and-publish -Do not silently rely on defaults. Current checked-in defaults are `minimum_pv_version=0.1.0` and `app_platforms=darwin-arm64`, but always re-read the workflow first. +Do not silently rely on defaults. Current checked-in defaults are `minimum_pv_version=0.2.6` and `app_platforms=darwin-arm64`, but always re-read the workflow first. Never set `minimum_pv_version` below 0.2.6 for a release after 0.2.6: only 0.2.6 moves runtimes from pid files onto monitors, so a lower minimum lets `pv update` skip that step. ## Bump The App Version diff --git a/.github/workflows/app-release.yml b/.github/workflows/app-release.yml index ed9e51ab1..62a32d8fc 100644 --- a/.github/workflows/app-release.yml +++ b/.github/workflows/app-release.yml @@ -4,9 +4,9 @@ on: workflow_dispatch: inputs: minimum_pv_version: - description: "Minimum installed PV version that may apply this app release" + description: "Minimum installed PV version that may apply this app release; at least 0.2.6, which moves pid-file runtimes onto monitors" required: true - default: "0.1.0" + default: "0.2.6" type: string app_platforms: description: "Comma-separated app platforms to release; default is the Apple Silicon preview lane" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9d29a7035..3e4b02b79 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,8 +58,10 @@ Fixture guards also clean up from `Drop` where they can, so normal return, early return, panic unwind, and cancellation cannot bypass teardown. The fallback uses an independent cleanup runtime and reports failures, because `Drop` cannot return them to the test. Whenever the test process exits, even by -SIGKILL, its lifeline closes and every monitor it started stops its runtime and -exits. A failed test therefore leaves nothing running. +SIGKILL, its lifeline closes and every monitor it started stops its runtime's +process group and exits. If a monitor can't prove the group is gone, it keeps its +records for recovery, so check for leftover processes after an unexplained +failure. Long-running fixtures that stand in for a runtime use `pv-fake`, not a script. `pv_fake::install(executable, persona)` places a native fake at the diff --git a/DESIGN.md b/DESIGN.md index bd96e5be8..8aee4fbf4 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -734,7 +734,7 @@ The daemon writes a minimal startup failure marker at `~/.pv/run/daemon-startup- `pv update` does not create an extra `pv.db` backup before every PV application self-update. The embedded migration system creates a timestamped backup only when migrations are about to run. -PV application self-update restarts the daemon/control plane but does not stop currently running Gateway, Project-serving workers, or backing Managed Resource processes before swapping the PV binary. The updated daemon automatically schedules startup System reconciliation to apply current desired state, adopting running runtimes through their monitors. A monitor never re-executes its binary after startup, so pruning an older release does not affect it. PV 0.2.6 is the only release that moves runtimes PV started before monitors, which it tracked by pid files, onto monitors. Later releases set `minimum_pv_version` to at least 0.2.6 so `pv update` cannot skip that step. +PV application self-update restarts the daemon/control plane but does not stop currently running Gateway, Project-serving workers, or backing Managed Resource processes before swapping the PV binary. The updated daemon automatically schedules startup System reconciliation to apply current desired state, adopting running runtimes through their monitors. A monitor never re-executes its binary after startup, so pruning an older release does not affect it. PV 0.2.6 is the only release that moves runtimes PV started before monitors, which it tracked by pid files, onto monitors. Later releases set `minimum_pv_version` to at least 0.2.6, the release workflow's default, so `pv update` cannot skip that step. App rollback is attempted after activation was attempted. Helper rollback is also attempted when helper replacement itself fails, before any incompatible app can be activated. If daemon startup, daemon health, or migration health fails after activation and rollback succeeds, stdout reports `PV application: update failed; rolled back to ` and stderr reports the original failure. A generic daemon health failure is reported as `daemon did not become healthy after update`; migration failure is reported as `database migration failed after update: `. From 9b5dc5208386858201d1039fce50492e3983ea8c Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Mon, 12 Oct 2026 21:34:08 -0400 Subject: [PATCH 4/4] fix(release): reject app releases that allow PV older than 0.2.6 The default alone left an editable input: a release dispatched with minimum_pv_version 0.2.5 would still let `pv update` skip the only release that moves pid-file runtimes onto monitors. The release metadata step now fails when the minimum sorts below 0.2.6. --- .agents/skills/release-app-version/SKILL.md | 2 +- .github/workflows/app-release.yml | 6 ++++++ DESIGN.md | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.agents/skills/release-app-version/SKILL.md b/.agents/skills/release-app-version/SKILL.md index c91dcd5cf..88c4f263a 100644 --- a/.agents/skills/release-app-version/SKILL.md +++ b/.agents/skills/release-app-version/SKILL.md @@ -48,7 +48,7 @@ Use this workflow for PV application releases. This is separate from Managed Res - `PV App Publication`: git ref, `source_run_id` - whether this is build-only or build-and-publish -Do not silently rely on defaults. Current checked-in defaults are `minimum_pv_version=0.2.6` and `app_platforms=darwin-arm64`, but always re-read the workflow first. Never set `minimum_pv_version` below 0.2.6 for a release after 0.2.6: only 0.2.6 moves runtimes from pid files onto monitors, so a lower minimum lets `pv update` skip that step. +Do not silently rely on defaults. Current checked-in defaults are `minimum_pv_version=0.2.6` and `app_platforms=darwin-arm64`, but always re-read the workflow first. Never set `minimum_pv_version` below 0.2.6 for a release after 0.2.6: only 0.2.6 moves runtimes from pid files onto monitors, so a lower minimum lets `pv update` skip that step. The workflow rejects lower values. ## Bump The App Version diff --git a/.github/workflows/app-release.yml b/.github/workflows/app-release.yml index 62a32d8fc..679c68836 100644 --- a/.github/workflows/app-release.yml +++ b/.github/workflows/app-release.yml @@ -60,6 +60,12 @@ jobs: printf '%s\n' "minimum_pv_version is required" >&2 exit 1 fi + # Only 0.2.6 moves runtimes from pid files onto monitors; no later release may skip it. + lowest_minimum=$(printf '%s\n' 0.2.6 "$MINIMUM_PV_VERSION" | sort -V | head -n 1) + if [ "$lowest_minimum" != "0.2.6" ]; then + printf '%s\n' "minimum_pv_version must be at least 0.2.6, the release that moves runtimes onto monitors" >&2 + exit 1 + fi case "$HELPER_ONLY" in true|false) ;; *) diff --git a/DESIGN.md b/DESIGN.md index 8aee4fbf4..aa41604ab 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -734,7 +734,7 @@ The daemon writes a minimal startup failure marker at `~/.pv/run/daemon-startup- `pv update` does not create an extra `pv.db` backup before every PV application self-update. The embedded migration system creates a timestamped backup only when migrations are about to run. -PV application self-update restarts the daemon/control plane but does not stop currently running Gateway, Project-serving workers, or backing Managed Resource processes before swapping the PV binary. The updated daemon automatically schedules startup System reconciliation to apply current desired state, adopting running runtimes through their monitors. A monitor never re-executes its binary after startup, so pruning an older release does not affect it. PV 0.2.6 is the only release that moves runtimes PV started before monitors, which it tracked by pid files, onto monitors. Later releases set `minimum_pv_version` to at least 0.2.6, the release workflow's default, so `pv update` cannot skip that step. +PV application self-update restarts the daemon/control plane but does not stop currently running Gateway, Project-serving workers, or backing Managed Resource processes before swapping the PV binary. The updated daemon automatically schedules startup System reconciliation to apply current desired state, adopting running runtimes through their monitors. A monitor never re-executes its binary after startup, so pruning an older release does not affect it. PV 0.2.6 is the only release that moves runtimes PV started before monitors, which it tracked by pid files, onto monitors. Later releases set `minimum_pv_version` to at least 0.2.6, so `pv update` cannot skip that step. The release workflow defaults to 0.2.6 and rejects any lower value. App rollback is attempted after activation was attempted. Helper rollback is also attempted when helper replacement itself fails, before any incompatible app can be activated. If daemon startup, daemon health, or migration health fails after activation and rollback succeeds, stdout reports `PV application: update failed; rolled back to ` and stderr reports the original failure. A generic daemon health failure is reported as `daemon did not become healthy after update`; migration failure is reported as `database migration failed after update: `.