fix(runner): restore providers before terminal cleanup
This commit is contained in:
parent
0485678d17
commit
6f0614ec74
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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!(
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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}"))
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue