From 6f0614ec74401de2e579173aa54ade0008151def Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 3 Sep 2026 01:10:06 -0500 Subject: [PATCH] fix(runner): restore providers before terminal cleanup --- .../runner-core/src/acpx_provider_backend.rs | 5 ++ .../src/managed_provider_backend.rs | 4 ++ .../src/native_provider_backend.rs | 5 ++ .../runner-core/src/provider_backend.rs | 5 ++ .../tests/native_provider_backend.rs | 46 ++++++++++++++++++- 5 files changed, 64 insertions(+), 1 deletion(-) diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs index 937ea2b69d..1492ab9206 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs @@ -1149,6 +1149,11 @@ impl CommandExecutor for AcpxCommandExecutor { } fn shutdown(&mut self) -> Result<(), DurableRunnerError> { + // A replacement durable runner may reach terminal reconciliation + // before any provider command or event poll. Restore the persisted + // session first so cleanup cannot succeed merely because this process + // has no in-memory session yet. + self.restore()?; if let Some(session) = self.session.as_mut() { session .shutdown("runner process shutdown") diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/managed_provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/managed_provider_backend.rs index 69ec37b5c7..5957b4c97d 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/managed_provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/managed_provider_backend.rs @@ -1696,6 +1696,10 @@ impl CommandExecutor for ManagedProviderCommandExecutor { } fn shutdown(&mut self) -> Result<(), DurableRunnerError> { + // A replacement runner has no live provider object until durable state + // is restored. Require that restoration before accepting terminal + // cleanup so a persisted remote session cannot be abandoned silently. + self.restore()?; if let Some(provider) = self.provider.as_mut() { provider.shutdown().map_err(|error| { DurableRunnerError::invalid(format!( diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs index 040bdea015..959b06c8d1 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs @@ -177,6 +177,11 @@ impl CommandExecutor for NativeProviderCommandExecutor { } fn shutdown(&mut self) -> Result<(), DurableRunnerError> { + // Terminal delivery can be reconciled by a replacement runner whose + // executor has not processed a provider command. Select the durable + // provider authority before cleanup so an absent in-memory selection + // can never turn the cleanup fence into a successful no-op. + self.select_recovery()?; if let Some(executor) = self.selected.as_mut() { executor.shutdown() } else { diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs index 4156973225..60fb12b77b 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs @@ -2810,6 +2810,11 @@ impl CommandExecutor for CodexCommandExecutor { } fn shutdown(&mut self) -> Result<(), DurableRunnerError> { + // Terminal-result recovery can invoke shutdown on a fresh executor. + // Loading the durable provider identity here ensures that cleanup is + // attempted against the persisted session instead of reporting a + // successful no-op from an empty in-memory provider slot. + self.restore()?; if let Some(provider) = self.provider.as_mut() { provider.shutdown().map_err(|error| { DurableRunnerError::invalid(format!("failed to stop Codex provider: {error}")) diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/native_provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/native_provider_backend.rs index 0ba6b87a27..f9ea97cc9d 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/native_provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/native_provider_backend.rs @@ -116,6 +116,14 @@ fn opencode_config(state_dir: &Path) -> DurableRunnerConfig { config } +fn opencode_call_count(state_dir: &Path, method: &str) -> usize { + fs::read_to_string(state_dir.join("fake-opencode-calls.log")) + .unwrap_or_default() + .lines() + .filter(|line| *line == method) + .count() +} + fn command(sequence: u64, command_type: &str, payload: Value) -> Command { Command { schema: "paperclip.prp.command.v1".to_owned(), @@ -424,6 +432,37 @@ fn executes_opencode_through_the_local_facade_without_codex_event_labels() { fs::remove_dir_all(directory).unwrap(); } +#[test] +fn replacement_shutdown_restores_the_persisted_provider_before_cleanup() { + let directory = temporary_directory("opencode-replacement-shutdown"); + let config = opencode_config(&directory); + let mut first = NativeProviderCommandExecutor::with_runner_config(&directory, &config); + + first + .execute(&command( + 1, + "run.prepare", + opencode_prepare_payload(&directory), + )) + .unwrap(); + first + .execute(&command(2, "session.open", json!({}))) + .unwrap(); + first.shutdown().unwrap(); + drop(first); + + let resumes_before_cleanup = opencode_call_count(&directory, "thread/resume"); + let mut replacement = NativeProviderCommandExecutor::with_runner_config(&directory, &config); + replacement.shutdown().unwrap(); + + assert_eq!( + opencode_call_count(&directory, "thread/resume"), + resumes_before_cleanup + 1, + "a replacement executor must restore the persisted provider before terminal cleanup", + ); + fs::remove_dir_all(directory).unwrap(); +} + #[test] fn rejects_a_mutable_opencode_command_outside_the_runner_launch_profile() { let directory = temporary_directory("opencode-command-override"); @@ -490,7 +529,12 @@ fn rejects_opencode_launch_profile_drift_across_fresh_recovery() { .contains("launch profile changed across durable recovery")); assert_eq!(fs::read(&state_path).unwrap(), state_before_recovery); - recovered.shutdown().unwrap(); + let shutdown_error = recovered + .shutdown() + .expect_err("invalid recovered launch authority also blocks cleanup"); + assert!(shutdown_error + .to_string() + .contains("launch profile changed across durable recovery")); fs::remove_dir_all(directory).unwrap(); }