diff --git a/DESIGN.md b/DESIGN.md index f3eb70fe..a26bd6e3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -423,9 +423,9 @@ Install and update commands submit daemon jobs, stream progress over the socket, `pv update` updates both the PV application and all installed Managed Resource tracks. Resource-specific update commands, such as `pv mysql:update`, update only installed tracks of that Managed Resource. -`pv update` self-updates the PV application before Managed Resources so the latest daemon owns resource update logic and manifest interpretation. It downloads the new `pv` binary, verifies its SHA-256 checksum and byte count, installs it under `~/.pv/bin/releases//pv`, atomically points `~/.pv/bin/pv` at the new release, coordinates daemon restart, reconnects to the daemon, releases the foreground update lock, and then runs a daemon-owned Managed Resource update job. If checksum verification, binary replacement, daemon restart, daemon health, or startup migration health fails, `pv update` stops and reports the failure instead of continuing to Managed Resource updates. +`pv update` self-updates the PV application before Managed Resources so the latest daemon owns resource update logic and manifest interpretation. It downloads the new `pv` binary, verifies its SHA-256 checksum and byte count, installs it under `~/.pv/bin/releases//pv`, atomically points `~/.pv/bin/pv` at the new release, coordinates daemon restart, reconnects to the daemon, releases the self-update/daemon-mutation coordination lock, and then runs a daemon-owned Managed Resource update job. If checksum verification, binary replacement, daemon restart, daemon health, or startup migration health fails, `pv update` stops and reports the failure instead of continuing to Managed Resource updates. -If the PV application is already current, `pv update` still continues to the daemon-owned Managed Resource update phase in the same foreground process after releasing the update lock. If a newer PV application release is activated successfully, `pv update` re-execs the active `~/.pv/bin/pv` through an internal hidden continuation path before submitting the Managed Resource update job. The continuation path skips the app-update phase, does not reprint the `PV update` header or app-phase lines, validates that it is running from the installed active release layout, and submits only the daemon-owned Managed Resource update phase. +If the PV application is already current, `pv update` still continues to the daemon-owned Managed Resource update phase in the same foreground process after releasing the coordination lock. If a newer PV application release is activated successfully, `pv update` re-execs the active `~/.pv/bin/pv` through an internal hidden continuation path before submitting the Managed Resource update job. The continuation path skips the app-update phase, does not reprint the `PV update` header or app-phase lines, validates that it is running from the installed active release layout, and submits only the daemon-owned Managed Resource update phase. `pv update` does not prompt for confirmation before applying available PV application or Managed Resource updates. Running the command is the explicit user intent to update. Safety comes from checksum verification, atomic release installation, rollback, and non-destructive data handling. @@ -435,7 +435,7 @@ If the PV application is already current, `pv update` still continues to the dae `pv update --check` requires the PV daemon to be running. If the daemon is not available, it fails with a clear message suggesting `pv daemon:restart` or `pv setup`. -`pv update --check` is read-only and does not take the self-update lock. If a real update is already in progress, it fails clearly instead of waiting or mutating state. +`pv update --check` is read-only and does not take the coordination lock. If a foreground self-update or daemon mutation owns the lock, it fails clearly instead of waiting or mutating state. `pv update --check --json` is supported in v1 and reports machine-readable PV application update availability plus installed Managed Resource track update availability. Non-check update progress does not need JSON output in v1. @@ -594,22 +594,22 @@ PV v1 relies on HTTPS/GitHub trust for the PV app update manifest itself. The ap For `pv update`, the CLI fetches the PV app update manifest and performs PV binary self-update before handing Managed Resource update work to the daemon. The daemon owns Managed Resource manifest refresh, install, update, and runtime reconciliation through a mutating `RunJob` request with `kind = "update"` and `scope = "system"`. -The mutating PV application phase runs in the foreground `pv update` process. The foreground process activates the new `~/.pv/bin/pv` symlink when needed, restarts or kickstarts the daemon, waits for daemon health, reports success or rollback, and releases the update lock before Managed Resource work begins. App-current continuation submits the daemon update job from the same process. App-updated continuation re-execs the active `~/.pv/bin/pv` into a hidden internal continuation path so the newly active CLI/protocol submits the daemon update job. +The mutating PV application phase runs in the foreground `pv update` process. The foreground process activates the new `~/.pv/bin/pv` symlink when needed, restarts or kickstarts the daemon, waits for daemon health, reports success or rollback, and releases the coordination lock before Managed Resource work begins. App-current continuation submits the daemon update job from the same process. App-updated continuation re-execs the active `~/.pv/bin/pv` into a hidden internal continuation path so the newly active CLI/protocol submits the daemon update job. The PV application update phase runs in this order: 1. Print `PV update`. -2. Acquire the OS update lock at `~/.pv/run/update.lock`. +2. Acquire the self-update/daemon-mutation coordination lock at `~/.pv/run/update.lock`. 3. Validate the installed active release symlink and running version. 4. Validate or normalize the PV-owned LaunchAgent to `~/.pv/bin/pv daemon:run`. 5. Fetch and parse the PV app update manifest. -6. If the manifest version is not newer than the running version, report current, release the update lock, and continue to the Managed Resource phase in the same process. +6. If the manifest version is not newer than the running version, report current, release the coordination lock, and continue to the Managed Resource phase in the same process. 7. Download, verify, install, and activate the newer app release. 8. Restart or kickstart the daemon without submitting `reconcile system`. 9. Wait for daemon health. -10. Release the update lock and re-exec the active `~/.pv/bin/pv` into the internal Managed Resource continuation. +10. Release the coordination lock and re-exec the active `~/.pv/bin/pv` into the internal Managed Resource continuation. -The update lock covers the network fetch, binary swap, and daemon transition. `pv update` does not enqueue `reconcile system` or `update system` while the update lock is held. The Managed Resource update job is submitted only after the foreground app phase releases the lock or after the re-execed continuation starts. +The coordination lock covers the network fetch, binary swap, and daemon transition. `pv update` does not enqueue `reconcile system` or `update system` while the coordination lock is held. The Managed Resource update job is submitted only after the foreground app phase releases the lock or after the re-execed continuation starts. If the PV application is already current, top-level `pv update` reports current and continues to the daemon-owned Managed Resource update phase without download, reinstall, reactivation, or daemon restart. @@ -647,7 +647,11 @@ Rollback is attempted only after the active binary has changed. If daemon startu If restoring the previous active symlink fails, stdout reports that rollback failed and stderr includes both the original failure and the symlink restore failure. If the previous symlink is restored but daemon restart or health after rollback fails, stdout reports that the previous app release was restored, stderr includes the original failure, says daemon restart after rollback failed, and suggests `pv daemon:restart` or `pv setup`. After the previous symlink is restored, PV attempts to remove the failed new release directory even if daemon restart after rollback fails; cleanup failure is a warning, not the primary result. -PV self-update holds an OS-level filesystem lock at `~/.pv/run/update.lock` for the binary swap and daemon transition. The foreground `pv update` process owns the lock, and both old and new daemon processes check the same lock before accepting mutating work. While the lock is held, concurrent mutating commands fail clearly with an update-in-progress message. The daemon rejects mutating requests while the lock is held; it does not queue them for later execution. Simple local read-only commands that do not require daemon protocol compatibility, such as `pv env`, may still run. Read-only commands that need daemon state, such as `pv status`, fail clearly during the transition. The lock file may remain on disk after the update; the active OS lock is released automatically when the owning process closes it or exits. +PV coordinates foreground self-update with queued daemon mutations through an OS-level advisory filesystem lock whose legacy path remains `~/.pv/run/update.lock`. The foreground `pv update` process holds it during the binary swap and daemon transition. The daemon mutation queue holds the same lock while reconciliation or update work is queued or running. This is not a universal serializer for every foreground PV mutation. + +Contention errors describe the active self-update/daemon-mutation coordination lock and its backing path. The persistent `update.lock` file is not itself evidence of contention: it may remain after the owning process exits, and an unlocked stale file does not block work. The active OS lock is released automatically when its owning file handle closes. Newly generated CLI, daemon, and job errors use this neutral coordination terminology. Previously persisted job summaries and errors remain historical data and are returned verbatim; PV does not migrate or retroactively normalize their wording. + +While the coordination lock is held, conflicting mutating commands fail clearly instead of waiting. The daemon rejects new mutating requests; it does not queue them behind a foreground self-update. Simple local read-only commands that do not require daemon protocol compatibility, such as `pv env`, may still run. Read-only commands that need daemon state, such as `pv status`, fail clearly during the transition. After a successful PV application self-update, PV keeps the current app release plus one previous app release under `~/.pv/bin/releases/`. Older app releases are pruned. Pruning never removes the active release or the previous rollback release. If pruning fails after a successful app update and healthy daemon restart, `pv update` exits zero and reports the cleanup failure as a warning on stderr. diff --git a/crates/cli/src/commands/mod.rs b/crates/cli/src/commands/mod.rs index 3857eefa..bee27b99 100644 --- a/crates/cli/src/commands/mod.rs +++ b/crates/cli/src/commands/mod.rs @@ -301,7 +301,7 @@ fn command_blocked_during_update(command: &Command) -> bool { fn update_lock_error(error: StateError) -> ExecuteError { match error { - StateError::UpdateInProgress { path } => CliError::UpdateInProgress { + StateError::CoordinationLockHeld { path } => CliError::CoordinationLockHeld { path: path.to_string(), } .into(), diff --git a/crates/cli/src/commands/update.rs b/crates/cli/src/commands/update.rs index 22c615c2..07060b56 100644 --- a/crates/cli/src/commands/update.rs +++ b/crates/cli/src/commands/update.rs @@ -354,7 +354,7 @@ fn launch_agent_is_already_unloaded(error: &platform::PlatformError) -> bool { fn update_state_error(error: StateError) -> ExecuteError { match error { - StateError::UpdateInProgress { path } => CliError::UpdateInProgress { + StateError::CoordinationLockHeld { path } => CliError::CoordinationLockHeld { path: path.to_string(), } .into(), diff --git a/crates/cli/src/error.rs b/crates/cli/src/error.rs index e975eef0..d3c07a51 100644 --- a/crates/cli/src/error.rs +++ b/crates/cli/src/error.rs @@ -60,8 +60,8 @@ pub enum CliError { #[error("no installed {resource} tracks were found; pass --track explicitly")] MissingLogResourceTrack { resource: String }, - #[error("PV update is already in progress; update lock is held at {path}")] - UpdateInProgress { path: String }, + #[error("PV self-update/daemon mutation coordination is active; the OS lock is held on {path}")] + CoordinationLockHeld { path: String }, #[error("PV application update requires an installed active release: {message}")] AppUpdateInvalidActiveRelease { message: String }, diff --git a/crates/cli/tests/snapshots/project_open__link_rejects_update_lock_without_recording_project.snap b/crates/cli/tests/snapshots/project_open__link_rejects_update_lock_without_recording_project.snap index b7af5426..a646b973 100644 --- a/crates/cli/tests/snapshots/project_open__link_rejects_update_lock_without_recording_project.snap +++ b/crates/cli/tests/snapshots/project_open__link_rejects_update_lock_without_recording_project.snap @@ -9,5 +9,5 @@ RunOutput { ), ), stdout: "", - stderr: "error: PV update is already in progress; update lock is held at /home/.pv/run/update.lock\n", + stderr: "error: PV self-update/daemon mutation coordination is active; the OS lock is held on /home/.pv/run/update.lock\n", } diff --git a/crates/cli/tests/snapshots/update__update_tests__internal_managed_resource_continuation_rejects_update_lock_before_daemon.snap b/crates/cli/tests/snapshots/update__update_tests__internal_managed_resource_continuation_rejects_update_lock_before_daemon.snap index 70e6eac3..25b31c56 100644 --- a/crates/cli/tests/snapshots/update__update_tests__internal_managed_resource_continuation_rejects_update_lock_before_daemon.snap +++ b/crates/cli/tests/snapshots/update__update_tests__internal_managed_resource_continuation_rejects_update_lock_before_daemon.snap @@ -9,5 +9,5 @@ RunOutput { ), ), stdout: "", - stderr: "error: PV update is already in progress; update lock is held at /run/update.lock\n", + stderr: "error: PV self-update/daemon mutation coordination is active; the OS lock is held on /run/update.lock\n", } diff --git a/crates/cli/tests/snapshots/update__update_tests__update_check_rejects_update_lock_before_daemon_or_manifest.snap b/crates/cli/tests/snapshots/update__update_tests__update_check_rejects_update_lock_before_daemon_or_manifest.snap index 70e6eac3..25b31c56 100644 --- a/crates/cli/tests/snapshots/update__update_tests__update_check_rejects_update_lock_before_daemon_or_manifest.snap +++ b/crates/cli/tests/snapshots/update__update_tests__update_check_rejects_update_lock_before_daemon_or_manifest.snap @@ -9,5 +9,5 @@ RunOutput { ), ), stdout: "", - stderr: "error: PV update is already in progress; update lock is held at /run/update.lock\n", + stderr: "error: PV self-update/daemon mutation coordination is active; the OS lock is held on /run/update.lock\n", } diff --git a/crates/cli/tests/snapshots/update__update_tests__update_rejects_concurrent_update_lock_before_fetching_manifest.snap b/crates/cli/tests/snapshots/update__update_tests__update_rejects_concurrent_update_lock_before_fetching_manifest.snap index 9481a2bd..80e9cfd6 100644 --- a/crates/cli/tests/snapshots/update__update_tests__update_rejects_concurrent_update_lock_before_fetching_manifest.snap +++ b/crates/cli/tests/snapshots/update__update_tests__update_rejects_concurrent_update_lock_before_fetching_manifest.snap @@ -9,5 +9,5 @@ RunOutput { ), ), stdout: "PV update\n", - stderr: "error: PV update is already in progress; update lock is held at /run/update.lock\n", + stderr: "error: PV self-update/daemon mutation coordination is active; the OS lock is held on /run/update.lock\n", } diff --git a/crates/cli/tests/update.rs b/crates/cli/tests/update.rs index ef82bf0c..a21e16ef 100644 --- a/crates/cli/tests/update.rs +++ b/crates/cli/tests/update.rs @@ -158,7 +158,7 @@ mod update_tests { if let Some(paths) = self.lock_probe.borrow().as_ref() { let probe = match state::UpdateLock::acquire(paths) { Ok(_lock) => "lock probe free".to_string(), - Err(state::StateError::UpdateInProgress { .. }) => { + Err(state::StateError::CoordinationLockHeld { .. }) => { "lock probe held".to_string() } Err(error) => format!("lock probe failed: {error}"), diff --git a/crates/daemon/src/jobs.rs b/crates/daemon/src/jobs.rs index 86b61d91..454f8436 100644 --- a/crates/daemon/src/jobs.rs +++ b/crates/daemon/src/jobs.rs @@ -149,7 +149,7 @@ async fn run_reconciliation_job( ) -> Result<(), DaemonError> { let result = match enqueue_reconciliation_job(&paths, &queue, scope) { Ok(result) => result, - Err(DaemonError::State(error @ StateError::UpdateInProgress { .. })) => { + Err(DaemonError::State(error @ StateError::CoordinationLockHeld { .. })) => { write_line(&mut transport, &DaemonResponse::error(error.to_string())).await?; return Ok(()); @@ -221,7 +221,7 @@ async fn run_update_job( ) -> Result<(), DaemonError> { let result = match enqueue_update_job(&paths, &queue) { Ok(result) => result, - Err(DaemonError::State(error @ StateError::UpdateInProgress { .. })) => { + Err(DaemonError::State(error @ StateError::CoordinationLockHeld { .. })) => { write_line(&mut transport, &DaemonResponse::error(error.to_string())).await?; return Ok(()); @@ -2278,7 +2278,7 @@ mod tests { assert!(matches!( result, - Err(crate::DaemonError::State(StateError::UpdateInProgress { path })) + Err(crate::DaemonError::State(StateError::CoordinationLockHeld { path })) if path == paths.update_lock() )); drop(update_lock); @@ -2312,7 +2312,7 @@ mod tests { assert!(matches!( update_lock, - Err(StateError::UpdateInProgress { path }) if path == paths.update_lock() + Err(StateError::CoordinationLockHeld { path }) if path == paths.update_lock() )); queued_task.abort(); diff --git a/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_mutating_jobs_but_keeps_health_available.snap b/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_mutating_jobs_but_keeps_health_available.snap index d0212477..95b2a9b0 100644 --- a/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_mutating_jobs_but_keeps_health_available.snap +++ b/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_mutating_jobs_but_keeps_health_available.snap @@ -5,7 +5,7 @@ expression: snapshot ( [ Object { - "message": String("PV update is already in progress; update lock is held at "), + "message": String("PV self-update/daemon mutation coordination is active; the OS lock is held on "), "protocol_version": Number(3), "status": String("error"), "type": String("response"), @@ -21,7 +21,7 @@ expression: snapshot ], [ Object { - "message": String("state error: PV update is already in progress; update lock is held at "), + "message": String("state error: PV self-update/daemon mutation coordination is active; the OS lock is held on "), "protocol_version": Number(3), "status": String("error"), "type": String("response"), diff --git a/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_update_jobs_before_manifest_refresh.snap b/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_update_jobs_before_manifest_refresh.snap index 8181b6e0..03ee94b3 100644 --- a/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_update_jobs_before_manifest_refresh.snap +++ b/crates/daemon/tests/snapshots/daemon_foundation__update_lock_rejects_update_jobs_before_manifest_refresh.snap @@ -5,7 +5,7 @@ expression: snapshot ( [ Object { - "message": String("PV update is already in progress; update lock is held at "), + "message": String("PV self-update/daemon mutation coordination is active; the OS lock is held on "), "protocol_version": Number(3), "status": String("error"), "type": String("response"), diff --git a/crates/state/src/error.rs b/crates/state/src/error.rs index 75ab8729..372c57cb 100644 --- a/crates/state/src/error.rs +++ b/crates/state/src/error.rs @@ -56,8 +56,8 @@ pub enum StateError { #[error("active PV binary symlink at {path} targets invalid release `{target}`")] InvalidAppReleasePointer { path: Utf8PathBuf, target: String }, - #[error("PV update is already in progress; update lock is held at {path}")] - UpdateInProgress { path: Utf8PathBuf }, + #[error("PV self-update/daemon mutation coordination is active; the OS lock is held on {path}")] + CoordinationLockHeld { path: Utf8PathBuf }, #[error("unsafe permissions for {path}: expected {expected:o}, found {actual:o}")] UnsafePermissions { diff --git a/crates/state/src/update_lock.rs b/crates/state/src/update_lock.rs index f0dc2099..46bf832e 100644 --- a/crates/state/src/update_lock.rs +++ b/crates/state/src/update_lock.rs @@ -10,7 +10,7 @@ use crate::{PvPaths, StateError, fs}; #[derive(Debug)] #[expect( clippy::disallowed_types, - reason = "update lock guard owns the OS-locked file handle" + reason = "coordination lock guard owns the OS-locked file handle" )] pub struct UpdateLock { _file: std::fs::File, @@ -65,7 +65,7 @@ fn lock_exclusively( match rustix::fs::flock(file, FlockOperation::NonBlockingLockExclusive) { Ok(()) => Ok(()), Err(error) if error.kind() == io::ErrorKind::WouldBlock => { - Err(StateError::UpdateInProgress { + Err(StateError::CoordinationLockHeld { path: path.to_path_buf(), }) } @@ -95,7 +95,7 @@ fn inspect_existing_lock( Ok(()) } Err(error) if error.kind() == io::ErrorKind::WouldBlock => { - Err(StateError::UpdateInProgress { + Err(StateError::CoordinationLockHeld { path: path.to_path_buf(), }) } @@ -120,7 +120,7 @@ fn require_file_locking() -> Result<(), StateError> { #[expect( clippy::disallowed_types, - reason = "update lock helper owns direct file handles for OS locking" + reason = "coordination lock helper owns direct file handles for OS locking" )] fn open_update_lock_file(path: &Utf8Path) -> Result { std::fs::OpenOptions::new() @@ -135,7 +135,7 @@ fn open_update_lock_file(path: &Utf8Path) -> Result { #[cfg(unix)] #[expect( clippy::disallowed_types, - reason = "update lock helper owns direct file handles for OS lock inspection" + reason = "coordination lock helper owns direct file handles for OS lock inspection" )] fn open_existing_update_lock_file(path: &Utf8Path) -> Result { std::fs::OpenOptions::new() @@ -224,7 +224,7 @@ mod tests { #[cfg(windows)] #[expect( clippy::disallowed_methods, - reason = "Windows update lock test creates an isolated existing-lock fixture" + reason = "Windows coordination lock test creates an isolated existing-lock fixture" )] fn create_lock_fixture(path: &Utf8Path) -> anyhow::Result<()> { if let Some(parent) = path.parent() { diff --git a/crates/state/tests/state_foundation.rs b/crates/state/tests/state_foundation.rs index 7c921e37..f3cf1683 100644 --- a/crates/state/tests/state_foundation.rs +++ b/crates/state/tests/state_foundation.rs @@ -274,7 +274,7 @@ fn update_lock_rejects_concurrent_holder_and_ignores_stale_file() -> Result<()> assert!(matches!( second, - Err(StateError::UpdateInProgress { path }) if path == paths.update_lock() + Err(StateError::CoordinationLockHeld { path }) if path == paths.update_lock() )); drop(first);