Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ The PV application update phase runs in this order:
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.
7. Download, verify, install, and activate the newer app release.
8. Restart or kickstart the daemon without submitting `reconcile system`.
8. Reload the validated PV-owned LaunchAgent with tolerant bootout/bootstrap, then 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.

Expand All @@ -625,7 +625,7 @@ User-facing `pv update` only performs PV application self-update when PV is runn

`pv update` is version-driven. If the manifest version equals or is lower than the running version, it reports current and does not download, reinstall, or reactivate the same version. Downgrades are out of v1 scope.

Installed and self-updating PV uses the stable active symlink as the LaunchAgent program path: `~/.pv/bin/pv daemon:run`. During update preflight, a PV-owned stale LaunchAgent plist is normalized to that stable path. When an update later needs to restart the daemon after stale plist normalization, PV reloads the launchd job with bootout/bootstrap before kickstart so launchd uses the normalized `ProgramArguments`. A missing LaunchAgent fails before activation with guidance to run `pv setup` or `pv daemon:enable`. A non-PV-owned LaunchAgent fails before activation and is left unchanged. If LaunchAgent normalization succeeds but the manifest says the PV application is current, `pv update` exits zero without restarting the daemon.
Installed and self-updating PV uses the stable active symlink as the LaunchAgent program path: `~/.pv/bin/pv daemon:run`. During update preflight, a PV-owned stale LaunchAgent plist is normalized to that stable path. After every actual app release activation, PV reloads the validated PV-owned launchd job with tolerant bootout/bootstrap before kickstart so launchd resolves the active symlink again and uses the current `ProgramArguments`. This reload is required whether the plist was already current or was normalized during preflight. If post-activation health fails, rollback restores the previous active symlink and repeats the same bootout/bootstrap/kickstart sequence before checking daemon health. A missing LaunchAgent fails before activation with guidance to run `pv setup` or `pv daemon:enable`. A non-PV-owned LaunchAgent fails before activation and is left unchanged. If LaunchAgent normalization succeeds but the manifest says the PV application is current, `pv update` exits zero without restarting or reloading the daemon.

PV app binary downloads use a freshly created command-scoped temporary file under `~/.pv/downloads/`; an existing path is never reused or truncated. While streaming the response, PV writes to that temporary file, computes SHA-256, and counts bytes. After the stream completes, PV verifies byte count and digest against the selected manifest asset. Verification failure deletes the temporary file, fails before app release installation, and leaves the active release unchanged. Successful installation uses the self-update release layout helper, then removes the temporary download; v1 does not introduce a persistent PV app binary download cache.

Expand Down
30 changes: 11 additions & 19 deletions crates/cli/src/commands/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,15 @@ fn validate_active_release(
fn normalize_launch_agent(
environment: &impl Environment,
paths: &PvPaths,
) -> Result<LaunchAgentReload, ExecuteError> {
) -> Result<Utf8PathBuf, ExecuteError> {
let expected = launch_agent_config(paths);
let path = launch_agent_path(environment)?;
match platform::inspect_launch_agent_file(&path, Some(&expected)) {
LaunchAgentFileState::Current { .. } => Ok(LaunchAgentReload::NotRequired),
LaunchAgentFileState::Current { path, .. } => Ok(path),
LaunchAgentFileState::Stale { .. } => {
platform::write_launch_agent_file(&path, &expected)?;

Ok(LaunchAgentReload::Required { path })
Ok(path)
}
LaunchAgentFileState::Missing { path } => Err(CliError::AppUpdateLaunchAgentMissing {
path: path.to_string(),
Expand All @@ -193,12 +193,6 @@ fn normalize_launch_agent(
}
}

#[derive(Clone, Debug)]
enum LaunchAgentReload {
NotRequired,
Required { path: Utf8PathBuf },
}

fn launch_agent_config(paths: &PvPaths) -> LaunchAgentConfig {
LaunchAgentConfig::new(
paths.active_pv_binary(),
Expand Down Expand Up @@ -287,13 +281,11 @@ fn remove_download(path: &Utf8Path) -> Result<(), ExecuteError> {
fn restart_daemon_without_reconciliation(
environment: &impl Environment,
paths: &PvPaths,
reload: &LaunchAgentReload,
launch_agent_path: &Utf8Path,
health_check: DaemonHealthCheck,
) -> Result<(), ExecuteError> {
if let LaunchAgentReload::Required { path } = reload {
bootout_launch_agent_if_loaded(environment)?;
environment.bootstrap_launch_agent(path)?;
}
bootout_launch_agent_if_loaded(environment)?;
environment.bootstrap_launch_agent(launch_agent_path)?;
clear_daemon_startup_failure_marker(paths)?;
environment.kickstart_launch_agent()?;
wait_until_daemon_started(paths.clone(), health_check)?;
Expand Down Expand Up @@ -484,7 +476,7 @@ fn run_app_update_phase(
let layout = state::AppReleaseLayout::new(paths.clone());
let current_version = AppUpdateVersion::current()?;
let previous_version = validate_active_release(&layout, &current_version)?;
let launch_agent_reload = normalize_launch_agent(environment, &paths)?;
let launch_agent_path = normalize_launch_agent(environment, &paths)?;

let manifest = fetch_app_update_manifest(environment)?;
if manifest.version() <= &current_version {
Expand Down Expand Up @@ -523,15 +515,15 @@ fn run_app_update_phase(
if let Err(error) = restart_daemon_without_reconciliation(
environment,
&paths,
&launch_agent_reload,
&launch_agent_path,
DaemonHealthCheck::AcceptProtocolMismatch,
) {
return rollback_app_update(
environment,
RollbackContext {
paths: &paths,
layout: &layout,
launch_agent_reload: &launch_agent_reload,
launch_agent_path: &launch_agent_path,
},
RollbackVersions {
previous: &previous_version,
Expand Down Expand Up @@ -566,7 +558,7 @@ struct RollbackVersions<'a> {
struct RollbackContext<'a> {
paths: &'a PvPaths,
layout: &'a state::AppReleaseLayout,
launch_agent_reload: &'a LaunchAgentReload,
launch_agent_path: &'a Utf8Path,
}

fn rollback_app_update(
Expand All @@ -592,7 +584,7 @@ fn rollback_app_update(
if let Err(rollback_error) = restart_daemon_without_reconciliation(
environment,
context.paths,
context.launch_agent_reload,
context.launch_agent_path,
DaemonHealthCheck::RequireCompatibleProtocol,
) {
output.line(&format!(
Expand Down
70 changes: 68 additions & 2 deletions crates/cli/tests/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ mod update_tests {
operations: RefCell<Vec<String>>,
execs: RefCell<Vec<(PathBuf, Vec<String>)>>,
exec_result: RefCell<Result<ExitCode, io::Error>>,
launch_agent_unloaded: bool,
delete_on_first_kickstart: RefCell<Option<Utf8PathBuf>>,
lock_probe: RefCell<Option<PvPaths>>,
startup_marker_on_first_kickstart: RefCell<Option<(Utf8PathBuf, String)>>,
Expand All @@ -47,6 +48,7 @@ mod update_tests {
operations: RefCell::new(Vec::new()),
execs: RefCell::new(Vec::new()),
exec_result: RefCell::new(Ok(ExitCode::SUCCESS)),
launch_agent_unloaded: false,
delete_on_first_kickstart: RefCell::new(None),
lock_probe: RefCell::new(None),
startup_marker_on_first_kickstart: RefCell::new(None),
Expand All @@ -66,6 +68,11 @@ mod update_tests {
self
}

fn with_unloaded_launch_agent(mut self) -> Self {
self.launch_agent_unloaded = true;
self
}

fn with_delete_on_first_kickstart(self, path: Utf8PathBuf) -> Self {
self.delete_on_first_kickstart.replace(Some(path));
self
Expand Down Expand Up @@ -148,6 +155,12 @@ mod update_tests {
.borrow_mut()
.push(format!("bootout {LAUNCH_AGENT_LABEL}"));

if self.launch_agent_unloaded {
return Err(platform::PlatformError::LaunchAgent(
"launch agent is not loaded".to_string(),
));
}

Ok(())
}

Expand Down Expand Up @@ -548,7 +561,11 @@ mod update_tests {
);
assert_eq!(
environment.operations(),
vec![format!("kickstart {LAUNCH_AGENT_LABEL}")]
vec![
format!("bootout {LAUNCH_AGENT_LABEL}"),
format!("bootstrap {}", launch_agent_path(&paths)),
format!("kickstart {LAUNCH_AGENT_LABEL}"),
]
);
assert_eq!(
environment.execs(),
Expand Down Expand Up @@ -694,6 +711,45 @@ mod update_tests {
Ok(())
}

#[test]
fn update_bootstraps_launch_agent_when_bootout_reports_already_unloaded() -> anyhow::Result<()>
{
let tempdir = tempdir()?;
let home = tempdir.path().join("home");
let paths = PvPaths::for_home(home.clone());
state::fs::ensure_layout(&paths)?;
let layout = install_current_release(&paths)?;
write_launch_agent(&paths, &paths.active_pv_binary())?;
let daemon = FakeDaemon::start(&paths, vec![health_response()])?;
let environment = TestEnvironment::new(
&home,
ScriptedClient::new()
.with_text(&app_manifest(
"0.3.0",
APP_BINARY_SHA256,
u64::try_from(APP_BINARY.len())?,
))
.with_download(APP_BINARY),
)
.with_unloaded_launch_agent();

let output = run_pv(&["update"], &environment)?;
let _daemon_requests = daemon.join()?;

assert_eq!(output.exit_code, ExitCode::SUCCESS);
assert_eq!(layout.active_release()?, Some("0.3.0".to_string()));
assert_eq!(
environment.operations(),
vec![
format!("bootout {LAUNCH_AGENT_LABEL}"),
format!("bootstrap {}", launch_agent_path(&paths)),
format!("kickstart {LAUNCH_AGENT_LABEL}"),
]
);

Ok(())
}

#[test]
fn update_holds_lock_until_daemon_transition_finishes() -> anyhow::Result<()> {
let tempdir = tempdir()?;
Expand Down Expand Up @@ -724,6 +780,8 @@ mod update_tests {
assert_eq!(
environment.operations(),
vec![
format!("bootout {LAUNCH_AGENT_LABEL}"),
format!("bootstrap {}", launch_agent_path(&paths)),
format!("kickstart {LAUNCH_AGENT_LABEL}"),
"lock probe held".to_string(),
]
Expand Down Expand Up @@ -1078,7 +1136,11 @@ mod update_tests {
assert_eq!(
environment.operations(),
vec![
format!("bootout {LAUNCH_AGENT_LABEL}"),
format!("bootstrap {}", launch_agent_path(&paths)),
format!("kickstart {LAUNCH_AGENT_LABEL}"),
format!("bootout {LAUNCH_AGENT_LABEL}"),
format!("bootstrap {}", launch_agent_path(&paths)),
format!("kickstart {LAUNCH_AGENT_LABEL}"),
]
);
Expand Down Expand Up @@ -1137,7 +1199,11 @@ mod update_tests {
assert_eq!(layout.active_release()?, Some("0.3.0".to_string()));
assert_eq!(
environment.operations(),
vec![format!("kickstart {LAUNCH_AGENT_LABEL}")]
vec![
format!("bootout {LAUNCH_AGENT_LABEL}"),
format!("bootstrap {}", launch_agent_path(&paths)),
format!("kickstart {LAUNCH_AGENT_LABEL}"),
]
);
assert_eq!(
daemon_requests,
Expand Down
Loading