diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs b/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs index 9bb1784012..d14fd2307d 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs @@ -560,6 +560,9 @@ fn run() -> Result<(), Box> { let require_completion_contract = args .iter() .any(|value| value == "--require-completion-contract"); + let require_external_sandbox = args + .iter() + .any(|value| value == "--require-external-sandbox"); let expected_canonical_task_context = argument(&args, "--expected-canonical-task-context") .map(|value| serde_json::from_str::(&value)) .transpose()?; @@ -574,6 +577,14 @@ fn run() -> Result<(), Box> { let emit_post_completion_warning = args .iter() .any(|value| value == "--emit-post-completion-warning"); + let emit_post_completion_passive_statuses = args + .iter() + .any(|value| value == "--emit-post-completion-passive-statuses"); + let emit_post_completion_foreign_turn = args + .iter() + .any(|value| value == "--emit-post-completion-foreign-turn"); + let post_completion_notification_gate = + argument(&args, "--post-completion-notification-gate").map(PathBuf::from); let fail_after_turn_completion = args .iter() .any(|value| value == "--fail-after-turn-completion"); @@ -818,6 +829,12 @@ fn run() -> Result<(), Box> { }))?, "initialized" => {} "thread/start" => { + if require_external_sandbox + && (message.pointer("/params/sandbox") != Some(&json!("danger-full-access")) + || message.pointer("/params/permissions").is_some()) + { + return Err("thread/start omitted the external sandbox boundary".into()); + } if require_dynamic_tool && !has_task_context_tool(&message) { return Err("thread/start omitted the authorized dynamic tool".into()); } @@ -845,6 +862,12 @@ fn run() -> Result<(), Box> { }))?; } "thread/resume" => { + if require_external_sandbox + && (message.pointer("/params/sandbox") != Some(&json!("danger-full-access")) + || message.pointer("/params/permissions").is_some()) + { + return Err("thread/resume omitted the external sandbox boundary".into()); + } if require_dynamic_tool && !has_task_context_tool(&message) { return Err("thread/resume omitted the authorized dynamic tool".into()); } @@ -900,6 +923,16 @@ fn run() -> Result<(), Box> { } } "turn/start" => { + if require_external_sandbox + && (message.pointer("/params/sandboxPolicy") + != Some(&json!({ + "type": "externalSandbox", + "networkAccess": "enabled", + })) + || message.pointer("/params/permissions").is_some()) + { + return Err("turn/start omitted the external sandbox boundary".into()); + } turn_start_count += 1; if durable_turn_ids { state.next_turn = state @@ -1187,6 +1220,60 @@ fn run() -> Result<(), Box> { "params": {"message": "provider remained live after terminal"} }))?; } + if emit_post_completion_passive_statuses { + for notification in [ + json!({ + "method": "remoteControl/status/changed", + "params": {"status": "disabled", "environmentId": null} + }), + json!({ + "method": "mcpServer/startupStatus/updated", + "params": {"name": "codex_apps", "status": "ready", "error": null} + }), + json!({ + "method": "account/rateLimits/updated", + "params": {"rateLimits": {}} + }), + json!({ + "method": "rawResponseItem/completed", + "params": {"item": {"id": "raw-tail", "type": "reasoning"}} + }), + json!({ + "method": "rawResponse/completed", + "params": {"response": {"id": "response-tail"}} + }), + json!({ + "method": "thread/goal/updated", + "params": {"threadId": state.thread_id, "goal": "finish the turn"} + }), + json!({ + "method": "thread/goal/cleared", + "params": {"threadId": state.thread_id} + }), + ] { + send(notification)?; + } + } + if emit_post_completion_foreign_turn { + if let Some(gate) = post_completion_notification_gate.as_ref() { + let deadline = std::time::Instant::now() + Duration::from_secs(5); + while !gate.is_file() { + if std::time::Instant::now() >= deadline { + return Err( + "post-completion notification gate timed out".into() + ); + } + thread::sleep(Duration::from_millis(1)); + } + } + send(json!({ + "method": "turn/started", + "params": {"threadId": state.thread_id, "turn": {"id": "unowned-turn"}} + }))?; + if let Some(gate) = post_completion_notification_gate.as_ref() { + fs::write(gate.with_extension("emitted"), b"emitted")?; + } + } if fail_after_turn_completion { if let Some(delay_ms) = fail_after_turn_completion_delay_ms { thread::sleep(Duration::from_millis(delay_ms)); diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs index d6fd0e4411..b4ec2944f0 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs @@ -28,6 +28,9 @@ const QUALIFIED_OPENCODE_VERSION: &str = "1.18.17"; const DEFAULT_PROVIDER_TRACE_MAX_BYTES: usize = 64 * 1024 * 1024; const MAX_BUFFERED_MESSAGES: usize = 1_024; const MAX_BUFFERED_MESSAGE_BYTES: usize = 16 * 1024 * 1024; +const WARM_ATTACHMENT_TAIL_DRAIN_LIMIT: usize = 256; +const WARM_ATTACHMENT_QUIET_WINDOW: Duration = Duration::from_millis(10); +const WARM_ATTACHMENT_DRAIN_DEADLINE: Duration = Duration::from_millis(100); const OPENCODE_PROVIDER_ENVIRONMENT_KEYS: &[&str] = &[ "OPENROUTER_API_KEY", "PAPERCLIP_NATIVE_MCP_NAME", @@ -282,6 +285,8 @@ pub struct CodexProviderConfig { pub instructions: String, #[serde(default = "default_approval_policy")] pub approval_policy: String, + #[serde(default)] + pub externally_sandboxed: bool, } impl CodexProviderConfig { @@ -304,6 +309,11 @@ impl CodexProviderConfig { "OpenCode providerVersion must equal the qualified {QUALIFIED_OPENCODE_VERSION} release", ))); } + if self.externally_sandboxed && self.provider != "codex" { + return Err(LocalRunnerError::invalid( + "external sandbox delegation is only supported by the Codex provider", + )); + } if self.command.as_os_str().is_empty() { return Err(LocalRunnerError::invalid("Codex command is required")); } @@ -522,6 +532,7 @@ pub struct CodexProvider { expected_shutdown: bool, process_generation: u64, completed_turn_authority: Option, + active_turn_result_authoritative: bool, completion_reconciliation_pending: bool, ambiguous_turn_start_pending: bool, settled_provider_turn_ids: SettledProviderTurnIds, @@ -677,7 +688,8 @@ impl CodexProvider { let authorized_tools = authorized_tools.into_iter().collect::>(); let permission_profile = codex_permission_profile( &config.provider, - std::env::var("PAPERCLIP_RUNNER_EXTERNAL_SANDBOX").as_deref() == Ok("1"), + config.externally_sandboxed + || std::env::var("PAPERCLIP_RUNNER_EXTERNAL_SANDBOX").as_deref() == Ok("1"), ); let (dynamic_tools, authorized_tool_ids) = codex_dynamic_tools(authorized_tools.iter().cloned())?; @@ -764,6 +776,7 @@ impl CodexProvider { expected_shutdown: false, process_generation, completed_turn_authority: None, + active_turn_result_authoritative: false, completion_reconciliation_pending: false, ambiguous_turn_start_pending: false, settled_provider_turn_ids: SettledProviderTurnIds::default(), @@ -800,7 +813,6 @@ impl CodexProvider { "cwd": config.cwd, "model": config.model, "approvalPolicy": config.approval_policy, - "permissions": provider.permission_profile, "runtimeWorkspaceRoots": [config.cwd], "baseInstructions": config.instructions, "dynamicTools": dynamic_tools, @@ -808,6 +820,14 @@ impl CodexProvider { let params_object = params .as_object_mut() .expect("Codex thread parameters are an object"); + if provider.permission_profile == "paperclip-runner-external-sandbox" { + // The execution target (for example Daytona) is the OS sandbox. + // Codex must not try to create nested user/network namespaces, + // which correctly fail inside an unprivileged container. + params_object.insert("sandbox".to_owned(), json!("danger-full-access")); + } else { + params_object.insert("permissions".to_owned(), json!(provider.permission_profile)); + } if config.provider == "opencode" { if let Some(contract) = provider.completion_contract.as_ref() { params_object.insert( @@ -926,17 +946,13 @@ impl CodexProvider { { return Ok(false); } - if self.process.try_wait()?.is_some() - || self.quarantined - || self.active_provider_turn_id.is_some() - || self.ambiguous_turn_start_pending - || !self.pending_messages.is_empty() - || !self.deferred_ambiguous_messages.is_empty() - || !self.pending_tool_requests.is_empty() - || !self.pending_runtime_requests.is_empty() - { + let blockers = self.warm_run_attachment_blockers(true)?; + if !blockers.is_empty() { return Err(LocalRunnerError::invalid( - "Codex warm run attachment requires an idle live provider with no pending work", + format!( + "Codex warm run attachment requires an idle live provider with no pending work ({})", + blockers.join(",") + ), )); } // The provider process and its thread remain authoritative. Exact @@ -945,11 +961,150 @@ impl CodexProvider { // tool or completion contract returns false so the caller can preserve // the existing cold-resume behavior for that incompatible boundary. self.completed_turn_authority = None; + self.active_turn_result_authoritative = false; self.completion_reconciliation_pending = false; self.expected_shutdown = false; Ok(true) } + fn drain_completed_turn_tail_for_warm_attachment(&mut self) -> Result<(), LocalRunnerError> { + let Some(completed_turn_id) = self + .completed_turn_authority + .as_ref() + .map(|authority| authority.provider_turn_id.clone()) + else { + return Ok(()); + }; + if self.active_provider_turn_id.is_some() { + return Ok(()); + } + + // Readiness probes run over the PRP command channel while provider + // stdout is drained by runnerd's adjacent control-loop iteration. A + // final usage/warning/item frame can therefore land after the last + // successful probe but before run.attach executes. Close that race in + // the same critical section as authority rotation. Only bounded tail + // notifications for the already-settled turn may be discarded: a new + // turn, provider request, process exit, or mismatched turn remains a + // fail-closed attachment error. + let deadline = std::time::Instant::now() + WARM_ATTACHMENT_DRAIN_DEADLINE; + let mut quiet_since: Option = None; + let mut drained = 0usize; + loop { + if std::time::Instant::now() >= deadline { + return Err(LocalRunnerError::invalid( + "Codex warm run attachment tail did not become quiescent", + )); + } + match self.poll()? { + Some(CodexProviderEvent::Notification { method, params }) => { + quiet_since = None; + drained = drained.saturating_add(1); + if drained > WARM_ATTACHMENT_TAIL_DRAIN_LIMIT { + return Err(LocalRunnerError::invalid( + "Codex warm run attachment tail exceeded its bounded frame limit", + )); + } + let names_other_turn = notification_turn_id(¶ms) + .is_some_and(|turn_id| turn_id != completed_turn_id); + let safe_tail_method = matches!( + method.as_str(), + "warning" + | "configWarning" + | "remoteControl/status/changed" + | "mcpServer/startupStatus/updated" + | "account/rateLimits/updated" + | "item/started" + | "item/completed" + | "item/agentMessage/delta" + | "rawResponseItem/completed" + | "rawResponse/completed" + | "thread/goal/updated" + | "thread/goal/cleared" + | "thread/tokenUsage/updated" + | "thread/status/changed" + | "turn/diff/updated" + | "turn/plan/updated" + ); + if names_other_turn || !safe_tail_method { + return Err(LocalRunnerError::invalid(format!( + "Codex warm run attachment observed unsafe post-terminal provider method {}", + bounded_method(&method) + ))); + } + if let Some(frame_id) = self.take_provider_trace_frame_id() { + self.record_provider_trace_interpretation( + frame_id, + "codex.warm_attachment.completed_turn_tail", + "ignored", + Vec::new(), + "Provider emitted a bounded tail notification after the prior turn terminal and before run attachment", + ); + } + } + Some(CodexProviderEvent::ToolCall { .. }) + | Some(CodexProviderEvent::RuntimeRequest { .. }) => { + return Err(LocalRunnerError::invalid( + "Codex warm run attachment observed a post-terminal provider request", + )); + } + Some(CodexProviderEvent::Exited { .. }) => { + return Err(LocalRunnerError::invalid( + "Codex exited while quiescing for warm run attachment", + )); + } + None => { + let now = std::time::Instant::now(); + let quiet_start = quiet_since.get_or_insert(now); + if now.duration_since(*quiet_start) >= WARM_ATTACHMENT_QUIET_WINDOW { + return Ok(()); + } + std::thread::sleep(Duration::from_millis(1)); + } + } + } + } + + pub(crate) fn warm_run_attachment_blockers( + &mut self, + quiesce_completed_tail: bool, + ) -> Result, LocalRunnerError> { + // Only an explicit attachment-readiness probe may consume the bounded, + // already-settled provider suffix. Ordinary checkpoint snapshots run + // immediately after a terminal frame while the provider can still be + // unwinding; turning those observations into quiescence barriers can + // quarantine an otherwise reusable runner before its next turn. + if quiesce_completed_tail { + self.drain_completed_turn_tail_for_warm_attachment()?; + } + let mut blockers = Vec::new(); + if self.process.try_wait()?.is_some() { + blockers.push("process_exited"); + } + if self.quarantined { + blockers.push("quarantined"); + } + if self.active_provider_turn_id.is_some() { + blockers.push("active_turn"); + } + if self.ambiguous_turn_start_pending { + blockers.push("ambiguous_turn_start"); + } + if !self.pending_messages.is_empty() { + blockers.push("pending_messages"); + } + if !self.deferred_ambiguous_messages.is_empty() { + blockers.push("deferred_messages"); + } + if !self.pending_tool_requests.is_empty() { + blockers.push("pending_tool_requests"); + } + if !self.pending_runtime_requests.is_empty() { + blockers.push("pending_runtime_requests"); + } + Ok(blockers) + } + pub(crate) fn restore_completed_turn_authority( &mut self, authoritative: bool, @@ -964,6 +1119,7 @@ impl CodexProvider { .unwrap_or("durable-completed-turn") .to_owned(), }); + self.active_turn_result_authoritative = false; if let Some(authority) = self.completed_turn_authority.as_ref() { self.settled_provider_turn_ids .restore(authority.provider_turn_id.clone())?; @@ -998,6 +1154,16 @@ impl CodexProvider { }) } + pub(crate) fn mark_active_turn_result_authoritative(&mut self) -> Result<(), LocalRunnerError> { + if self.active_provider_turn_id.is_none() || self.ambiguous_turn_start_pending { + return Err(LocalRunnerError::invalid( + "Codex semantic result cannot authorize a turn without exact active provider identity", + )); + } + self.active_turn_result_authoritative = true; + Ok(()) + } + pub(crate) fn take_rejected_accepted_turn(&mut self) -> Option { self.rejected_accepted_turn.take() } @@ -1111,16 +1277,24 @@ impl CodexProvider { let prior_buffered_message_count = self.pending_messages.len(); self.ambiguous_turn_start_pending = true; let runtime_request_scope = new_runtime_request_scope()?; - let result = match self.request_classified( - "turn/start", - json!({ - "threadId": self.thread_id, - "cwd": cwd, - "permissions": self.permission_profile, - "runtimeWorkspaceRoots": [cwd], - "input": [{"type": "text", "text": message, "text_elements": []}], - }), - ) { + let mut turn_params = json!({ + "threadId": self.thread_id, + "cwd": cwd, + "runtimeWorkspaceRoots": [cwd], + "input": [{"type": "text", "text": message, "text_elements": []}], + }); + let turn_params_object = turn_params + .as_object_mut() + .expect("Codex turn parameters are an object"); + if self.permission_profile == "paperclip-runner-external-sandbox" { + turn_params_object.insert( + "sandboxPolicy".to_owned(), + json!({"type": "externalSandbox", "networkAccess": "enabled"}), + ); + } else { + turn_params_object.insert("permissions".to_owned(), json!(self.permission_profile)); + } + let result = match self.request_classified("turn/start", turn_params) { Ok(result) => result, Err(ProviderRequestError::Rejected(error)) => { // A definite rejection proves no replacement work began. @@ -1216,6 +1390,7 @@ impl CodexProvider { self.ambiguous_turn_start_pending = false; self.expected_shutdown = false; self.completed_turn_authority = None; + self.active_turn_result_authoritative = false; self.completion_reconciliation_pending = false; self.completed_tool_call_ids.clear(); // Retain the prior settled identity while the next turn runs. Besides @@ -1814,14 +1989,16 @@ impl CodexProvider { .active_provider_turn_id .clone() .expect("active provider turn checked above"); - let completed_turn_authority = if terminal_event_type == "turn.completed" { - Some(CompletedTurnAuthority { - process_generation: self.process_generation, - provider_turn_id: provider_turn_id.clone(), - }) - } else { - None - }; + let result_authoritative = self.active_turn_result_authoritative; + let completed_turn_authority = + if terminal_event_type == "turn.completed" || result_authoritative { + Some(CompletedTurnAuthority { + process_generation: self.process_generation, + provider_turn_id: provider_turn_id.clone(), + }) + } else { + None + }; if !self .settled_provider_turn_ids .insert(provider_turn_id.clone()) @@ -1831,9 +2008,11 @@ impl CodexProvider { )); } self.active_provider_turn_id = None; + self.active_turn_result_authoritative = false; self.expected_shutdown = true; self.completed_turn_authority = completed_turn_authority; - self.completion_reconciliation_pending = terminal_event_type == "turn.completed"; + self.completion_reconciliation_pending = + terminal_event_type == "turn.completed" || result_authoritative; // The provider terminal is authoritative once received. Clear // local request ownership and attempt courtesy responses, but // a provider that already closed stdin must not turn the @@ -2927,6 +3106,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }; config.validate().unwrap(); config.provider_version = "1.18.18".to_owned(); diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs b/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs index f88dc0bbc2..6be7c85dbf 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs @@ -213,7 +213,7 @@ fn apply_authority_rotation( ) -> Result<(), DurableRunnerError> { let reconnect_count = state.reconnect_count.saturating_add(1); let mut diagnostics = std::mem::take(&mut state.diagnostics); - *endpoint = RunnerTransportEndpoint::new(&next.connect_url, &next.run_id)?; + endpoint.rotate(&next.connect_url, &next.run_id)?; *config = next; let mut rotated = DurableState::new(config); rotated.reconnect_count = reconnect_count; @@ -1208,6 +1208,40 @@ mod tests { fs::remove_dir_all(directory).unwrap(); } + #[test] + fn warm_run_attachment_reuses_the_provider_ingress_listener() { + let directory = std::env::temp_dir().join(format!( + "paperclip-runner-warm-listener-authority-{}", + std::process::id() + )); + let _ = fs::remove_dir_all(&directory); + let mut current = config(directory.clone()); + current.connect_url = "listen://0.0.0.0:43127/api/runner/v1/connect/run_1".to_owned(); + current.runner_digest = format!("sha256:{}", "a".repeat(64)); + let mut next = current.clone(); + next.run_id = "run_2".to_owned(); + next.turn_id = "turn_2".to_owned(); + next.item_id = "item_2".to_owned(); + next.connect_url = "listen://0.0.0.0:43127/api/runner/v1/connect/run_2".to_owned(); + + let store = DurableStateStore::new(&directory).unwrap(); + let (mut state, _) = store.load_or_create(¤t).unwrap(); + let mut endpoint = + RunnerTransportEndpoint::new(¤t.connect_url, ¤t.run_id).unwrap(); + + apply_authority_rotation(&mut state, &store, &mut current, &mut endpoint, next).unwrap(); + + assert_eq!(current.run_id, "run_2"); + assert_eq!(state.run_id, "run_2"); + match endpoint { + RunnerTransportEndpoint::Listen { path, .. } => { + assert_eq!(path, "/api/runner/v1/connect/run_2"); + } + RunnerTransportEndpoint::Dial(_) => panic!("listener mode must remain active"), + } + fs::remove_dir_all(directory).unwrap(); + } + #[test] fn terminal_lifecycle_is_durable_before_fallible_cleanup() { let directory = std::env::temp_dir().join(format!( diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs b/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs index ddbeec4a01..4cd4ad5895 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/durable/transport.rs @@ -215,6 +215,39 @@ impl RunnerTransportEndpoint { Ok(Self::Dial(ResolvedWsTarget::resolve(input)?)) } + /// Advance a run-bound transport endpoint without needlessly rebinding the + /// fixed provider-ingress listener. `run.attach` changes the WebSocket path + /// for the next run while retaining the same sandbox listener. Constructing + /// a second `TcpListener` before dropping the first one fails with + /// `EADDRINUSE`, which would terminate an otherwise healthy warm runner. + pub(crate) fn rotate(&mut self, input: &str, run_id: &str) -> Result<(), DurableRunnerError> { + if let Some(remainder) = input.strip_prefix("listen://") { + if let Self::Listen { path, .. } = self { + let (authority, next_path) = remainder.split_once('/').ok_or_else(|| { + DurableRunnerError::invalid( + "runner_ingress_bind_conflict: listener path is required", + ) + })?; + if authority != "0.0.0.0:43127" { + return Err(DurableRunnerError::invalid( + "runner_ingress_bind_conflict: listener must bind 0.0.0.0:43127", + )); + } + let next_path = format!("/{next_path}"); + validate_listener_path(&next_path)?; + if next_path != format!("/api/runner/v1/connect/{run_id}") { + return Err(DurableRunnerError::invalid( + "runner listener path does not match the configured run", + )); + } + *path = next_path; + return Ok(()); + } + } + *self = Self::new(input, run_id)?; + Ok(()) + } + fn open( &self, max_frame_bytes: usize, 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 c335667184..9b2aab8c63 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 @@ -260,6 +260,14 @@ fn validate_opencode_run_result( let result = params.get("result").cloned().ok_or_else(|| { DurableRunnerError::invalid("OpenCode paperclip/runResult omitted its result") })?; + let (fingerprint, disposition) = validate_run_result(state, &result)?; + Ok((result, fingerprint, disposition)) +} + +fn validate_run_result( + state: &CodexProviderState, + result: &Value, +) -> Result<(String, String), DurableRunnerError> { let schema: Value = serde_json::from_str(include_str!( "../../../../protocol/schemas/result.schema.json" )) @@ -267,9 +275,9 @@ fn validate_opencode_run_result( let validator = jsonschema::validator_for(&schema).map_err(|_| { DurableRunnerError::invalid("embedded Paperclip result schema cannot compile") })?; - if !validator.is_valid(&result) { + if !validator.is_valid(result) { return Err(DurableRunnerError::invalid( - "OpenCode paperclip/runResult failed the Paperclip result schema", + "provider semantic result failed the Paperclip result schema", )); } let contract = state.completion_contract.as_ref().ok_or_else(|| { @@ -322,8 +330,70 @@ fn validate_opencode_run_result( .and_then(Value::as_str) .expect("the validated result schema requires a disposition") .to_owned(); - let fingerprint = semantic_value_digest(&result); - Ok((result, fingerprint, disposition)) + let fingerprint = semantic_value_digest(result); + Ok((fingerprint, disposition)) +} + +fn admit_terminal_tool_authority( + state: &mut CodexProviderState, + operation_id: &str, + input: &Value, + result_is_error: bool, +) -> Result<(), DurableRunnerError> { + if result_is_error || !matches!(operation_id, "paperclip_finish" | "paperclip_block") { + return Ok(()); + } + // The correlated TypeScript semantic-tool handler validates the provider + // input against the operation schema, normalizes its defaults, and commits + // the accepted result before returning success. The bridge deliberately + // retains the original provider input, so validating that raw value against + // the stricter canonical result schema here would reject valid omitted + // defaults. Record the authenticated tool authority without trying to + // repeat the controller's normalization. + let reported_disposition = input + .get("reportedWorkDisposition") + .and_then(Value::as_str) + .ok_or_else(|| { + DurableRunnerError::invalid(format!("{operation_id} omitted its work disposition")) + })?; + let disposition = match reported_disposition { + "complete" | "completed" => "done", + other => other, + } + .to_owned(); + let fingerprint = semantic_value_digest(input); + let disposition_matches_operation = match operation_id { + "paperclip_finish" => matches!(disposition.as_str(), "done" | "needs_review"), + "paperclip_block" => disposition == "blocked", + _ => false, + }; + if !disposition_matches_operation { + return Err(DurableRunnerError::invalid(format!( + "{operation_id} supplied an incompatible work disposition" + ))); + } + match ( + state.active_provider_result_fingerprint.as_deref(), + state.active_provider_result_disposition.as_deref(), + ) { + (None, None) => { + // The TypeScript driver commits this exact tool input while + // servicing the correlated provider request. Retain only its + // digest and disposition here so runnerd does not synthesize a + // second, conflicting result when the provider turn terminates. + state.active_provider_result_fingerprint = Some(fingerprint); + state.active_provider_result_disposition = Some(disposition); + Ok(()) + } + (Some(existing_fingerprint), Some(existing_disposition)) + if existing_fingerprint == fingerprint && existing_disposition == disposition => + { + Ok(()) + } + _ => Err(DurableRunnerError::invalid( + "provider emitted conflicting terminal semantic tool results for one turn", + )), + } } fn normalize_provider_notification( @@ -366,7 +436,12 @@ fn terminal_events(state: &CodexProviderState, event_type: &str) -> Vec Result { + fn snapshot(&mut self, payload: &Value) -> Result { self.restore_provider_if_needed()?; + let quiesce_for_warm_attach = payload + .get("quiesceForWarmAttach") + .and_then(Value::as_bool) + .unwrap_or(false); + let mut warm_attach_blockers = self + .provider + .as_mut() + .map(|provider| provider.warm_run_attachment_blockers(quiesce_for_warm_attach)) + .transpose() + .map_err(|error| { + DurableRunnerError::invalid(format!( + "failed to inspect Codex warm attachment readiness: {error}" + )) + })? + .unwrap_or_else(|| vec!["provider_unavailable"]); let state = self .state .as_ref() .ok_or_else(|| DurableRunnerError::invalid("Codex provider is not prepared"))?; + if state.active_provider_turn_id.is_some() { + warm_attach_blockers.push("durable_active_turn"); + } + if state.ambiguous_turn_start_pending { + warm_attach_blockers.push("durable_ambiguous_turn_start"); + } + if !state.pending_events.is_empty() { + warm_attach_blockers.push("durable_pending_events"); + } + if !state.queued_events.is_empty() { + warm_attach_blockers.push("durable_queued_events"); + } + let warm_attach_ready = warm_attach_blockers.is_empty(); Ok(CommandExecution::result(json!({ "status": state.lifecycle, "provider": state.config.provider, @@ -2699,6 +2866,8 @@ impl CodexCommandExecutor { "sessionId": state.provider_session_id, "providerAccountSessionId": state.provider_session_id, "activeProviderTurnId": state.active_provider_turn_id, + "warmAttachReady": warm_attach_ready, + "warmAttachBlockers": warm_attach_blockers, "cwd": state.config.cwd, }))) } @@ -2762,15 +2931,21 @@ impl CodexCommandExecutor { }; let normalized_terminal_type = normalized_codex_terminal_event_type(&method, ¶ms); - let completed_turn_authority = - if normalized_terminal_type == Some("turn.completed") { - self.provider - .as_ref() - .and_then(CodexProvider::completed_turn_authority) - .map(|(generation, turn_id)| (generation, turn_id.to_owned())) - } else { - None - }; + let result_authoritative = normalized_terminal_type.is_some() + && self.state.as_ref().is_some_and(|state| { + state.active_provider_result_fingerprint.is_some() + }); + let completed_turn_authority = if normalized_terminal_type + == Some("turn.completed") + || result_authoritative + { + self.provider + .as_ref() + .and_then(CodexProvider::completed_turn_authority) + .map(|(generation, turn_id)| (generation, turn_id.to_owned())) + } else { + None + }; let terminal_event_type = normalized_terminal_type.map(str::to_owned); let identity = self.event_identity.clone(); let state = self @@ -2820,7 +2995,9 @@ impl CodexCommandExecutor { } } state.active_provider_turn_id = None; - if terminal_event_type.as_deref() == Some("turn.completed") { + if terminal_event_type.as_deref() == Some("turn.completed") + || result_authoritative + { let (process_generation, provider_turn_id) = completed_turn_authority .ok_or_else(|| { DurableRunnerError::invalid( @@ -3017,7 +3194,7 @@ impl CommandExecutor for CodexCommandExecutor { "turn.stop" => self.stop_turn_for_suspension(&command.command_type), "request.resolve" => self.resolve_request(&command.payload), "semantic_tool.result" => self.deliver_semantic_result(&command.payload), - "session.snapshot" => self.snapshot(), + "session.snapshot" => self.snapshot(&command.payload), "session.close" | "session.destroy" => self.close_session(), "runner.drain" | "runner.suspend" | "runner.shutdown" => { Ok(CommandExecution::result(json!({"status": "completed"}))) @@ -3101,6 +3278,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, Some(CompletionContractBinding { revision: "revision-1".to_owned(), @@ -3162,6 +3340,80 @@ mod tests { assert!(state.validate().is_ok()); } + #[test] + fn accepted_terminal_tool_suppresses_the_generated_terminal_fallback() { + let mut state = opencode_result_state(); + let mut result = valid_opencode_result(); + result["reportedWorkDisposition"] = json!("needs_review"); + result.as_object_mut().unwrap().remove("attentionRequests"); + result.as_object_mut().unwrap().remove("artifacts"); + + admit_terminal_tool_authority(&mut state, "paperclip_finish", &result, false).unwrap(); + let terminal = terminal_events(&state, "turn.completed"); + + assert_eq!(terminal.len(), 1); + assert_eq!(terminal[0].event_type, "run.terminal"); + assert_eq!( + terminal[0].payload["reportedWorkDisposition"], + "needs_review" + ); + assert!(state.validate().is_ok()); + } + + #[test] + fn accepted_terminal_tool_remains_successful_after_controller_interrupt() { + let mut state = opencode_result_state(); + let result = valid_opencode_result(); + + admit_terminal_tool_authority(&mut state, "paperclip_finish", &result, false).unwrap(); + let terminal = terminal_events(&state, "turn.interrupted"); + + assert_eq!(terminal.len(), 1); + assert_eq!(terminal[0].event_type, "run.terminal"); + assert_eq!(terminal[0].payload["runTerminalState"], "succeeded"); + assert_eq!(terminal[0].payload["turnTerminalState"], "completed"); + assert_eq!(terminal[0].payload["reportedWorkDisposition"], "done"); + assert!(state.validate().is_ok()); + } + + #[test] + fn codex_terminal_tool_authority_is_valid_durable_state() { + let mut state = opencode_result_state(); + state.config.provider = "codex".to_owned(); + state.config.driver = "codex_app_server".to_owned(); + state.config.provider_version = "test".to_owned(); + + admit_terminal_tool_authority( + &mut state, + "paperclip_finish", + &valid_opencode_result(), + false, + ) + .unwrap(); + + assert_eq!( + state.active_provider_result_disposition.as_deref(), + Some("done") + ); + assert!(state.validate().is_ok()); + } + + #[test] + fn terminal_tool_authority_rejects_an_incompatible_disposition() { + let mut state = opencode_result_state(); + + assert!(admit_terminal_tool_authority( + &mut state, + "paperclip_block", + &valid_opencode_result(), + false, + ) + .unwrap_err() + .to_string() + .contains("incompatible work disposition")); + assert!(state.active_provider_result_fingerprint.is_none()); + } + #[test] fn rejects_unbound_conflicting_or_spoofed_opencode_results() { let params = |result: Value| { @@ -3254,6 +3506,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, Some(CompletionContractBinding { revision: "revision-1".to_owned(), @@ -3296,6 +3549,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, opencode_launch_profile_digest: None, completion_contract: None, @@ -3342,6 +3596,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), @@ -3382,6 +3637,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, Some(CompletionContractBinding { revision: "1".to_owned(), @@ -3437,6 +3693,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), @@ -3501,6 +3758,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), @@ -3548,6 +3806,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), @@ -3669,6 +3928,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, bridge, @@ -3778,6 +4038,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, bridge, @@ -3832,6 +4093,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), @@ -3950,6 +4212,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, bridge, @@ -3988,6 +4251,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), @@ -4023,6 +4287,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), @@ -4097,6 +4362,7 @@ mod tests { provider_session_id: None, instructions: String::new(), approval_policy: "never".to_owned(), + externally_sandboxed: false, }, None, ProviderToolBridge::default(), diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs index 3b653b9902..91b80125fb 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs @@ -1,6 +1,7 @@ use std::fs; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Mutex, MutexGuard}; use paperclip_runner_core::codex_provider::{ CodexProvider, CodexProviderConfig, CodexProviderEvent, @@ -17,6 +18,13 @@ use paperclip_runner_core::provider_events::normalize_codex_notification; use serde_json::{json, Value}; static NEXT_DIRECTORY: AtomicU64 = AtomicU64::new(1); +static RECEIPT_LIMIT_TEST_LOCK: Mutex<()> = Mutex::new(()); + +fn lock_receipt_limit_test() -> MutexGuard<'static, ()> { + RECEIPT_LIMIT_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} fn temporary_directory(label: &str) -> PathBuf { let directory = std::env::temp_dir().join(format!( @@ -54,9 +62,25 @@ fn provider_config(directory: &Path, switches: &[&str]) -> CodexProviderConfig { provider_session_id: None, instructions: "Stay inside the test workspace.".to_owned(), approval_policy: "never".to_owned(), + externally_sandboxed: false, } } +#[test] +fn delegates_command_isolation_to_an_explicit_external_sandbox() { + let directory = temporary_directory("external-sandbox"); + let mut config = provider_config(&directory, &["--require-external-sandbox"]); + config.externally_sandboxed = true; + + let mut provider = CodexProvider::start(&config, None) + .expect("start Codex with an externally owned sandbox boundary"); + provider + .start_turn("Write the requested workspace file.", &config.cwd) + .expect("start the turn with the external sandbox policy"); + provider.shutdown().expect("stop fake Codex provider"); + fs::remove_dir_all(directory).expect("remove external sandbox test directory"); +} + #[test] fn provider_receives_the_isolated_codex_auth_home() { let directory = temporary_directory("isolated-auth-home"); @@ -3210,6 +3234,178 @@ fn durable_backend_rotates_tool_authority_for_fresh_run_attach() { fs::remove_dir_all(directory).expect("remove Codex integration-test directory"); } +#[test] +fn durable_backend_drains_a_bounded_completed_turn_tail_during_warm_attach() { + let directory = temporary_directory("durable-warm-attach-tail"); + let config = provider_config( + &directory, + &[ + "--durable-turn-ids", + "--emit-post-completion-warning", + "--emit-post-completion-passive-statuses", + ], + ); + let runner_config = durable_config(&directory); + let mut executor = CodexCommandExecutor::with_runner_config(&directory, &runner_config); + executor + .execute(&command( + "prepare", + 1, + "run.prepare", + json!({ + "provider": config, + "authorizedTools": task_context_tool_set(), + }), + )) + .expect("prepare the durable provider"); + executor + .execute(&command("open", 2, "session.open", json!({}))) + .expect("open the provider session"); + executor + .execute(&command( + "turn", + 3, + "turn.start", + json!({"text": "Complete before a late provider warning."}), + )) + .expect("start the first turn"); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + assert!( + std::time::Instant::now() < deadline, + "the first turn must settle before attachment" + ); + let events = poll_and_ack(&mut executor).expect("poll the first turn terminal"); + if events + .iter() + .any(|event| event.event_type == "turn.completed") + { + // Intentionally do not perform the usual final empty poll. The + // fake provider emitted a warning after its terminal, reproducing + // the readiness-probe/run.attach race from a real warm sandbox. + break; + } + std::thread::sleep(std::time::Duration::from_millis(1)); + } + + let readiness = executor + .execute(&command( + "readiness", + 4, + "session.snapshot", + json!({"quiesceForWarmAttach": true}), + )) + .expect("readiness probe drains only the completed turn tail"); + assert_eq!(readiness.result["warmAttachReady"], true); + assert_eq!(readiness.result["warmAttachBlockers"], json!([])); + + let attached = executor + .execute(&command( + "attach", + 5, + "run.attach", + json!({"authorizedTools": task_context_tool_set()}), + )) + .expect("warm attachment drains only the completed turn tail"); + assert!(attached + .events + .iter() + .any(|(event_type, _, _)| event_type == "run.attached")); + + executor.shutdown().expect("stop the warm provider"); + fs::remove_dir_all(directory).expect("remove Codex integration-test directory"); +} + +#[test] +fn durable_backend_rejects_new_work_in_a_completed_turn_tail() { + let directory = temporary_directory("durable-warm-attach-foreign-turn"); + let notification_gate = directory.join("emit-foreign-turn"); + let config = provider_config( + &directory, + &[ + "--durable-turn-ids", + "--emit-post-completion-foreign-turn", + "--post-completion-notification-gate", + notification_gate + .to_str() + .expect("notification gate path is UTF-8"), + ], + ); + let runner_config = durable_config(&directory); + let mut executor = CodexCommandExecutor::with_runner_config(&directory, &runner_config); + executor + .execute(&command( + "prepare", + 1, + "run.prepare", + json!({ + "provider": config, + "authorizedTools": task_context_tool_set(), + }), + )) + .expect("prepare the durable provider"); + executor + .execute(&command("open", 2, "session.open", json!({}))) + .expect("open the provider session"); + executor + .execute(&command( + "turn", + 3, + "turn.start", + json!({"text": "Complete before unowned work appears."}), + )) + .expect("start the first turn"); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + assert!( + std::time::Instant::now() < deadline, + "the first turn must settle before attachment" + ); + let events = poll_and_ack(&mut executor).expect("poll the first turn terminal"); + if events + .iter() + .any(|event| event.event_type == "turn.completed") + { + break; + } + std::thread::sleep(std::time::Duration::from_millis(1)); + } + fs::write(¬ification_gate, b"release").expect("release the foreign-turn barrier"); + let emitted_gate = notification_gate.with_extension("emitted"); + let emitted_deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !emitted_gate.is_file() { + assert!( + std::time::Instant::now() < emitted_deadline, + "the fake provider must acknowledge foreign-turn emission" + ); + std::thread::sleep(std::time::Duration::from_millis(1)); + } + + executor + .execute(&command("checkpoint", 4, "session.snapshot", json!({}))) + .expect("ordinary checkpoint snapshots must not consume provider tail frames"); + + let error = executor + .execute(&command( + "attach", + 5, + "run.attach", + json!({"authorizedTools": task_context_tool_set()}), + )) + .expect_err("warm attachment must not discard a new provider turn"); + assert!( + error + .to_string() + .contains("unsafe post-terminal provider method turn/started"), + "unexpected attachment error: {error}" + ); + + executor.shutdown().expect("stop the warm provider"); + fs::remove_dir_all(directory).expect("remove Codex integration-test directory"); +} + #[test] fn durable_backend_attaches_after_a_settled_restore_notice() { let directory = temporary_directory("durable-settled-attach"); @@ -3744,6 +3940,7 @@ fn provider_exit_preserves_and_reconciles_the_active_turn() { #[test] fn receipt_limit_rejects_the_call_and_keeps_polling_when_interrupt_fails() { + let _receipt_limit_test = lock_receipt_limit_test(); let directory = temporary_directory("receipt-limit-interrupt-failure"); let config = provider_config( &directory, @@ -3793,24 +3990,9 @@ fn receipt_limit_rejects_the_call_and_keeps_polling_when_interrupt_fails() { ); assert_eq!(call_count(&directory, "tool-response:failure"), 1); assert_eq!(call_count(&directory, "turn/interrupt"), 1); - - let mut settled = Vec::new(); - for _ in 0..4 { - settled.extend( - poll_and_ack(&mut recovered) - .expect("polling must autonomously retry the durable receipt-limit interrupt"), - ); - if settled - .iter() - .any(|event| event.event_type == "turn.interrupted") - { - break; - } - } - assert!( - settled - .iter() - .any(|event| event.event_type == "turn.interrupted"), + let interrupted = wait_for_executor_event(&mut recovered, "turn.interrupted"); + assert_eq!( + interrupted.payload["provider"], "codex", "the retry must settle the receipt-exhausted turn" ); assert_eq!(call_count(&directory, "turn/interrupt"), 3); @@ -3821,6 +4003,7 @@ fn receipt_limit_rejects_the_call_and_keeps_polling_when_interrupt_fails() { #[test] fn receipt_limit_retry_preserves_a_turn_settled_during_provider_recovery() { + let _receipt_limit_test = lock_receipt_limit_test(); let directory = temporary_directory("receipt-limit-recovered-settlement"); let config = provider_config( &directory, @@ -3913,6 +4096,7 @@ fn receipt_limit_retry_preserves_a_turn_settled_during_provider_recovery() { #[test] fn receipt_limit_accepts_a_terminal_after_the_initial_interrupt_deadline() { + let _receipt_limit_test = lock_receipt_limit_test(); let directory = temporary_directory("receipt-limit-delayed-terminal"); let config = provider_config( &directory, @@ -3989,6 +4173,7 @@ fn receipt_limit_accepts_a_terminal_after_the_initial_interrupt_deadline() { #[test] fn receipt_limit_polls_an_authoritative_terminal_with_unacknowledged_events() { + let _receipt_limit_test = lock_receipt_limit_test(); let directory = temporary_directory("receipt-limit-terminal-with-unacked-events"); let config = provider_config( &directory, @@ -4133,6 +4318,7 @@ fn pending_runtime_request_count_limit_rejects_the_overflowing_request() { #[test] fn receipt_limit_polling_bounds_and_rejects_runtime_request_floods() { + let _receipt_limit_test = lock_receipt_limit_test(); let directory = temporary_directory("receipt-limit-runtime-request-flood"); let config = provider_config( &directory, @@ -4202,6 +4388,7 @@ fn receipt_limit_polling_bounds_and_rejects_runtime_request_floods() { #[test] fn receipt_limit_synthesizes_interrupted_after_an_accepted_terminal_deadline() { + let _receipt_limit_test = lock_receipt_limit_test(); let directory = temporary_directory("receipt-limit-missing-terminal"); let config = provider_config( &directory, diff --git a/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts b/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts index f5e0c49447..d5a5fed36b 100644 --- a/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts +++ b/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.test.ts @@ -37,6 +37,90 @@ const identity: DurableRecoveryIdentity = { const expectedRunnerVersion = "0.3.0"; const expectedRunnerDigest = `sha256:${"a".repeat(64)}`; +it("persists the initial warm attachment seed idempotently and rejects replacement", () => { + const root = mkdtempSync( + resolve(tmpdir(), "runner-initial-attachment-seed-test-"), + ); + const template = { + provider: { + kind: "codex", + runId: identity.runId, + normalizedSessionId: identity.normalizedSessionId, + }, + authorizedTools: {}, + }; + try { + const core = new DurablePrpControlPlane({ + stateDirectory: root, + identity, + expectedRunnerVersion, + expectedRunnerDigest, + }); + core.persistRunAttachTemplate(template); + core.persistRunAttachTemplate(structuredClone(template)); + + expect( + JSON.parse( + readFileSync(resolve(root, "control-plane-state.json"), "utf8"), + ).runAttachTemplate, + ).toEqual(template); + expect(() => + core.persistRunAttachTemplate({ + ...template, + provider: { ...template.provider, kind: "opencode" }, + }), + ).toThrow("attachment template conflicts"); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +it("persists a connection-free warm attachment seed when rotating run identity", () => { + const root = mkdtempSync(resolve(tmpdir(), "runner-attachment-seed-test-")); + const nextIdentity: DurableRecoveryIdentity = { + ...identity, + runId: "00000000-0000-4000-8000-000000000002", + turnId: "turn-test-2", + itemId: "item-test-2", + }; + const template = { + provider: { + kind: "acpx", + runId: nextIdentity.runId, + normalizedSessionId: nextIdentity.normalizedSessionId, + }, + workspace: { cwd: "/workspace" }, + }; + try { + const core = new DurablePrpControlPlane({ + stateDirectory: root, + identity, + expectedRunnerVersion, + expectedRunnerDigest, + }); + core.rotateRunIdentity(nextIdentity, template); + + const stored = JSON.parse( + readFileSync(resolve(root, "control-plane-state.json"), "utf8"), + ) as Record; + expect(stored.identity).toEqual(nextIdentity); + expect(stored.commands).toEqual([]); + expect(stored.runAttachTemplate).toEqual(template); + + expect( + () => + new DurablePrpControlPlane({ + stateDirectory: root, + identity: nextIdentity, + expectedRunnerVersion, + expectedRunnerDigest, + }), + ).not.toThrow(); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + it.skipIf(process.platform === "win32")( "never persists raw child stdout or stderr as durable diagnostics", async () => { diff --git a/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.ts b/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.ts index 6d9a7e3e7e..ffdefe9e0d 100644 --- a/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.ts +++ b/packages/paperclip-runner/src/control-plane/durable-prp-control-plane.ts @@ -120,6 +120,13 @@ interface ConnectionLeaseRecord { interface StoredCoreState { schema: typeof coreStateSchema; identity: DurableRecoveryIdentity; + /** + * Connection-free provider attachment payload retained across authority + * epochs. Commands are intentionally reset when a reusable runner changes + * run identity, so the next controller cannot rely on command history to + * reconstruct another warm attachment. + */ + runAttachTemplate?: Record | null; tickets: Record; leases: Record; commands: DurableRecoveryCoreCommand[]; @@ -385,6 +392,13 @@ function isStoredCoreState( ) { return false; } + if ( + value.runAttachTemplate !== undefined && + value.runAttachTemplate !== null && + !isRecord(value.runAttachTemplate) + ) { + return false; + } if ( !commands.every( (command, index) => @@ -563,6 +577,7 @@ function initialCoreState(identity: DurableRecoveryIdentity): StoredCoreState { return { schema: coreStateSchema, identity, + runAttachTemplate: null, tickets: {}, leases: {}, commands: [], @@ -1050,7 +1065,10 @@ export class DurablePrpControlPlane { * retaining its existing connection lease secret. The runner performs the * matching state transition only after acknowledging `run.attach`. */ - rotateRunIdentity(identity: DurableRecoveryIdentity): void { + rotateRunIdentity( + identity: DurableRecoveryIdentity, + runAttachTemplate?: Record, + ): void { if ( !Object.values(identity).every( (value) => typeof value === "string" && stableIdPattern.test(value), @@ -1070,11 +1088,41 @@ export class DurablePrpControlPlane { { ...lease, identity: structuredClone(identity) }, ]), ); - Object.assign(this.#store.state, initialCoreState(identity), { leases }); + Object.assign(this.#store.state, initialCoreState(identity), { + leases, + runAttachTemplate: + runAttachTemplate === undefined + ? null + : structuredClone(runAttachTemplate), + }); this.#identity = structuredClone(identity); this.#store.save(); } + /** + * Retain the connection-free provider preparation payload before the first + * runner bootstrap. Completed command history is bounded and may be + * compacted before a warm continuation arrives, so it cannot be the sole + * source for a later run.attach. Repeating the same write is idempotent; + * changing an established seed fails closed. + */ + persistRunAttachTemplate(runAttachTemplate: Record): void { + if (!isRecord(runAttachTemplate.provider)) { + throw new Error("Durable PRP run attachment template is invalid."); + } + const existing = this.#store.state.runAttachTemplate; + if ( + existing !== undefined && + existing !== null && + canonicalJson(existing) !== canonicalJson(runAttachTemplate) + ) { + throw new Error("Durable PRP run attachment template conflicts."); + } + if (existing !== undefined && existing !== null) return; + this.#store.state.runAttachTemplate = structuredClone(runAttachTemplate); + this.#store.save(); + } + issueBootstrapTicket(ttlMs = 5_000): string { if (!Number.isInteger(ttlMs) || ttlMs < 1_000 || ttlMs > 60_000) { throw new Error("Durable PRP bootstrap TTL is invalid."); diff --git a/packages/paperclip-runner/src/drivers/codex/app-server-transport.ts b/packages/paperclip-runner/src/drivers/codex/app-server-transport.ts index e6a44fe932..7abae58cb0 100644 --- a/packages/paperclip-runner/src/drivers/codex/app-server-transport.ts +++ b/packages/paperclip-runner/src/drivers/codex/app-server-transport.ts @@ -46,7 +46,11 @@ export interface CodexAppServerTransport { turnId: string; resolution: HarnessRuntimeRequestResolution; }): Promise; - close(): Promise; + /** + * Close the provider transport. The optional reason is controller-owned + * diagnostic context; transports must not forward it to the provider. + */ + close(reason?: string): Promise; /** Relinquish controller authority while leaving durable runner work alive. */ detachControllerForRestart?(): Promise; processInfo?(): CodexTransportProcessInfo; diff --git a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.lifecycle.test.ts b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.lifecycle.test.ts index 0d3b6f0d25..4cdd48129f 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.lifecycle.test.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.lifecycle.test.ts @@ -92,7 +92,49 @@ class BlockingBootstrapTransport extends FakeCodexTransport { } } +class WarmAttachTransport extends FakeCodexTransport { + readonly attachments: Array<{ + runId: string; + turnId: string; + itemId: string; + }> = []; + + async attachRun(input: { + runId: string; + turnId: string; + itemId: string; + }): Promise { + this.attachments.push(structuredClone(input)); + } +} + describe("Codex app-server Codex driver", () => { + it("accepts runner-proven warm attachment when the host active-turn reducer is stale", async () => { + const transport = new WarmAttachTransport(); + const driver = makeDriver([transport]); + const session = await driver.openSession({ + runId: "run-warm-first", + normalizedSessionId: "normalized-warm", + workingDirectory: WORKSPACE, + }); + + await session.startTurn({ + message: { role: "user", text: "first turn" }, + }); + await expect( + session.attachRun?.({ runId: "run-warm-second" }), + ).resolves.toBeUndefined(); + expect(transport.attachments).toHaveLength(1); + expect((await session.snapshot()).activeTurnId).toBeNull(); + + await expect( + session.startTurn({ + message: { role: "user", text: "second turn" }, + }), + ).resolves.toMatchObject({ turnId: "turn-1" }); + await session.close({ reason: "test complete" }); + }); + it("does not create a transport for a pre-aborted session open", async () => { const transportFactory = vi.fn(() => new FakeCodexTransport()); const driver = makeDriver([], { transportFactory }); diff --git a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.semantic-results.test.ts b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.semantic-results.test.ts index 2885a30675..eefca9df37 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.semantic-results.test.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.semantic-results.test.ts @@ -42,6 +42,7 @@ import { type PrpEvent, type PrpStructuredRunResult, } from "./codex-app-server-driver.test-support.js"; +import { RUNNERD_CANONICAL_ITEM } from "./codex-driver-values.js"; describe("Codex app-server Codex driver", () => { it("makes duplicate semantic completion idempotent and rejects changed payloads", async () => { @@ -79,9 +80,7 @@ describe("Codex app-server Codex driver", () => { }); expect(resultMapping?.emittedEventIds).toHaveLength(2); expect(resultMapping?.emittedEventIds).toEqual( - expect.arrayContaining([ - expect.stringContaining(":run-result:"), - ]), + expect.arrayContaining([expect.stringContaining(":run-result:")]), ); expect( await transport.invoke({ @@ -129,12 +128,17 @@ describe("Codex app-server Codex driver", () => { workingDirectory: WORKSPACE, }); await session.startTurn({ message: { role: "user", text: "Complete." } }); - const toolShaped = structuredClone(result) as unknown as Record; - toolShaped.verification = [{ - commandOrCheck: "read hello.txt", - status: "passed", - result: "hello", - }]; + const toolShaped = structuredClone(result) as unknown as Record< + string, + unknown + >; + toolShaped.verification = [ + { + commandOrCheck: "read hello.txt", + status: "passed", + result: "hello", + }, + ]; delete toolShaped.attentionRequests; expect( await transport.invoke({ @@ -155,11 +159,13 @@ describe("Codex app-server Codex driver", () => { itemId: "tool-result", result: { ...result, - verification: [{ - commandOrCheck: "read hello.txt", - status: "passed", - detail: "hello", - }], + verification: [ + { + commandOrCheck: "read hello.txt", + status: "passed", + detail: "hello", + }, + ], }, }); transport.push("turn/completed", { @@ -283,6 +289,61 @@ describe("Codex app-server Codex driver", () => { expect((await session.snapshot()).semanticResult?.result).toEqual(result); }); + it("does not infer another result from runnerd's canonical activity item", async () => { + const transport = new FakeCodexTransport(); + const session = await makeDriver([transport]).openSession({ + runId: "run-runnerd-authoritative-result", + normalizedSessionId: "normalized-runnerd-authoritative-result", + workingDirectory: WORKSPACE, + }); + await session.startTurn({ message: { role: "user", text: "Complete." } }); + expect( + await transport.invoke({ + id: "tool-result", + method: "item/tool/call", + params: { + threadId: "thread-1", + turnId: "turn-1", + callId: "tool-result", + tool: "paperclip_finish", + arguments: result, + }, + }), + ).toMatchObject({ success: true }); + const changed = structuredClone(result); + changed.summary = "Schema-shaped post-tool assistant activity."; + transport.push("item/completed", { + threadId: "thread-1", + turnId: "turn-1", + item: { + [RUNNERD_CANONICAL_ITEM]: true, + id: "runnerd-message-result", + type: "agentMessage", + text: JSON.stringify(changed), + }, + }); + transport.push("turn/completed", { + threadId: "thread-1", + turn: { id: "turn-1", status: "completed", items: [] }, + }); + + const events = await collectUntilTerminal(session.events()); + expect( + events.filter((event) => event.eventType === "run.result.proposed"), + ).toHaveLength(1); + expect(events.some((event) => event.eventType === "session.failed")).toBe( + false, + ); + expect( + events.find( + (event) => + event.eventType === "item.completed" && + event.itemId === "runnerd-message-result", + )?.payload, + ).toMatchObject({ text: JSON.stringify(changed) }); + expect((await session.snapshot()).semanticResult?.result).toEqual(result); + }); + it("rejects a tool result that changes an agent-message commitment", async () => { const transport = new FakeCodexTransport(); const session = await makeDriver([transport]).openSession({ @@ -699,5 +760,4 @@ describe("Codex app-server Codex driver", () => { ?.payload, ).toMatchObject({ reportedWorkDisposition: "blocked" }); }); - }); diff --git a/packages/paperclip-runner/src/drivers/codex/codex-driver-values.ts b/packages/paperclip-runner/src/drivers/codex/codex-driver-values.ts index 4064019c6c..505ddc68cf 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-driver-values.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-driver-values.ts @@ -10,7 +10,10 @@ import { validatePrpStructuredRunResult, type PrpStructuredRunResult, } from "../../protocol/replay-contract.js"; -import { boundedCodexValue, isRetainableCodexPayload } from "./codex-boundaries.js"; +import { + boundedCodexValue, + isRetainableCodexPayload, +} from "./codex-boundaries.js"; export function record(value: unknown): Record { return typeof value === "object" && value !== null && !Array.isArray(value) @@ -18,6 +21,18 @@ export function record(value: unknown): Record { : {}; } +/** + * Marks an item reconstructed from runnerd's canonical PRP event stream. + * + * The symbol is intentionally process-local: a provider JSON payload cannot + * forge it. Runnerd has already selected the authoritative semantic result, + * so the compatibility Codex facade must preserve the item as activity + * without trying to infer a second result from its text. + */ +export const RUNNERD_CANONICAL_ITEM = Symbol( + "paperclip.runnerd.canonical-item", +); + export function text(value: unknown, fallback = ""): string { return typeof value === "string" ? value : fallback; } @@ -166,13 +181,19 @@ export function differingJsonPaths( if (limit <= 0) return []; if (Array.isArray(left) && Array.isArray(right)) { const paths: string[] = []; - for (let index = 0; index < Math.max(left.length, right.length); index += 1) { - paths.push(...differingJsonPaths( - left[index], - right[index], - `${prefix}[${index}]`, - limit - paths.length, - )); + for ( + let index = 0; + index < Math.max(left.length, right.length); + index += 1 + ) { + paths.push( + ...differingJsonPaths( + left[index], + right[index], + `${prefix}[${index}]`, + limit - paths.length, + ), + ); if (paths.length >= limit) break; } return paths.length > 0 ? paths : [prefix || "result"]; @@ -180,23 +201,26 @@ export function differingJsonPaths( const leftRecord = record(left); const rightRecord = record(right); if ( - (typeof left === "object" && left !== null) && - (typeof right === "object" && right !== null) && + typeof left === "object" && + left !== null && + typeof right === "object" && + right !== null && !Array.isArray(left) && !Array.isArray(right) ) { const paths: string[] = []; - const keys = [...new Set([ - ...Object.keys(leftRecord), - ...Object.keys(rightRecord), - ])].sort(); + const keys = [ + ...new Set([...Object.keys(leftRecord), ...Object.keys(rightRecord)]), + ].sort(); for (const key of keys) { - paths.push(...differingJsonPaths( - leftRecord[key], - rightRecord[key], - prefix ? `${prefix}.${key}` : key, - limit - paths.length, - )); + paths.push( + ...differingJsonPaths( + leftRecord[key], + rightRecord[key], + prefix ? `${prefix}.${key}` : key, + limit - paths.length, + ), + ); if (paths.length >= limit) break; } return paths.length > 0 ? paths : [prefix || "result"]; diff --git a/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts b/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts index 74d7ca9942..5d30ad21f5 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-harness-session.ts @@ -78,10 +78,11 @@ export class CodexHarnessSession } async attachRun(input: { runId: string }): Promise { + const transportOwnsQuiescence = this.transport.attachRun !== undefined; if ( - this.activeTurnId !== null || this.turnStartPending || - this.pendingRuntimeRequestMap.size > 0 + (!transportOwnsQuiescence && + (this.activeTurnId !== null || this.pendingRuntimeRequestMap.size > 0)) ) { throw new Error("codex_run_attach_busy"); } @@ -91,6 +92,16 @@ export class CodexHarnessSession turnId: `turn_attachment_${randomUUID().replaceAll("-", "")}`, itemId: `item_attachment_${randomUUID().replaceAll("-", "")}`, }); + if (transportOwnsQuiescence) { + // Runnerd's attachment contract performs two durable readiness probes, + // drains the settled provider tail, and rotates authority atomically. + // Its proof supersedes host reducer state that can remain stale when a + // semantic-result consumer stops before the interrupt terminal arrives. + // Drop only the prior run's already-proven-settled buffered suffix. + this.activeTurnId = null; + this.pendingRuntimeRequestMap.clear(); + this.eventQueue.clear(); + } this.runId = input.runId; this.result = null; this.resultFingerprint = null; @@ -684,10 +695,10 @@ export class CodexHarnessSession }; } - async close(): Promise { + async close(input?: { reason: string }): Promise { this.cancelPendingRequests("session_closed"); this.eventQueue.close(); - await this.transport.close(); + await this.transport.close(input?.reason); } async detachControllerForRestart(): Promise { diff --git a/packages/paperclip-runner/src/drivers/codex/codex-session-state.ts b/packages/paperclip-runner/src/drivers/codex/codex-session-state.ts index c623ca1c07..e3d2b5fd69 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-session-state.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-session-state.ts @@ -48,6 +48,10 @@ class AsyncQueue implements AsyncIterable { waiter({ value: undefined, done: true }); } + clear(): void { + this.#values = []; + } + [Symbol.asyncIterator](): AsyncIterator { return { next: async () => { @@ -340,7 +344,7 @@ export class CodexSessionState { } this.terminal = true; this.eventQueue.close(); - void this.transport.close(); + void this.transport.close(`protocol_failure:${code}`); } emit( diff --git a/packages/paperclip-runner/src/drivers/codex/codex-session-terminal.ts b/packages/paperclip-runner/src/drivers/codex/codex-session-terminal.ts index 23ad91499b..ef947dd626 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-session-terminal.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-session-terminal.ts @@ -2,33 +2,49 @@ import { HarnessReconciliationError } from "../../contracts/harness-driver.js"; import type { PrpStructuredRunResult } from "../../protocol/replay-contract.js"; import { boundedCodexPayload as boundedPayload, isRetainableCodexPayload } from "./codex-boundaries.js"; import type { CodexSessionState } from "./codex-session-state.js"; -import type { SemanticResultAdmission, TerminalReplayConflict } from "./codex-driver-types.js"; -import { canonicalJson, record, terminalState, text, tryParseResult } from "./codex-driver-values.js"; +import type { + SemanticResultAdmission, + TerminalReplayConflict, +} from "./codex-driver-types.js"; +import { + RUNNERD_CANONICAL_ITEM, + canonicalJson, + record, + terminalState, + text, + tryParseResult, +} from "./codex-driver-values.js"; export function captureResultFromItem( state: CodexSessionState, - item: Record, - turnId: string, - ): boolean { - if (state.conversationMode === "direct") return true; - if ( - text(item.type) !== "agentMessage" || - !isRetainableCodexPayload(item.text) - ) - return true; - const result = tryParseResult(item.text); - if (result !== null && isRetainableCodexPayload(result)) { - const admission = admitResult(state, result, text(item.id), turnId); - if (admission === "conflict") { - state.failProtocol( - "conflicting_semantic_result", - "Provider agentMessage supplied a different schema-valid semantic result after one was committed.", - ); - return false; - } - } + item: Record, + turnId: string, +): boolean { + if (state.conversationMode === "direct") return true; + // Runnerd is the semantic authority for its canonical PRP stream. Its + // normalized activity items can still contain schema-shaped assistant + // text after paperclip_finish, but re-parsing that text here would create + // a second, potentially conflicting result from one provider turn. + if ((item as Record)[RUNNERD_CANONICAL_ITEM] === true) return true; + if ( + text(item.type) !== "agentMessage" || + !isRetainableCodexPayload(item.text) + ) + return true; + const result = tryParseResult(item.text); + if (result !== null && isRetainableCodexPayload(result)) { + const admission = admitResult(state, result, text(item.id), turnId); + if (admission === "conflict") { + state.failProtocol( + "conflicting_semantic_result", + "Provider agentMessage supplied a different schema-valid semantic result after one was committed.", + ); + return false; + } } + return true; +} export function admitResult( state: CodexSessionState, diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts index 08815660d1..00ba33f701 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts @@ -34,11 +34,13 @@ import { codexSemanticToolSpecs, } from "../drivers/codex/codex-app-server-driver.js"; import { releaseMaterializedNativeRuntimeSkills } from "../drivers/runtime-context-materializer.js"; +import { RUNNERD_CANONICAL_ITEM } from "../drivers/codex/codex-driver-values.js"; import { authorizedToolSetForProvider, createCapabilityRunnerdCodexTransport, createCapabilityRunnerdProviderEnvironment, + createRunnerdCodexAppServerArgs, defaultCapabilityRunnerdBinary, expandRunnerdCanonicalNotifications, rehydrateRunnerdItemNotification, @@ -64,6 +66,70 @@ it("launches runnerd with its production durable outbox limits", () => { expect(runnerdLaunchProfileInternals.p0ReserveBytes).toBe(1024 * 1024); }); +it("carries the provider attachment seed across consecutive authority rotations", () => { + const baseIdentity = { + runnerInstanceId: "runner-warm-seed", + environmentLeaseId: "lease-warm-seed", + runId: "run-warm-one", + normalizedSessionId: "session-warm-seed", + turnId: "turn-warm-one", + itemId: "item-warm-one", + }; + const secondIdentity = { + ...baseIdentity, + runId: "run-warm-two", + turnId: "turn-warm-two", + itemId: "item-warm-two", + }; + const thirdIdentity = { + ...baseIdentity, + runId: "run-warm-three", + turnId: "turn-warm-three", + itemId: "item-warm-three", + }; + const secondTemplate = runnerdRecoveryInternals.rotatedRunAttachPayload( + { + commands: [ + { + type: "run.prepare", + payload: { + provider: { + kind: "acpx", + runId: baseIdentity.runId, + normalizedSessionId: baseIdentity.normalizedSessionId, + }, + workspace: { cwd: "/workspace" }, + }, + }, + ], + }, + secondIdentity, + null, + undefined, + ); + const thirdTemplate = runnerdRecoveryInternals.rotatedRunAttachPayload( + { commands: [], runAttachTemplate: secondTemplate }, + thirdIdentity, + null, + undefined, + ); + + expect(secondTemplate).toMatchObject({ + provider: { + runId: secondIdentity.runId, + normalizedSessionId: secondIdentity.normalizedSessionId, + }, + workspace: { cwd: "/workspace" }, + }); + expect(thirdTemplate).toMatchObject({ + provider: { + runId: thirdIdentity.runId, + normalizedSessionId: thirdIdentity.normalizedSessionId, + }, + workspace: { cwd: "/workspace" }, + }); +}); + it("replays the durable run attachment outcome and latest provider identity", () => { expect( runnerdRecoveryInternals.recoveredRunAttachment({ @@ -931,6 +997,27 @@ it("allows trusted package-manager runtime roots without exposing HOME paths", ( ).toEqual(["/opt/homebrew", "/usr/local"]); }); +it("denies the isolated Codex home without denying a remote execution workspace", () => { + const args = createRunnerdCodexAppServerArgs({ + environment: { + HOME: "/workspaces/task", + CODEX_HOME: "/workspaces/task/.codex", + PATH: "/usr/local/bin:/usr/bin:/bin", + }, + codexHome: + "/workspaces/task/.paperclip-runtime/paperclip-runner/sessions/session/filesystem/codex-home", + readOnlyRoots: ["/usr/local"], + }); + const serialized = args.join("\n"); + + expect(serialized).toContain( + '"/workspaces/task/.paperclip-runtime/paperclip-runner/sessions/session/filesystem/codex-home"="none"', + ); + expect(serialized).not.toContain('"/workspaces/task"="none"'); + expect(serialized).not.toContain('"/workspaces/task/.codex"="none"'); + expect(serialized).toContain('\":workspace_roots\"={\".\"=\"write\"}'); +}); + it("rejects remote OpenCode before spawn when provider-pack paths are absent", async () => { const root = await mkdtemp(join(tmpdir(), "paperclip-runner-remote-pack-")); const { transport } = createCapabilityRunnerdCodexTransport({ @@ -1031,6 +1118,7 @@ it("rehydrates a canonical agent item for the strict Codex facade", () => { threadId: "opened-thread-1", turnId: "provider-turn-1", item: { + [RUNNERD_CANONICAL_ITEM]: true, id: "message-1", type: "agentMessage", status: "completed", @@ -2154,6 +2242,139 @@ it("rotates PRP authority in place for a warm cross-run attachment", async () => } }, 30_000); +it("waits for a warm runner to re-authenticate before probing attachment readiness", async () => { + const stateDirectory = await mkdtemp( + join(tmpdir(), "runnerd-warm-reattach-before-probe-"), + ); + const server = createServer(); + const authorities = new Map(); + let blockFirstAuthorityReconnect = false; + let resolveRejectedReconnect!: () => void; + const rejectedReconnect = new Promise((resolvePromise) => { + resolveRejectedReconnect = resolvePromise; + }); + server.on("upgrade", (request, socket, head) => { + const route = request.url ?? ""; + if (route === "/runner-1" && blockFirstAuthorityReconnect) { + resolveRejectedReconnect(); + socket.destroy(); + return; + } + const authority = authorities.get(route); + if (!authority) { + socket.destroy(); + return; + } + authority.handleUpgrade(request, socket, route, head); + }); + await new Promise((resolveListen) => + server.listen(0, "127.0.0.1", resolveListen), + ); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Expected warm reconnect test listener"); + } + let registrationCount = 0; + const diagnostics: string[] = []; + const bundle = createCapabilityRunnerdCodexTransport({ + runnerBinary: defaultCapabilityRunnerdBinary(), + codexCommand: fakeCodex, + codexArgs: fakeCodexArgs(stateDirectory), + stateDirectory, + lifecyclePolicy: { mode: "warm", idleTimeoutMs: 60_000 }, + runnerReconnectGraceMs: 5_000, + onDiagnostic: (message) => diagnostics.push(message), + controlPlaneRegistration: async (authority) => { + registrationCount += 1; + const route = `/runner-${registrationCount}`; + authorities.set(route, authority); + return { + connectUrl: `ws://127.0.0.1:${address.port}${route}`, + release: () => { + if (authorities.get(route) === authority) authorities.delete(route); + }, + }; + }, + }); + bundle.transport.setServerRequestHandler(async () => ({ + success: true, + contentItems: [], + })); + let runnerPid: number | null = null; + try { + await bundle.transport.request("thread/start", { + cwd: tmpdir(), + dynamicTools: codexSemanticToolSpecs(), + }); + runnerPid = bundle.evidence().runnerPid; + const firstAuthority = authorities.get("/runner-1"); + if (!firstAuthority) throw new Error("Missing first warm authority"); + const priorSnapshotCount = firstAuthority.store.state.commands.filter( + (command) => command.type === "session.snapshot", + ).length; + + blockFirstAuthorityReconnect = true; + firstAuthority.disconnectActiveRunner(); + const attachment = bundle.transport.attachRun!({ + runId: "run-warm-after-reconnect", + turnId: "turn-warm-after-reconnect", + itemId: "item-warm-after-reconnect", + }); + await Promise.race([ + rejectedReconnect, + new Promise((_, reject) => + setTimeout( + () => reject(new Error("runner did not attempt to reconnect")), + 5_000, + ), + ), + ]); + + // No command may be queued while its sole authenticated consumer is + // absent. The generic 30-second command timeout used to turn this state + // into same-run recovery and replace the healthy warm runner process. + expect( + firstAuthority.store.state.commands.filter( + (command) => command.type === "session.snapshot", + ), + ).toHaveLength(priorSnapshotCount); + + blockFirstAuthorityReconnect = false; + await Promise.race([ + attachment, + new Promise((_, reject) => + setTimeout(() => reject(new Error("warm attachment timeout")), 10_000), + ), + ]); + expect(bundle.evidence()).toMatchObject({ + runnerPid, + runnerExited: false, + }); + expect(diagnostics).toContain( + "warm runner connection interrupted; waiting for re-authentication before authority rotation", + ); + expect(diagnostics).toContain( + "warm runner re-authenticated before authority rotation", + ); + } finally { + await bundle.transport.close().catch(() => undefined); + if (runnerPid) { + try { + process.kill(-runnerPid, "SIGKILL"); + } catch { + // A successful durable close already stopped the runner process group. + } + } + server.closeAllConnections(); + if (server.listening) { + await new Promise((resolveClose) => + server.close(() => resolveClose()), + ); + } + await rm(stateDirectory, { recursive: true, force: true }); + } +}, 30_000); + it("releases both PRP authorities when warm rotation activation fails", async () => { const stateDirectory = await mkdtemp( join(tmpdir(), "runnerd-warm-attach-activation-failure-"), diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index 9698fb766a..497185158c 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -66,6 +66,7 @@ import { prepareIsolatedCodexHome, releaseMaterializedNativeRuntimeSkills, } from "../drivers/runtime-context-materializer.js"; +import { RUNNERD_CANONICAL_ITEM } from "../drivers/codex/codex-driver-values.js"; // URL directory conversion preserves a trailing separator while path-derived // build artifacts do not. Normalize once so a source build cannot be @@ -402,7 +403,7 @@ async function rotateExternalAuthorityEpoch( } function rotatedRunAttachPayload( - state: { commands?: unknown }, + state: { commands?: unknown; runAttachTemplate?: unknown }, desired: DurableRecoveryIdentity, authorizedTools: Record | null, completionContract: @@ -411,17 +412,24 @@ function rotatedRunAttachPayload( const commands = Array.isArray(state.commands) ? state.commands.map(record) : []; - const seed = [...commands] + const persistedTemplate = + state.runAttachTemplate !== null && + typeof state.runAttachTemplate === "object" && + !Array.isArray(state.runAttachTemplate) + ? (state.runAttachTemplate as Record) + : null; + const commandSeed = [...commands] .reverse() .find( (command) => (command.type === "run.prepare" || command.type === "run.attach") && record(command.payload).provider !== undefined, ); - if (!seed) + const seed = persistedTemplate ?? record(commandSeed?.payload); + if (seed.provider === undefined) throw new Error("native_runner_authority_rotation_seed_unavailable"); return retargetRunAttachPayload( - record(seed.payload), + seed, desired, authorizedTools, completionContract, @@ -941,6 +949,12 @@ export interface CapabilityRunnerdCodexTransportOptions { runnerRuntimeContext?: NativeRuntimeContextSnapshot | null; /** Root path visible to runnerd when it is not on the Paperclip host. */ runnerFilesystemRoot?: string; + /** + * The provider process is already confined by a sandbox execution target. + * Codex must use its explicit external-sandbox policy because container + * runtimes such as Daytona intentionally omit nested namespace privileges. + */ + externallySandboxed?: boolean; /** Current run's authority catalog, used when a suspended session is rebound. */ resumeDynamicTools?: readonly Readonly>[]; /** Current run's completion authority, rebound without changing provider identity. */ @@ -1444,6 +1458,7 @@ export function rehydrateRunnerdItemNotification( turnId: activeTurnId, item: { ...rawItem, + [RUNNERD_CANONICAL_ITEM]: true, id: rawItem.id ?? rawParams.itemId, type: rawItem.type ?? rawParams.kind, status: rawItem.status ?? rawParams.status, @@ -1902,6 +1917,25 @@ export function trustedRuntimeReadOnlyRoots( return [...roots]; } +export function createRunnerdCodexAppServerArgs(input: { + environment: NodeJS.ProcessEnv | undefined; + codexHome: string; + readOnlyRoots?: string[]; +}): string[] { + // The filesystem policy denies HOME and CODEX_HOME to keep credentials and + // runner state outside provider reach. Always bind those names to the actual + // isolated runner home; a stale controller environment must never cause the + // execution workspace itself to become an explicit deny root. + return createIsolatedCodexAppServerArgs( + { + ...input.environment, + HOME: input.codexHome, + CODEX_HOME: input.codexHome, + }, + input.readOnlyRoots, + ); +} + function unwrapToolResponse(response: Record): { readonly __paperclipSemanticToolOutcome: true; readonly result: unknown; @@ -2238,6 +2272,82 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { this.#handler = handler; } + async #awaitWarmRunAttachmentReady(): Promise { + // Remote runner ingress already has a bounded reconnect budget. Reuse the + // same budget here so a transient tunnel reconnect cannot trip the shorter + // generic command timeout and replace an otherwise healthy warm runner. + const reconnectGraceMs = this.options.runnerReconnectGraceMs ?? 5_000; + const deadline = Date.now() + reconnectGraceMs; + let consecutiveReadyProbes = 0; + let lastBlockers: unknown = null; + while (Date.now() < deadline) { + await this.#awaitWarmRunnerConnection(deadline); + const snapshot = await this.#commandResult( + "session.snapshot", + { + quiesceForWarmAttach: true, + }, + deadline, + ); + lastBlockers = snapshot.warmAttachBlockers; + if (snapshot.warmAttachReady === true) { + consecutiveReadyProbes += 1; + // A second barrier prevents a provider frame emitted immediately after + // its terminal notification from racing the authority rotation. Each + // snapshot wakes runnerd, polls the provider, and drains the preceding + // durable event prefix before the next probe. + if (consecutiveReadyProbes >= 2) return; + } else { + consecutiveReadyProbes = 0; + } + await new Promise((resolveWait) => setTimeout(resolveWait, 25)); + } + throw new Error( + `native_runner_warm_attachment_not_quiescent: ${JSON.stringify(lastBlockers)}`, + ); + } + + async #awaitWarmRunnerConnection(deadline: number): Promise { + const core = this.#core; + if (core === null) throw new Error("native_runner_authority_unavailable"); + let reportedReconnectWait = false; + while (Date.now() < deadline) { + this.#throwIfFailed(); + const connectionCount = core.activeRunnerConnectionCount(); + if (connectionCount === 1) { + if (reportedReconnectWait) { + this.#diagnostic( + "warm runner re-authenticated before authority rotation", + ); + } + return; + } + if (connectionCount > 1) { + throw new Error( + `native_runner_warm_attachment_ambiguous: expected one authenticated runner, found ${connectionCount}`, + ); + } + if (!reportedReconnectWait) { + reportedReconnectWait = true; + this.#diagnostic( + "warm runner connection interrupted; waiting for re-authentication before authority rotation", + ); + } + if (await this.#runnerHasExited()) { + throw new Error( + "native_runner_warm_attachment_runner_exited: runner exited before authority rotation", + ); + } + await Promise.race([ + new Promise((resolveWait) => setTimeout(resolveWait, 25)), + this.#failureSignal, + ]); + } + throw new Error( + `provider_transport_failed: warm runner did not re-authenticate within ${this.options.runnerReconnectGraceMs ?? 5_000}ms`, + ); + } + async attachRun(input: { runId: string; turnId: string; @@ -2247,6 +2357,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { if (!core || !this.#startupComplete) { throw new Error("native_runner_prp_run_rotation_unavailable"); } + await this.#awaitWarmRunAttachmentReady(); const prior = core.store.state.identity; const desired: DurableRecoveryIdentity = { ...prior, @@ -2295,7 +2406,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { } const previousRelease = this.#controlPlaneRelease; - core.rotateRunIdentity(desired); + core.rotateRunIdentity(desired, runAttachTemplate); this.#eventSourceSeq = 0; this.#deferredTurnStartEvents = []; this.#durableTurnId = desired.turnId; @@ -2540,7 +2651,12 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { ); } - close(): Promise { + close(reason?: string): Promise { + if (reason) { + this.#diagnostic( + `runner transport close requested: ${reason.replaceAll(/[\r\n]/g, " ").slice(0, 1_000)}`, + ); + } this.#closePromise ??= this.#closeOnce(); return this.#closePromise; } @@ -2937,7 +3053,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { } } const completionContract = record(params.completionContract); - core.queueCommand("run.prepare", { + const runAttachTemplate = { authorizedTools: this.#authorizedTools, ...(completionContract.revision && Array.isArray(completionContract.criterionIds) @@ -3033,9 +3149,10 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { provider === "opencode" ? [opencodeProxyPath] : (this.options.codexArgs ?? - createIsolatedCodexAppServerArgs( - this.options.environment, - [ + createRunnerdCodexAppServerArgs({ + environment: this.options.environment, + codexHome, + readOnlyRoots: [ ...trustedRuntimeReadOnlyRoots( this.options.environment, ), @@ -3049,7 +3166,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { ] : []), ], - )), + })), cwd: String(params.cwd ?? tmpdir()), model: typeof params.model === "string" ? params.model : null, approvalPolicy: @@ -3057,6 +3174,9 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { params.approvalPolicy === "untrusted" ? params.approvalPolicy : "never", + externallySandboxed: + provider === "codex" && + this.options.externallySandboxed === true, instructions: provider === "codex" ? withCodexCollaborationRuntimeInstructions( @@ -3075,7 +3195,13 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { provider === "codex" && runtimeContext !== null, runtimeContext, }, - }); + }; + // Preserve the first generation's provider attachment seed independently + // of bounded command history. The in-memory copy serves a live warm + // continuation; the control-plane copy serves a controller/runner resume. + this.#runAttachTemplate = structuredClone(runAttachTemplate); + core.persistRunAttachTemplate(runAttachTemplate); + core.queueCommand("run.prepare", runAttachTemplate); core.queueCommand("session.open", { reuse: "same_session" }); const registration = this.options.controlPlaneRegistration ? await this.options.controlPlaneRegistration(core) @@ -3721,12 +3847,13 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { async #commandResult( type: string, payload: Record, + deadline?: number, ): Promise> { const core = this.#core; if (core === null) throw new Error("PRP provider thread is not started"); const commandId = `command_lab_${randomUUID().replaceAll("-", "")}`; core.queueCommand(type, payload, commandId, true); - await this.#waitCommand(type, commandId); + await this.#waitCommand(type, commandId, deadline); const command = core.store.state.commands.find( (candidate) => candidate.commandId === commandId, ); @@ -3759,8 +3886,11 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { throw new Error("runnerd did not report its provider identity"); } - async #waitCommand(type: string, commandId?: string): Promise { - const deadline = Date.now() + 30_000; + async #waitCommand( + type: string, + commandId?: string, + deadline = Date.now() + 30_000, + ): Promise { while (Date.now() < deadline) { this.#throwIfFailed(); const command = this.#core?.store.state.commands.find((candidate) => @@ -4589,6 +4719,7 @@ export const runnerdRecoveryInternals = Object.freeze({ providerTurnIsActiveFromCommittedEvents, recoveredRunAttachment, releaseRunnerProcessOwnership, + rotatedRunAttachPayload, rotateExternalAuthorityEpoch, turnStartCommandResultValid, turnStartNotificationDisposition, diff --git a/packages/paperclip-runner/src/native-session-runtime.test.ts b/packages/paperclip-runner/src/native-session-runtime.test.ts index 54e34896cd..21467b3321 100644 --- a/packages/paperclip-runner/src/native-session-runtime.test.ts +++ b/packages/paperclip-runner/src/native-session-runtime.test.ts @@ -2022,6 +2022,7 @@ describe("executeNativeSession recovery", () => { completeRun, }; const retainedSessions: Array = []; + const enrichmentFailures: Array<"checkpoint" | "usage"> = []; const execution = executeNativeSession({ input, @@ -2032,6 +2033,8 @@ describe("executeNativeSession recovery", () => { timeoutMs: 10, keepSessionOpen: true, onSession: (current) => retainedSessions.push(current), + onPostCompletionEnrichmentFailure: ({ stage }) => + enrichmentFailures.push(stage), }); await enrichmentStalled; if (stalledSignal !== undefined) @@ -2049,8 +2052,16 @@ describe("executeNativeSession recovery", () => { if (stalledSignal !== undefined) expect(stalledSignal.aborted).toBe(true); expect(completeRun).toHaveBeenCalledOnce(); - expect(close).toHaveBeenCalledOnce(); - expect(retainedSessions).toEqual([session, null]); + expect(enrichmentFailures).toEqual([ + stalledBoundary === "provider usage" ? "usage" : "checkpoint", + ]); + if (stalledBoundary === "provider usage") { + expect(close).not.toHaveBeenCalled(); + expect(retainedSessions).toEqual([session]); + } else { + expect(close).toHaveBeenCalledOnce(); + expect(retainedSessions).toEqual([session, null]); + } } finally { vi.useRealTimers(); } @@ -3086,17 +3097,29 @@ describe("executeNativeSession recovery", () => { ]); }); - it("finalizes a durable semantic result without waiting for a provider terminal", async () => { + it("retains a reusable session after its remote semantic-result cancellation settles", async () => { const lifecycle: string[] = []; + let releaseProvider = () => {}; + const providerReleased = new Promise((resolve) => { + releaseProvider = resolve; + }); const cancel = vi.fn(() => { lifecycle.push("cancelled"); - return { cleanup: Promise.resolve() }; + return { + cleanup: new Promise((resolve) => + setTimeout(() => { + releaseProvider(); + resolve(); + }, 150), + ), + }; }); const providerResult = vi.fn(async () => null); const close = vi.fn(async () => { lifecycle.push("closed"); }); const events: PrpEvent[] = []; + const retainedSessions: Array = []; const session: NativeSession = { identity: () => identity, async capabilities() { @@ -3110,7 +3133,7 @@ describe("executeNativeSession recovery", () => { }, async *events() { yield runnerEvent(1, "run.result.proposed", result); - await new Promise(() => undefined); + await providerReleased; }, async startTurn() { return { turnId: "turn-recovery" }; @@ -3182,6 +3205,8 @@ describe("executeNativeSession recovery", () => { runnerInstanceId: "runner-recovery", controlPlaneInstanceId: "control-recovery", semanticResultTerminalGraceMs: 0, + keepSessionOpen: true, + onSession: (current) => retainedSessions.push(current), }), ).resolves.toMatchObject({ result, terminal }); @@ -3195,7 +3220,101 @@ describe("executeNativeSession recovery", () => { "run.result.accepted", "run.terminal", ]); - expect(lifecycle).toEqual(["cancelled", "closed"]); + expect(lifecycle).toEqual(["cancelled"]); + expect(close).not.toHaveBeenCalled(); + expect(retainedSessions).toEqual([session]); + }); + + it("retains a reusable session while a semantic terminal releases its remote subscription", async () => { + const close = vi.fn(async () => undefined); + const retainedSessions: Array = []; + const session: NativeSession = { + identity: () => identity, + async capabilities() { + return { + resume: true, + typedEvents: true, + steering: false, + interruption: true, + structuredResult: true, + }; + }, + async *events() { + try { + yield runnerEvent(1, "run.result.proposed", result); + yield { + ...runnerEvent(2, "turn.completed"), + turnId: "turn-recovery", + }; + } finally { + await new Promise((resolve) => setTimeout(resolve, 150)); + } + }, + async startTurn() { + return { turnId: "turn-recovery" }; + }, + async result() { + return null; + }, + async snapshot() { + return { + backendKind: "mock", + sessionId: identity.sessionId, + identity, + providerSessionId: "provider-recovery", + cursor: "2", + activeTurnId: null, + pendingRuntimeRequests: [], + lineage: [], + }; + }, + close, + }; + const backend: NativeSessionBackend = { + async descriptor() { + return { + kind: "mock", + name: "semantic-terminal-subscription-backend", + version: "1", + capabilities: await session.capabilities(), + }; + }, + async openSession() { + return session; + }, + }; + const events: PrpEvent[] = []; + const port: ControlPlanePort = { + async openRun() {}, + async checkpointSession() {}, + async appendEvent(event) { + events.push(structuredClone(event as PrpEvent)); + return { + cursor: events.length, + highestContiguousSourceSeq: highestContiguous(events), + disposition: "committed", + }; + }, + async replayEvents() { + return { events: [], highestContiguousSourceSeq: 0 }; + }, + async completeRun() {}, + }; + + await expect( + executeNativeSession({ + input, + backend, + controlPlane: port, + runnerInstanceId: "runner-recovery", + controlPlaneInstanceId: "control-recovery", + keepSessionOpen: true, + onSession: (current) => retainedSessions.push(current), + }), + ).resolves.toMatchObject({ result, terminal }); + + expect(close).not.toHaveBeenCalled(); + expect(retainedSessions).toEqual([session]); }); it("retains provider output emitted after a durable semantic result", async () => { diff --git a/packages/paperclip-runner/src/native-session-runtime.ts b/packages/paperclip-runner/src/native-session-runtime.ts index 5a1c72da05..6de8d753a5 100644 --- a/packages/paperclip-runner/src/native-session-runtime.ts +++ b/packages/paperclip-runner/src/native-session-runtime.ts @@ -29,6 +29,12 @@ export const DEFAULT_NATIVE_RUNTIME_INPUT_LIVE_WINDOW_MS = 120_000; export const DEFAULT_NATIVE_SEMANTIC_RESULT_TERMINAL_GRACE_MS = 5_000; const OPTIONAL_SESSION_CANCELLATION_GRACE_MS = 100; const FAILED_OPERATION_SETTLEMENT_GRACE_MS = 100; +// Retaining a remote provider requires its semantic-result interruption to be +// acknowledged before the session is handed to another run. Daytona command +// round trips routinely exceed the generic failed-operation grace, but remain +// bounded by the transport. Other iterator and handoff cleanup keeps the short +// fail-closed quarantine boundary below. +const REUSABLE_SESSION_CANCELLATION_SETTLEMENT_GRACE_MS = 10_000; const DEFAULT_NATIVE_CHECKPOINT_TIMEOUT_MS = 30_000; type NativeSessionCleanupDomain = string; @@ -83,6 +89,8 @@ export interface ExecuteNativeSessionOptions { /** Internal test seam; production gives the provider five seconds to end after a result. */ semanticResultTerminalGraceMs?: number; onSession?: (session: NativeSession | null) => void; + /** Observes why a retained session was removed from warm reuse. */ + onSessionQuarantined?: (reason: string) => Promise | void; existingSession?: NativeSession; persistedSession?: PersistedNativeSession | null; keepSessionOpen?: boolean; @@ -96,6 +104,15 @@ export interface ExecuteNativeSessionOptions { snapshot: PersistedNativeSession, options?: CheckpointControlPlaneSessionOptions, ) => Promise | void; + /** + * Observes a post-result failure after completeRun has committed. Checkpoint + * failures quarantine the retained session; usage failures only omit + * optional accounting enrichment. + */ + onPostCompletionEnrichmentFailure?: (input: { + stage: "checkpoint" | "usage"; + error: unknown; + }) => Promise | void; /** Called when exact provider recovery failed and policy opened a new provider session. */ onContinuityBreak?: (input: { reason: string; @@ -686,8 +703,9 @@ async function consumeTurn( timeoutMs: number, runtimeInputLiveWindowMs: number, semanticResultTerminalGraceMs: number, + reusableSessionCancellationGraceMs: number, closeFailedSession: () => Promise, - quarantineSession: () => void, + quarantineSession: (reason: string) => void, resolveGovernedWait?: ExecuteNativeSessionOptions["resolveGovernedWait"], externalSignal?: AbortSignal, ) { @@ -697,6 +715,8 @@ async function consumeTurn( const appendAbort = new AbortController(); const governedCleanupOperations = new Set>(); let governedCancellationCommitted = false; + let semanticCancellationCommitted = false; + let semanticResultObserved = false; let deferredGovernedCleanupSettlement: Promise | null = null; let deferredSessionCancellationSettlement: Promise | null = null; const inputTimers = new Map>(); @@ -755,10 +775,11 @@ async function consumeTurn( signal: appendAbort.signal, }); governedCancellationCommitted = true; + semanticCancellationCommitted = event.eventType === "run.result.proposed"; const cleanup = cancellation.cleanup; governedCleanupOperations.add(cleanup); void cleanup - .catch(() => quarantineSession()) + .catch(() => quarantineSession("governed_cleanup_failed")) .finally(() => governedCleanupOperations.delete(cleanup)); return { event, @@ -904,6 +925,7 @@ async function consumeTurn( governedResult = validation.result; resultSource = "semantic_result"; semanticResultEvent = event; + semanticResultObserved = true; if (session.cancel !== undefined) { semanticResultDeadline = new Promise((resolve) => { semanticResultTimer = setTimeout( @@ -1059,14 +1081,21 @@ async function consumeTurn( ); } else { // Iterator and provider cleanup own no control-plane mutation authority. - // A slow subscription or cleanup remains observed and is released by the - // normal session close, but it cannot erase an already committed - // terminal fact or prevent result retrieval and durable finalization. + // A reusable session with a semantic result needs a longer bounded + // window for the remote event subscription to release. This applies + // both when Paperclip forced an interrupt and when the provider emitted + // its own terminal immediately afterward: the latter still crosses the + // remote PRP acknowledgement boundary and routinely takes longer than + // the generic local cleanup grace. Governed waits and unrelated stalled + // cleanup retain the short fail-closed boundary. const teardownSettled = await settlesWithin( passiveTeardownSettlement, - FAILED_OPERATION_SETTLEMENT_GRACE_MS, + semanticCancellationCommitted || semanticResultObserved + ? reusableSessionCancellationGraceMs + : FAILED_OPERATION_SETTLEMENT_GRACE_MS, ); - if (!teardownSettled) quarantineSession(); + if (!teardownSettled) + quarantineSession("provider_event_teardown_timed_out"); } if (timer !== undefined) clearTimeout(timer); if (semanticResultTimer !== undefined) clearTimeout(semanticResultTimer); @@ -1630,9 +1659,14 @@ export async function executeNativeSession( } let sessionClosePromise: Promise | null = null; let sessionQuarantined = false; - const quarantineSession = () => { + const quarantineSession = (reason: string) => { if (sessionQuarantined) return; sessionQuarantined = true; + try { + void options.onSessionQuarantined?.(reason); + } catch { + // Diagnostics cannot prevent provider cleanup. + } try { options.onSession?.(null); } catch { @@ -1643,7 +1677,7 @@ export async function executeNativeSession( let sessionCloseRecoveryPromise: Promise | null = null; const retainFailedCleanup = (cleanup: Promise) => { failedCleanupDeferred = true; - quarantineSession(); + quarantineSession("session_close_recovery_retained"); retainFailedSessionCleanupOwner(cleanup, cleanupDomain); }; const startSessionClose = (reason: string) => { @@ -1651,9 +1685,9 @@ export async function executeNativeSession( sessionClosePromise = attempt; return attempt; }; - const closeSession = () => { + const closeSession = (quarantineReason = "session_close_started") => { if (sessionClosePromise === null) { - quarantineSession(); + quarantineSession(quarantineReason); const firstAttempt = startSessionClose( "native session execution complete", ); @@ -1851,6 +1885,9 @@ export async function executeNativeSession( DEFAULT_NATIVE_RUNTIME_INPUT_LIVE_WINDOW_MS, options.semanticResultTerminalGraceMs ?? DEFAULT_NATIVE_SEMANTIC_RESULT_TERMINAL_GRACE_MS, + options.keepSessionOpen + ? REUSABLE_SESSION_CANCELLATION_SETTLEMENT_GRACE_MS + : FAILED_OPERATION_SETTLEMENT_GRACE_MS, closeSession, quarantineSession, options.resolveGovernedWait, @@ -2121,13 +2158,19 @@ export async function executeNativeSession( }; }, }); - let enrichment: { - providerSessionId: string | null; - driverVersion: string; - usage: Record | null; + const observeEnrichmentFailure = async ( + stage: "checkpoint" | "usage", + error: unknown, + ) => { + try { + await options.onPostCompletionEnrichmentFailure?.({ stage, error }); + } catch { + // Diagnostics cannot revoke or delay a durably committed result. + } }; + let completedSnapshot: PersistedNativeSession; try { - enrichment = await finalizeWithin({ + completedSnapshot = await finalizeWithin({ timeoutMs: options.timeoutMs ?? 900_000, operation: async (signal) => { const snapshot = await session.snapshot({ signal }); @@ -2139,36 +2182,61 @@ export async function executeNativeSession( }; await persistCheckpoint(completedSnapshot, signal); signal.throwIfAborted(); - const usage = (await session.usage?.()) ?? null; - signal.throwIfAborted(); - return { - providerSessionId: snapshot.providerSessionId ?? null, - driverVersion: - typeof usage?.driverVersion === "string" - ? usage.driverVersion - : descriptor.version, - usage, - }; + return completedSnapshot; }, }); - } catch { - // completeRun is the durable commit boundary. Snapshot/checkpoint/usage - // enrichment cannot revoke that success, but a session whose final - // checkpoint is unknown must not remain available for reuse. - void closeSession().catch(() => undefined); - enrichment = { + } catch (error) { + await observeEnrichmentFailure("checkpoint", error); + // completeRun is the durable commit boundary. A final snapshot or + // checkpoint failure cannot revoke that success, but a session whose + // exact checkpoint is unknown must not remain available for reuse. + void closeSession("post_completion_checkpoint_failed").catch( + () => undefined, + ); + executionSucceeded = true; + return { + ...durableExecutionResult, providerSessionId: recoveredSnapshot.providerSessionId ?? null, driverVersion: descriptor.version, usage: null, }; } + + let usage: Record | null = null; + try { + usage = await finalizeWithin({ + timeoutMs: options.timeoutMs ?? 900_000, + operation: async (signal) => { + const current = (await session.usage?.()) ?? null; + signal.throwIfAborted(); + return current; + }, + }); + } catch (error) { + // Usage is optional accounting enrichment. Once the exact completed + // checkpoint is durable, an unavailable usage capability must not tear + // down an otherwise reusable warm provider session. + await observeEnrichmentFailure("usage", error); + } + const enrichment = { + providerSessionId: completedSnapshot.providerSessionId ?? null, + driverVersion: + typeof usage?.driverVersion === "string" + ? usage.driverVersion + : descriptor.version, + usage, + }; executionSucceeded = true; return { ...durableExecutionResult, ...enrichment }; } finally { const shouldClose = !options.keepSessionOpen || !executionSucceeded || sessionQuarantined; if (shouldClose && options.requireSessionCloseBeforeReturn) { - if (!failedCleanupDeferred) closeSession(); + if (!failedCleanupDeferred) { + closeSession( + `execution_finally_close:keep=${options.keepSessionOpen === true}:succeeded=${executionSucceeded}:quarantined=${sessionQuarantined}`, + ); + } // A remote runner close owns its suspension and verified checkpoint. // Its implementation is finite, and the host must not release the // environment until the complete close/retry owner has settled. diff --git a/server/src/__tests__/environment-routes.test.ts b/server/src/__tests__/environment-routes.test.ts index eca4e23086..773ce672a0 100644 --- a/server/src/__tests__/environment-routes.test.ts +++ b/server/src/__tests__/environment-routes.test.ts @@ -29,6 +29,9 @@ const mockProjectService = vi.hoisted(() => ({ const mockEnvironmentRuntimeService = vi.hoisted(() => ({ destroyReusableSandboxLeasesForEnvironment: vi.fn(async () => ({ destroyed: 0, failed: 0, skippedLiveRun: 0 })), })); +const mockCloseWarmNativeSessionsForEnvironment = vi.hoisted(() => + vi.fn(async () => ({ closed: 0, busy: 0, failed: 0 })), +); const mockInstanceSettingsService = vi.hoisted(() => ({ listCompanyIds: vi.fn(), getGeneral: vi.fn(), @@ -111,6 +114,10 @@ vi.mock("../services/environments.js", () => ({ vi.mock("../services/environment-runtime.js", () => ({ environmentRuntimeService: () => mockEnvironmentRuntimeService, })); +vi.mock("../services/native-runtime/native-session-executor.js", () => ({ + closeWarmNativeSessionsForEnvironment: + mockCloseWarmNativeSessionsForEnvironment, +})); vi.mock("../services/execution-workspaces.js", () => ({ executionWorkspaceService: () => mockExecutionWorkspaceService, @@ -229,7 +236,8 @@ const originalSecretsProviderEnv = process.env.PAPERCLIP_SECRETS_PROVIDER; // it only needs to be identity-checkable in assertions. const routeDbTx = { __routeDbTx: true }; const routeDb = { - transaction: async (fn: (tx: unknown) => Promise): Promise => fn(routeDbTx), + transaction: async (fn: (tx: unknown) => Promise): Promise => + fn(routeDbTx), }; function createApp(actor: Record, options: Record = {}) { @@ -279,7 +287,15 @@ describe("environment routes", () => { mockProjectService.getById.mockReset(); mockProjectService.clearExecutionWorkspaceEnvironmentSelection.mockReset(); mockEnvironmentRuntimeService.destroyReusableSandboxLeasesForEnvironment.mockReset(); - mockEnvironmentRuntimeService.destroyReusableSandboxLeasesForEnvironment.mockResolvedValue({ destroyed: 0, failed: 0, skippedLiveRun: 0 }); + mockEnvironmentRuntimeService.destroyReusableSandboxLeasesForEnvironment.mockResolvedValue( + { destroyed: 0, failed: 0, skippedLiveRun: 0 }, + ); + mockCloseWarmNativeSessionsForEnvironment.mockReset(); + mockCloseWarmNativeSessionsForEnvironment.mockResolvedValue({ + closed: 0, + busy: 0, + failed: 0, + }); mockInstanceSettingsService.listCompanyIds.mockReset(); mockInstanceSettingsService.getGeneral.mockReset(); mockInstanceSettingsService.getGeneral.mockResolvedValue({ executionMode: "any" }); @@ -1655,12 +1671,21 @@ describe("environment routes", () => { const res = await request(app).delete("/api/environments/env-1?destroyReusableSandboxLeases=true"); expect(res.status).toBe(200); - expect(mockEnvironmentRuntimeService.destroyReusableSandboxLeasesForEnvironment) - .toHaveBeenCalledExactlyOnceWith({ - environmentId: "env-1", - failureReason: "environment_deleted", - }); - expect(mockEnvironmentService.removeIfDeletable).toHaveBeenCalledWith("env-1"); + expect( + mockEnvironmentRuntimeService.destroyReusableSandboxLeasesForEnvironment, + ).toHaveBeenCalledExactlyOnceWith({ + environmentId: "env-1", + failureReason: "environment_deleted", + }); + expect( + mockCloseWarmNativeSessionsForEnvironment, + ).toHaveBeenCalledExactlyOnceWith({ + environmentId: "env-1", + reason: "environment deleted", + }); + expect(mockEnvironmentService.removeIfDeletable).toHaveBeenCalledWith( + "env-1", + ); expect(res.body.destroyedReusableSandboxLeaseCount).toBe(2); }); @@ -2154,11 +2179,11 @@ describe("environment routes", () => { expect(mockSecretService.create).not.toHaveBeenCalled(); }); - it("keeps the host-owned stream flag and drops a removed flag a saved config still carries", async () => { - // The host owns `streamRunLogs`. It reads it to select the run-log stream. A - // provider plugin normalizes only its own driver fields, so it drops the host - // flag from its normalized config. The host must re-apply it, or the saved - // environment loses the operator opt-out and the stream never starts. + it("keeps host-owned sandbox flags and drops a removed flag a saved config still carries", async () => { + // The host owns run-log streaming and runner lifecycle. A provider plugin + // may normalize only its own driver fields, so it drops these host flags. + // The host must re-apply them or a saved warm environment silently becomes + // per-turn at execution time. // // `streamAgentSessionOutput` is a removed operator flag. A saved config can // still carry it, but session-output streaming now follows the capability @@ -2172,22 +2197,31 @@ describe("environment routes", () => { config: { provider: "fake-plugin", image: "fake:test" }, }; mockEnvironmentService.create.mockResolvedValue(environment); - mockValidatePluginSandboxProviderConfig.mockImplementation(async ({ provider, config }) => { - // Drop the host flag to reproduce a plugin that allowlists driver fields. - const { streamRunLogs, ...driverConfig } = config as Record; - void streamRunLogs; - return { - normalizedConfig: driverConfig, - pluginId: `plugin-${provider}`, - pluginKey: `plugin.${provider}`, - driver: { - driverKey: provider, - kind: "sandbox_provider", - displayName: provider, - configSchema: { type: "object" }, - }, - }; - }); + mockValidatePluginSandboxProviderConfig.mockImplementation( + async ({ provider, config }) => { + // Drop the host flag to reproduce a plugin that allowlists driver fields. + const { + streamRunLogs, + runnerLifecycleMode, + runnerIdleTimeoutMs, + ...driverConfig + } = config as Record; + void streamRunLogs; + void runnerLifecycleMode; + void runnerIdleTimeoutMs; + return { + normalizedConfig: driverConfig, + pluginId: `plugin-${provider}`, + pluginKey: `plugin.${provider}`, + driver: { + driverKey: provider, + kind: "sandbox_provider", + displayName: provider, + configSchema: { type: "object" }, + }, + }; + }, + ); const pluginWorkerManager = {}; const app = createApp({ type: "board", @@ -2204,6 +2238,8 @@ describe("environment routes", () => { provider: "fake-plugin", image: "fake:test", streamRunLogs: false, + runnerLifecycleMode: "warm", + runnerIdleTimeoutMs: 180_000, streamAgentSessionOutput: true, }, }); @@ -2213,6 +2249,8 @@ describe("environment routes", () => { // The removed key never reaches the persisted config. expect(persisted.streamAgentSessionOutput).toBeUndefined(); expect(persisted.streamRunLogs).toBe(false); + expect(persisted.runnerLifecycleMode).toBe("warm"); + expect(persisted.runnerIdleTimeoutMs).toBe(180_000); }); it("creates a schema-driven sandbox environment with secret-ref fields persisted as secrets", async () => { diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index d24350465f..43c5bd26e8 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -115,6 +115,35 @@ describe("findReusableSandboxLeaseId", () => { expect(selected).toBe("sandbox-template-b"); }); + it("ignores host-only log streaming when matching a reusable plugin lease", () => { + const selected = findReusableSandboxLeaseId({ + config: { + provider: "fake-plugin", + image: "template-b", + timeoutMs: 300000, + reuseLease: true, + streamRunLogs: true, + runnerLifecycleMode: "warm", + target: null, + }, + leases: [ + { + providerLeaseId: "sandbox-template-b", + metadata: { + provider: "fake-plugin", + image: "template-b", + timeoutMs: 300000, + reuseLease: true, + runnerLifecycleMode: "warm", + target: "us", + }, + }, + ], + }); + + expect(selected).toBe("sandbox-template-b"); + }); + it("requires image identity for reusable fake sandbox leases", () => { const selected = findReusableSandboxLeaseId({ config: { @@ -610,6 +639,12 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { expect(acquired.lease.metadata?.nativeWorkspaceSync).toEqual( workspaceSyncStamp, ); + await expect( + environmentService(db).getLeaseById(first.lease.id), + ).resolves.toMatchObject({ + status: "expired", + cleanupStatus: "success", + }); expect( workerManager.call.mock.calls.filter( (call) => call[1] === "environmentAcquireLease", @@ -617,6 +652,76 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { ).toHaveLength(1); }); + it("reacquires one provider lease row idempotently during same-run recovery", async () => { + const seeded = await seedReusablePluginSandboxLease(); + const workerManager = { + isRunning: vi.fn((id: string) => id === seeded.pluginId), + call: vi.fn(async (_pluginId: string, method: string) => { + if (method === "environmentResumeLease") { + return { + providerLeaseId: seeded.reusableLease.providerLeaseId, + metadata: { + provider: "fake-plugin", + image: "fake:test", + timeoutMs: 1234, + reuseLease: true, + remoteCwd: "/workspace", + }, + }; + } + throw new Error( + `Unexpected plugin method during same-run reacquisition: ${method}`, + ); + }), + getWorker: vi.fn(() => ({ + supportedMethods: [ + "environmentResumeLease", + "environmentReleaseLease", + "environmentDestroyLease", + ], + })), + } as unknown as PluginWorkerManager; + const runtimeWithPlugin = environmentRuntimeService(db, { + pluginWorkerManager: workerManager, + }); + + const reacquired = await runtimeWithPlugin.acquireRunLease({ + companyId: seeded.companyId, + environment: seeded.environment, + issueId: null, + agentId: seeded.agentId, + heartbeatRunId: seeded.runId, + persistedExecutionWorkspace: { + id: seeded.executionWorkspaceId, + mode: "shared_workspace", + }, + }); + + expect(reacquired.lease).toMatchObject({ + id: seeded.reusableLease.id, + heartbeatRunId: seeded.runId, + providerLeaseId: seeded.reusableLease.providerLeaseId, + status: "active", + cleanupStatus: null, + }); + const rows = await environmentService(db).listLeases(seeded.environment.id); + expect( + rows.filter( + (lease) => + lease.heartbeatRunId === seeded.runId && + lease.providerLeaseId === seeded.reusableLease.providerLeaseId, + ), + ).toHaveLength(1); + expect(workerManager.call).toHaveBeenCalledWith( + seeded.pluginId, + "environmentResumeLease", + expect.objectContaining({ + providerLeaseId: seeded.reusableLease.providerLeaseId, + }), + expect.any(Number), + ); + }); + it("destroys a disposable paperclip_runner sandbox after the turn", async () => { const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease(); const workerManager = { @@ -658,6 +763,13 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease(); await environmentService(db).updateLeaseMetadata(reusableLease.id, { ...(reusableLease.metadata ?? {}), + reusableSandboxLease: { + ...((reusableLease.metadata?.reusableSandboxLease as Record< + string, + unknown + >) ?? {}), + adapterType: "paperclip_runner", + }, sandboxLeaseAcquisition: { outcome: "created" }, }); const workerManager = { @@ -6022,8 +6134,12 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { }); it("destroys scoped reusable plugin-backed sandbox leases", async () => { - const { pluginId, companyId, executionWorkspaceId, reusableLease } = + const { pluginId, companyId, runId, executionWorkspaceId, reusableLease } = await seedReusablePluginSandboxLease(); + await db + .update(heartbeatRuns) + .set({ status: "succeeded" }) + .where(eq(heartbeatRuns.id, runId)); const workerManager = { isRunning: vi.fn((id: string) => id === pluginId), @@ -6062,6 +6178,37 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { }); }); + it("does not destroy a scoped reusable lease while its run is active", async () => { + const { pluginId, companyId, executionWorkspaceId, reusableLease } = + await seedReusablePluginSandboxLease(); + const workerManager = { + isRunning: vi.fn((id: string) => id === pluginId), + call: vi.fn(async () => undefined), + getWorker: vi.fn(() => ({ + supportedMethods: [ + "environmentResumeLease", + "environmentReleaseLease", + "environmentDestroyLease", + ], + })), + } as unknown as PluginWorkerManager; + const runtimeWithPlugin = environmentRuntimeService(db, { + pluginWorkerManager: workerManager, + }); + + const destroyed = await runtimeWithPlugin.destroyReusableSandboxLeases({ + companyId, + executionWorkspaceId, + failureReason: "issue_terminal_done", + }); + + expect(destroyed).toEqual([]); + expect(workerManager.call).not.toHaveBeenCalled(); + await expect( + environmentService(db).getLeaseById(reusableLease.id), + ).resolves.toMatchObject({ status: "active" }); + }); + it("destroys reusable plugin-backed sandbox leases scoped to an environment", async () => { const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease(); // The holding run is finished, so the reservation is stale and destroyable. @@ -6192,8 +6339,12 @@ describeEmbeddedPostgres("environmentRuntimeService", () => { }); it("retries reusable plugin-backed sandbox destroy when the worker is unavailable", async () => { - const { pluginId, companyId, executionWorkspaceId, reusableLease } = + const { pluginId, companyId, executionWorkspaceId, runId, reusableLease } = await seedReusablePluginSandboxLease(); + await db + .update(heartbeatRuns) + .set({ status: "succeeded", finishedAt: new Date() }) + .where(eq(heartbeatRuns.id, runId)); const offlineWorkerManager = { isRunning: vi.fn(() => false), diff --git a/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts b/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts index ae6af37121..632eb2f779 100644 --- a/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts +++ b/server/src/__tests__/heartbeat-run-terminalize-before-release.test.ts @@ -148,9 +148,11 @@ describeEmbeddedPostgres("heartbeat teardown terminalizes the run before releasi runId: string; companyId: string; agentId: string; + providerResourceDisposition?: "keep_running" | "stop_and_retain" | "destroy"; }) { const { runId, companyId, agentId } = input; const releaseLeafCalls: Array<{ runId: string; status: string }> = []; + const ordering: string[] = []; const fakeEnvironmentRuntime = { // The orchestrator's `releaseForRun` calls this leaf with the mapped lease // status. There are no seeded leases, so return an empty release set. @@ -158,11 +160,18 @@ describeEmbeddedPostgres("heartbeat teardown terminalizes the run before releasi heartbeatRunId: string, status: "released" | "expired" | "failed", ) => { + ordering.push(`release:${heartbeatRunId}`); releaseLeafCalls.push({ runId: heartbeatRunId, status }); return []; }, } as unknown as HeartbeatEnvironmentRuntime; - const heartbeat = heartbeatService(db, { environmentRuntime: fakeEnvironmentRuntime }); + const heartbeat = heartbeatService(db, { + environmentRuntime: fakeEnvironmentRuntime, + closeWarmNativeSessionsForRun: async ({ runId }) => { + ordering.push(`close:${runId}`); + return { closed: 1, busy: 0, failed: 0 }; + }, + }); let latestRun = await db .select() @@ -182,6 +191,7 @@ describeEmbeddedPostgres("heartbeat teardown terminalizes the run before releasi companyId, agentId, status: statusThreadedToRelease, + providerResourceDisposition: input.providerResourceDisposition, }); const orchestratorObservedRunId = releaseLeafCalls.at(-1)?.runId ?? null; const orchestratorObservedLeaseStatus = releaseLeafCalls.at(-1)?.status ?? null; @@ -193,10 +203,43 @@ describeEmbeddedPostgres("heartbeat teardown terminalizes the run before releasi releaseCallCount: releaseLeafCalls.length, orchestratorObservedRunId, orchestratorObservedLeaseStatus, + ordering, terminalRun: latestRun, }; } + it("closes a warm native session before destroying its terminal lease", async () => { + const { companyId, agentId, runId } = await seed({ issueStatus: "done", runStatus: "running" }); + const observed = await runTeardownSequenceObservingRelease({ + runId, + companyId, + agentId, + providerResourceDisposition: "destroy", + }); + + expect(observed.ordering).toEqual([`close:${runId}`, `release:${runId}`]); + expect(observed.releaseCallCount).toBe(1); + }); + + it("does not destroy a terminal lease while its warm native session is busy", async () => { + const { companyId, agentId, runId } = await seed({ issueStatus: "done", runStatus: "running" }); + const releaseRunLeases = vi.fn(async () => []); + const heartbeat = heartbeatService(db, { + environmentRuntime: { releaseRunLeases } as unknown as HeartbeatEnvironmentRuntime, + closeWarmNativeSessionsForRun: async () => ({ closed: 0, busy: 1, failed: 0 }), + }); + + await heartbeat.releaseEnvironmentLeasesForRun({ + runId, + companyId, + agentId, + status: "succeeded", + providerResourceDisposition: "destroy", + }); + + expect(releaseRunLeases).not.toHaveBeenCalled(); + }); + it("terminalizes a running run to succeeded before release when the issue reached done", async () => { const { companyId, agentId, issueId, runId } = await seed({ issueStatus: "done", runStatus: "running" }); diff --git a/server/src/__tests__/heartbeat-workspace-session.test.ts b/server/src/__tests__/heartbeat-workspace-session.test.ts index 469ec3600f..1da2071e2c 100644 --- a/server/src/__tests__/heartbeat-workspace-session.test.ts +++ b/server/src/__tests__/heartbeat-workspace-session.test.ts @@ -2286,6 +2286,45 @@ describe("effective run session config freshness", () => { }); }); + it("does not reset when a reusable execution workspace becomes realized", async () => { + const base = await buildSessionConfigMetadata({ + workspaceConfig: { + requestedMode: "shared_workspace", + effectiveMode: "shared_workspace", + reusableExecutionWorkspaceConfig: null, + existingExecutionWorkspace: null, + }, + }); + const realized = await buildSessionConfigMetadata({ + workspaceConfig: { + requestedMode: "shared_workspace", + effectiveMode: "shared_workspace", + reusableExecutionWorkspaceConfig: { + strategyType: "project_primary", + workspaceGeneration: 1, + }, + existingExecutionWorkspace: { + id: "workspace-realized-after-first-turn", + mode: "shared_workspace", + strategyType: "project_primary", + }, + }, + }); + + expect( + resolveTaskSessionConfigFreshness({ + hasTaskSession: true, + configuredModel: "gpt-5.4-mini", + taskSessionParams: sessionParamsWithConfigMetadata(base), + configMetadata: realized, + }), + ).toMatchObject({ + reset: false, + changedCategories: [], + reasons: [], + }); + }); + it("keeps model-only compatibility as an additional reset reason", async () => { const base = await buildSessionConfigMetadata(); diff --git a/server/src/__tests__/issue-thread-interaction-routes.test.ts b/server/src/__tests__/issue-thread-interaction-routes.test.ts index ab0168e975..97b84248c4 100644 --- a/server/src/__tests__/issue-thread-interaction-routes.test.ts +++ b/server/src/__tests__/issue-thread-interaction-routes.test.ts @@ -1999,6 +1999,62 @@ describe.sequential("issue thread interaction routes", () => { ); }); + it("delivers generic confirmation rejection feedback as the next turn message", async () => { + mockInteractionService.rejectInteraction.mockResolvedValueOnce({ + id: "interaction-warm-turn", + companyId: "company-1", + issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + kind: "request_confirmation", + status: "rejected", + continuationPolicy: "wake_assignee", + idempotencyKey: "warm-turn-1", + sourceCommentId: null, + sourceRunId: RUN_3, + payload: { + version: 1, + prompt: "Continue to turn two?", + target: { + type: "custom", + key: "warm_turn_1", + revisionId: "turn-1", + }, + }, + result: { + version: 1, + outcome: "rejected", + reason: "Read T1, append T2, and verify both lines.", + }, + createdAt: "2026-04-20T12:00:00.000Z", + updatedAt: "2026-04-20T12:05:00.000Z", + resolvedAt: "2026-04-20T12:05:00.000Z", + }); + + const res = await request(await createApp()) + .post( + "/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-warm-turn/reject", + ) + .send({ reason: "Read T1, append T2, and verify both lines." }); + + expect(res.status).toBe(200); + expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( + ASSIGNEE_AGENT_ID, + expect.objectContaining({ + payload: expect.objectContaining({ + paperclipAgentMessage: { + text: "Read T1, append T2, and verify both lines.", + source: "interaction_rejection", + sessionId: "interaction-warm-turn", + }, + }), + contextSnapshot: expect.objectContaining({ + paperclipAgentMessage: expect.objectContaining({ + text: "Read T1, append T2, and verify both lines.", + }), + }), + }), + ); + }); + it("returns a rejected native completion review with a narrow reviewer-reason continuation", async () => { const issue = createIssue({ status: "in_review" }); mockIssueService.getById.mockResolvedValue(issue); diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index c6d2921fe4..e5b6f536b9 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -1906,6 +1906,56 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { })).rejects.toThrow("A decline reason is required for this confirmation"); }); + it("reopens an in-review issue before waking the assignee after rejection", async () => { + const { companyId, issueId } = await seedConfirmationIssue( + "Continue after review rejection", + ); + const created = await interactionsSvc.create( + { id: issueId, companyId }, + { + kind: "request_confirmation", + continuationPolicy: "wake_assignee", + payload: { + version: 1, + prompt: "Continue the next turn?", + rejectLabel: "Continue work", + rejectRequiresReason: true, + target: { + type: "custom", + key: "warm_turn_1", + revisionId: "warm-turn-1", + }, + }, + }, + { + userId: "local-board", + }, + ); + await db + .update(issues) + .set({ status: "in_review" }) + .where(eq(issues.id, issueId)); + + await interactionsSvc.rejectInteraction( + { + id: issueId, + companyId, + status: "in_review", + }, + created.id, + { + reason: "Proceed with turn two.", + }, + { + userId: "local-board", + }, + ); + + await expect(issuesSvc.getById(issueId)).resolves.toMatchObject({ + status: "todo", + }); + }); + it("records an authorized agent as the review-confirmation resolver", async () => { const { companyId, goalId, issueId } = await seedConfirmationIssue("Agent review verdict"); const resolverAgentId = randomUUID(); diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 6baed72c3a..f97cf65cd0 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -7130,4 +7130,26 @@ describeEmbeddedPostgres("issueService.addComment createdByRunId", () => { expect(await createdByRunIdFor(comment.id)).toBe(runId); }); + + it("deduplicates concurrent identical comments from the same run", async () => { + const runId = randomUUID(); + await db.insert(heartbeatRuns).values({ + id: runId, + companyId, + agentId, + status: "running", + }); + + const [first, second] = await Promise.all([ + svc.addComment(issueId, "one durable result", { agentId, runId }), + svc.addComment(issueId, "one durable result", { agentId, runId }), + ]); + + expect(second.id).toBe(first.id); + const duplicates = await db + .select({ id: issueComments.id }) + .from(issueComments) + .where(eq(issueComments.createdByRunId, runId)); + expect(duplicates).toHaveLength(1); + }); }); diff --git a/server/src/__tests__/native-sandbox-lifecycle.test.ts b/server/src/__tests__/native-sandbox-lifecycle.test.ts index 13b2553b9f..be7a68486c 100644 --- a/server/src/__tests__/native-sandbox-lifecycle.test.ts +++ b/server/src/__tests__/native-sandbox-lifecycle.test.ts @@ -14,11 +14,13 @@ const reusableSandbox = { describe("paperclip_runner sandbox lifecycle", () => { it("keeps a warm reusable sandbox running", () => { - expect(resolveNativeSandboxLifecycle({ - adapterType: "paperclip_runner", - lifecyclePolicy: { mode: "warm", idleTimeoutMs: 300_000 }, - target: reusableSandbox, - })).toEqual({ + expect( + resolveNativeSandboxLifecycle({ + adapterType: "paperclip_runner", + lifecyclePolicy: { mode: "warm", idleTimeoutMs: 300_000 }, + target: reusableSandbox, + }), + ).toEqual({ runnerProcess: "warm", sandboxResource: "keep_running", failoverBackup: "verified", @@ -39,11 +41,13 @@ describe("paperclip_runner sandbox lifecycle", () => { }); it("stops and reuses a per-turn reusable sandbox", () => { - expect(resolveNativeSandboxLifecycle({ - adapterType: "paperclip_runner", - lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, - target: reusableSandbox, - })).toEqual({ + expect( + resolveNativeSandboxLifecycle({ + adapterType: "paperclip_runner", + lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, + target: reusableSandbox, + }), + ).toEqual({ runnerProcess: "per_turn", sandboxResource: "stop_and_reuse", failoverBackup: "verified", @@ -51,14 +55,16 @@ describe("paperclip_runner sandbox lifecycle", () => { }); it("destroys a per-turn disposable sandbox", () => { - expect(resolveNativeSandboxLifecycle({ - adapterType: "paperclip_runner", - lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, - target: { - ...reusableSandbox, - reusableLeaseConfigured: false, - }, - })).toEqual({ + expect( + resolveNativeSandboxLifecycle({ + adapterType: "paperclip_runner", + lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, + target: { + ...reusableSandbox, + reusableLeaseConfigured: false, + }, + }), + ).toEqual({ runnerProcess: "per_turn", sandboxResource: "destroy_after_turn", failoverBackup: "verified", @@ -66,27 +72,33 @@ describe("paperclip_runner sandbox lifecycle", () => { }); it("rejects warm mode without an effective reusable-lease capability", () => { - expect(() => resolveNativeSandboxLifecycle({ - adapterType: "paperclip_runner", - lifecyclePolicy: { mode: "warm", idleTimeoutMs: 300_000 }, - target: { - ...reusableSandbox, - effectiveCapabilities: { reusableLeases: false }, - }, - })).toThrow("runner_warm_lifecycle_requires_reusable_provider_lease"); + expect(() => + resolveNativeSandboxLifecycle({ + adapterType: "paperclip_runner", + lifecyclePolicy: { mode: "warm", idleTimeoutMs: 300_000 }, + target: { + ...reusableSandbox, + effectiveCapabilities: { reusableLeases: false }, + }, + }), + ).toThrow("runner_warm_lifecycle_requires_reusable_provider_lease"); }); it("leaves legacy and non-sandbox targets untouched", () => { - expect(resolveNativeSandboxLifecycle({ - adapterType: "codex_local", - lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, - target: reusableSandbox, - })).toBeNull(); - expect(resolveNativeSandboxLifecycle({ - adapterType: "paperclip_runner", - lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, - target: { kind: "local" }, - })).toBeNull(); + expect( + resolveNativeSandboxLifecycle({ + adapterType: "codex_local", + lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, + target: reusableSandbox, + }), + ).toBeNull(); + expect( + resolveNativeSandboxLifecycle({ + adapterType: "paperclip_runner", + lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null }, + target: { kind: "local" }, + }), + ).toBeNull(); }); it("keeps a warm sandbox only after a successful turn", () => { diff --git a/server/src/__tests__/native-workspace-sync.test.ts b/server/src/__tests__/native-workspace-sync.test.ts index a55ee16512..c4598fc708 100644 --- a/server/src/__tests__/native-workspace-sync.test.ts +++ b/server/src/__tests__/native-workspace-sync.test.ts @@ -77,6 +77,14 @@ describe("native workspace sync durable metadata", () => { sameProviderLease: false, }), ).toBe("durable_seed"); + expect( + classifyNativeWorkspaceInbound({ + kind: "existing_run", + restartRecovery: false, + sameRunRecovery: true, + sameProviderLease: true, + }), + ).toBe("adopt_remote"); expect(() => classifyNativeWorkspaceInbound({ kind: "existing_run", diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 2b0916c49c..68d51234cb 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -59,6 +59,7 @@ import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; import { environmentService } from "../services/environments.js"; import { environmentRuntimeService } from "../services/environment-runtime.js"; import { executionWorkspaceService } from "../services/execution-workspaces.js"; +import { closeWarmNativeSessionsForEnvironment } from "../services/native-runtime/native-session-executor.js"; function isPlainRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); @@ -1294,10 +1295,23 @@ export function environmentRoutes( && impact.reusableSandboxLeaseCount > 0 && impact.deleteBlockedReasons.every((reason) => reason === "reusable_sandbox_lease") ) { - const destroyResult = await environmentRuntime.destroyReusableSandboxLeasesForEnvironment({ + const warmSessions = await closeWarmNativeSessionsForEnvironment({ environmentId: existing.id, - failureReason: "environment_deleted", + reason: "environment deleted", }); + if (warmSessions.busy > 0 || warmSessions.failed > 0) { + throw conflict( + warmSessions.busy > 0 + ? "Cannot delete this environment while a native runner session is active. Wait for its run to finish, then retry." + : "Cannot delete this environment because its warm native runner did not shut down cleanly. Retry after the runner exits.", + { nativeRunnerSessions: warmSessions }, + ); + } + const destroyResult = + await environmentRuntime.destroyReusableSandboxLeasesForEnvironment({ + environmentId: existing.id, + failureReason: "environment_deleted", + }); destroyedReusableSandboxLeaseCount = destroyResult.destroyed; impact = await svc.getDeleteBlastRadius(existing.id); if (!impact) { diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index e038d812af..9f547d0c98 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -2231,55 +2231,85 @@ async function queueResolvedInteractionContinuationWakeup(input: { result: interactionResult, } : null; - void input.heartbeat.wakeup(input.issue.assigneeAgentId, { - source: "automation", - triggerDetail: "system", - reason: "issue_commented", - payload: { - issueId: input.issue.id, - interactionId: input.interaction.id, - interactionKind: input.interaction.kind, - interactionStatus: input.interaction.status, - sourceCommentId: input.interaction.sourceCommentId ?? null, - sourceRunId: input.interaction.sourceRunId ?? null, - ...(planReviewInteraction ? { planReviewInteraction } : {}), - ...(nativeCompletionReview ? { nativeCompletionReview } : {}), - ...(checkboxSelection ? { checkboxSelection } : {}), - ...(toolAction ? { toolAction } : {}), - ...(secretProposal ? { secretProposal } : {}), - ...(itemVerdicts ? { itemVerdicts, newlyResolvedItemIds } : {}), - ...(reviewPathContext ?? {}), - mutation: "interaction", - }, - idempotencyKey: input.idempotencyKey ?? `interaction:${input.interaction.id}:${input.interaction.status}`, - requestedByActorType: input.actor.actorType, - requestedByActorId: input.actor.actorId, - contextSnapshot: { - issueId: input.issue.id, - taskId: input.issue.id, - interactionId: input.interaction.id, - interactionKind: input.interaction.kind, - interactionStatus: input.interaction.status, - sourceCommentId: input.interaction.sourceCommentId ?? null, - sourceRunId: input.interaction.sourceRunId ?? null, - ...(planReviewInteraction ? { planReviewInteraction } : {}), - ...(nativeCompletionReview ? { nativeCompletionReview } : {}), - ...(checkboxSelection ? { checkboxSelection } : {}), - ...(toolAction ? { toolAction } : {}), - ...(secretProposal ? { secretProposal } : {}), - ...(itemVerdicts ? { itemVerdicts, newlyResolvedItemIds } : {}), - ...(reviewPathContext ?? {}), - wakeReason: "issue_commented", - source: input.source, - ...(forceFreshSession ? { forceFreshSession: true } : {}), - ...(workspaceRefreshReason ? { workspaceRefreshReason } : {}), - }, - }).catch((err) => logger.warn({ - err, - issueId: input.issue.id, - interactionId: input.interaction.id, - agentId: input.issue.assigneeAgentId, - }, "failed to wake assignee on issue interaction resolution")); + const genericRejectionReason = + !planReviewInteraction && + !nativeCompletionReview && + input.interaction.status === "rejected" && + (input.interaction.kind === "request_confirmation" || + input.interaction.kind === "request_checkbox_confirmation") + ? readNonEmptyString(readObject(input.interaction.result).reason) + : null; + const rejectionAgentMessage = genericRejectionReason + ? { + text: genericRejectionReason, + source: "interaction_rejection", + sessionId: input.interaction.id, + } + : null; + void input.heartbeat + .wakeup(input.issue.assigneeAgentId, { + source: "automation", + triggerDetail: "system", + reason: "issue_commented", + payload: { + issueId: input.issue.id, + interactionId: input.interaction.id, + interactionKind: input.interaction.kind, + interactionStatus: input.interaction.status, + sourceCommentId: input.interaction.sourceCommentId ?? null, + sourceRunId: input.interaction.sourceRunId ?? null, + ...(planReviewInteraction ? { planReviewInteraction } : {}), + ...(nativeCompletionReview ? { nativeCompletionReview } : {}), + ...(checkboxSelection ? { checkboxSelection } : {}), + ...(toolAction ? { toolAction } : {}), + ...(secretProposal ? { secretProposal } : {}), + ...(itemVerdicts ? { itemVerdicts, newlyResolvedItemIds } : {}), + ...(rejectionAgentMessage + ? { paperclipAgentMessage: rejectionAgentMessage } + : {}), + ...(reviewPathContext ?? {}), + mutation: "interaction", + }, + idempotencyKey: + input.idempotencyKey ?? + `interaction:${input.interaction.id}:${input.interaction.status}`, + requestedByActorType: input.actor.actorType, + requestedByActorId: input.actor.actorId, + contextSnapshot: { + issueId: input.issue.id, + taskId: input.issue.id, + interactionId: input.interaction.id, + interactionKind: input.interaction.kind, + interactionStatus: input.interaction.status, + sourceCommentId: input.interaction.sourceCommentId ?? null, + sourceRunId: input.interaction.sourceRunId ?? null, + ...(planReviewInteraction ? { planReviewInteraction } : {}), + ...(nativeCompletionReview ? { nativeCompletionReview } : {}), + ...(checkboxSelection ? { checkboxSelection } : {}), + ...(toolAction ? { toolAction } : {}), + ...(secretProposal ? { secretProposal } : {}), + ...(itemVerdicts ? { itemVerdicts, newlyResolvedItemIds } : {}), + ...(rejectionAgentMessage + ? { paperclipAgentMessage: rejectionAgentMessage } + : {}), + ...(reviewPathContext ?? {}), + wakeReason: "issue_commented", + source: input.source, + ...(forceFreshSession ? { forceFreshSession: true } : {}), + ...(workspaceRefreshReason ? { workspaceRefreshReason } : {}), + }, + }) + .catch((err) => + logger.warn( + { + err, + issueId: input.issue.id, + interactionId: input.interaction.id, + agentId: input.issue.assigneeAgentId, + }, + "failed to wake assignee on issue interaction resolution", + ), + ); } function readCheckboxSelectionForWake(input: { diff --git a/server/src/services/environment-config.ts b/server/src/services/environment-config.ts index f0a14ec5c6..aab87b273c 100644 --- a/server/src/services/environment-config.ts +++ b/server/src/services/environment-config.ts @@ -408,22 +408,24 @@ export function stripSandboxProviderEnvelope(config: SandboxEnvironmentConfig): return driverConfig; } -// The host owns this sandbox run-behavior flag, not the provider plugin. The -// host reads it to select the run-log stream. The host passes the whole config -// to the plugin, so a plugin that allowlists its own driver fields drops the -// flag from its normalized config. Re-apply it from the parsed envelope after -// the plugin normalizes, or a saved environment loses the operator opt-in and -// the stream never starts. -const HOST_OWNED_SANDBOX_STREAM_FLAGS = [ +// The host owns these sandbox run-behavior flags, not the provider plugin. The +// host passes the whole config to the plugin, so a plugin that allowlists only +// its provider fields may drop them from its normalized config. Re-apply them +// from the parsed envelope after plugin normalization. Otherwise a saved +// environment can silently fall back to per-turn runner behavior even though +// the caller explicitly selected a warm lifecycle. +const HOST_OWNED_SANDBOX_FLAGS = [ "streamRunLogs", + "runnerLifecycleMode", + "runnerIdleTimeoutMs", ] as const; -function applyHostOwnedSandboxStreamFlags( +function applyHostOwnedSandboxFlags( normalizedConfig: Record, envelope: Record, ): Record { const merged: Record = { ...normalizedConfig }; - for (const key of HOST_OWNED_SANDBOX_STREAM_FLAGS) { + for (const key of HOST_OWNED_SANDBOX_FLAGS) { if (envelope[key] !== undefined) { merged[key] = envelope[key]; } @@ -518,7 +520,10 @@ export function normalizeEnvironmentConfigForProbe(input: { ...(await resolveConfigSecretRefsForProbe({ db: input.db, companyId: input.companyId, - config: applyHostOwnedSandboxStreamFlags(validated.normalizedConfig, parsed.data), + config: applyHostOwnedSandboxFlags( + validated.normalizedConfig, + parsed.data, + ), accessContext: input.accessContext, schema: validated.driver.configSchema && @@ -610,7 +615,7 @@ export async function normalizeEnvironmentConfigForPersistence(input: { secretProvider: input.secretProvider, config: { provider: parsed.data.provider, - ...applyHostOwnedSandboxStreamFlags(validated.normalizedConfig, parsed.data), + ...applyHostOwnedSandboxFlags(validated.normalizedConfig, parsed.data), }, schema: validated.driver.configSchema && typeof validated.driver.configSchema === "object" && !Array.isArray(validated.driver.configSchema) diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index f35b16fb40..af20c2bbf0 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -1077,7 +1077,16 @@ export function findReusableSandboxLeaseId(input: { config: SandboxEnvironmentConfig; leases: Array>; }): string | null { - return findReusableSandboxProviderLeaseId(input); + // Host-only run behavior (for example streamRunLogs) is intentionally not + // echoed by a sandbox provider in lease metadata and must not invalidate an + // otherwise identical reusable provider lease. + return findReusableSandboxProviderLeaseId({ + config: { + provider: input.config.provider, + ...stripSandboxProviderEnvelope(input.config), + } as SandboxEnvironmentConfig, + leases: input.leases, + }); } function createLocalEnvironmentDriver(db: Db): EnvironmentRuntimeDriver { @@ -2083,6 +2092,16 @@ function createSandboxEnvironmentDriver( acquiredLease.expiresAt ? new Date(acquiredLease.expiresAt) : undefined, ), metadata: pluginLeaseMetadata, + reusesReusableLeaseId: + providerLease && + reusableLease?.heartbeatRunId === input.heartbeatRunId + ? reusableLease.id + : null, + replacesReusableLeaseId: + providerLease && + reusableLease?.heartbeatRunId !== input.heartbeatRunId + ? reusableLease?.id + : null, }); } catch (error) { // The conditional lease insert rejected, so no lease row exists. A @@ -2312,6 +2331,18 @@ function createSandboxEnvironmentDriver( providerLease.expiresAt ? new Date(providerLease.expiresAt) : undefined, ), metadata: builtinLeaseMetadata, + reusesReusableLeaseId: + reusableLease && + providerLease.providerLeaseId === reusableLease.providerLeaseId && + reusableLease.heartbeatRunId === input.heartbeatRunId + ? reusableLease.id + : null, + replacesReusableLeaseId: + reusableLease && + providerLease.providerLeaseId === reusableLease.providerLeaseId && + reusableLease.heartbeatRunId !== input.heartbeatRunId + ? reusableLease.id + : null, }); } catch (error) { // The conditional lease insert rejected, so no lease row exists. A managed @@ -3717,6 +3748,9 @@ export function environmentRuntimeService( } if ( providerResourceDisposition === "destroy" && + isRecord(leaseSnapshot.metadata?.reusableSandboxLease) && + leaseSnapshot.metadata.reusableSandboxLease.adapterType === + "paperclip_runner" && leaseSnapshot.metadata?.sandboxLeaseAcquisition && (!leaseSnapshot.providerLeaseId || !verifyNativeHarnessBackupStamp( @@ -3845,8 +3879,39 @@ export function environmentRuntimeService( ), ); + const holdingRunIds = leaseRows + .map((row) => row.heartbeatRunId) + .filter((runId): runId is string => Boolean(runId)); + const liveRunIds = new Set(); + if (holdingRunIds.length > 0) { + const liveRuns = await db + .select({ id: heartbeatRuns.id }) + .from(heartbeatRuns) + .where( + and( + inArray(heartbeatRuns.id, holdingRunIds), + inArray(heartbeatRuns.status, [ + "queued", + "scheduled_retry", + "running", + ]), + ), + ); + for (const liveRun of liveRuns) liveRunIds.add(liveRun.id); + } + const destroyed: EnvironmentRuntimeLeaseRecord[] = []; for (const leaseRow of leaseRows) { + // An issue may become terminal inside its provider turn. Do not tear + // down the sandbox while that run is still exporting its workspace or + // polling the callback bridge. The heartbeat finalizer observes the + // terminal issue and destroys the resource after those boundaries. + if ( + leaseRow.heartbeatRunId && + liveRunIds.has(leaseRow.heartbeatRunId) + ) { + continue; + } const environment = leaseRow.environmentId ? await environmentsSvc.getById(leaseRow.environmentId) : null; diff --git a/server/src/services/environments.ts b/server/src/services/environments.ts index b491bc83af..393b24bbf7 100644 --- a/server/src/services/environments.ts +++ b/server/src/services/environments.ts @@ -1335,6 +1335,21 @@ export function environmentService(db: Db) { providerLeaseId?: string | null; expiresAt?: Date | null; metadata?: Record | null; + /** + * Atomically retire the previous database ownership record when this + * acquisition reuses the same provider resource for a new run. The + * provider resume happens before this write, so a failed transaction + * leaves the prior retained row recoverable instead of publishing two + * reusable owners for one sandbox. + */ + replacesReusableLeaseId?: string | null; + /** + * Reactivate the exact lease row already owned by this heartbeat run. + * Native same-run recovery re-enters environment startup after the + * provider process is interrupted; it must not manufacture a second + * database owner for the same run and provider resource. + */ + reusesReusableLeaseId?: string | null; /** * Re-check the environment company binding inside the lease insert * transaction. The login routes set this to close the check-to-lease @@ -1368,47 +1383,168 @@ export function environmentService(db: Db) { createdAt: now, updatedAt: now, }; - const row = input.assertCompanyBinding - ? await db.transaction(async (tx) => { - // Lock the environment row first. Managed reconciliation locks the - // same sandbox environment rows with `for update` before it writes a - // company binding, so this lock serializes the two transactions on - // this row and closes the time-of-check to time-of-use window. - await tx - .select({ id: environments.id }) - .from(environments) - .where(eq(environments.id, input.environmentId)) - .for("update"); - // Re-read the company binding inside the locked transaction. A - // binding a reconciliation committed after the route guard now - // appears here. Reject a foreign-company environment before the - // insert, so the login holds no lease. - const boundRows = await tx - .select({ companyId: builtInManagedResources.companyId }) - .from(builtInManagedResources) - .where( - and( - eq(builtInManagedResources.resourceKind, MANAGED_ENVIRONMENT_RESOURCE_KIND), - eq(builtInManagedResources.resourceId, input.environmentId), - ), - ); - const boundCompanyIds = Array.from(new Set(boundRows.map((boundRow) => boundRow.companyId))); - if (boundCompanyIds.length > 0 && !boundCompanyIds.includes(input.companyId)) { - throw forbidden("The selected environment belongs to another company.", { - code: "environment_company_mismatch", - }); - } - return tx + if ( + (input.replacesReusableLeaseId || input.reusesReusableLeaseId) && + (!input.executionWorkspaceId || !input.providerLeaseId) + ) { + throw new Error( + "A reusable lease handoff requires an execution workspace and provider lease id.", + ); + } + if (input.reusesReusableLeaseId && !input.heartbeatRunId) { + throw new Error( + "A same-run reusable lease reacquisition requires a heartbeat run id.", + ); + } + if (input.replacesReusableLeaseId && input.reusesReusableLeaseId) { + throw new Error( + "A reusable lease cannot be replaced and reacquired in the same operation.", + ); + } + const row = + input.assertCompanyBinding || + input.replacesReusableLeaseId || + input.reusesReusableLeaseId + ? await db.transaction(async (tx) => { + if (input.assertCompanyBinding) { + // Lock the environment row first. Managed reconciliation locks the + // same sandbox environment rows with `for update` before it writes a + // company binding, so this lock serializes the two transactions on + // this row and closes the time-of-check to time-of-use window. + await tx + .select({ id: environments.id }) + .from(environments) + .where(eq(environments.id, input.environmentId)) + .for("update"); + // Re-read the company binding inside the locked transaction. A + // binding a reconciliation committed after the route guard now + // appears here. Reject a foreign-company environment before the + // insert, so the login holds no lease. + const boundRows = await tx + .select({ companyId: builtInManagedResources.companyId }) + .from(builtInManagedResources) + .where( + and( + eq( + builtInManagedResources.resourceKind, + MANAGED_ENVIRONMENT_RESOURCE_KIND, + ), + eq( + builtInManagedResources.resourceId, + input.environmentId, + ), + ), + ); + const boundCompanyIds = Array.from( + new Set(boundRows.map((boundRow) => boundRow.companyId)), + ); + if ( + boundCompanyIds.length > 0 && + !boundCompanyIds.includes(input.companyId) + ) { + throw forbidden( + "The selected environment belongs to another company.", + { + code: "environment_company_mismatch", + }, + ); + } + } + if (input.replacesReusableLeaseId) { + const retired = await tx + .update(environmentLeases) + .set({ + status: "expired", + releasedAt: now, + lastUsedAt: now, + updatedAt: now, + cleanupStatus: "success", + }) + .where( + and( + eq(environmentLeases.id, input.replacesReusableLeaseId), + eq(environmentLeases.companyId, input.companyId), + eq(environmentLeases.environmentId, input.environmentId), + eq( + environmentLeases.executionWorkspaceId, + input.executionWorkspaceId!, + ), + eq(environmentLeases.leasePolicy, "reuse_by_environment"), + eq( + environmentLeases.providerLeaseId, + input.providerLeaseId!, + ), + inArray(environmentLeases.status, [ + "released", + "retained", + ]), + ), + ) + .returning({ id: environmentLeases.id }); + if (retired.length !== 1) { + throw conflict( + "Reusable sandbox lease ownership changed during acquisition.", + ); + } + } + if (input.reusesReusableLeaseId) { + const reacquired = await tx + .update(environmentLeases) + .set({ + status: "active", + releasedAt: null, + lastUsedAt: now, + expiresAt: input.expiresAt ?? null, + failureReason: null, + cleanupStatus: null, + metadata: input.metadata ?? null, + updatedAt: now, + }) + .where( + and( + eq(environmentLeases.id, input.reusesReusableLeaseId), + eq(environmentLeases.companyId, input.companyId), + eq(environmentLeases.environmentId, input.environmentId), + eq( + environmentLeases.executionWorkspaceId, + input.executionWorkspaceId!, + ), + eq( + environmentLeases.heartbeatRunId, + input.heartbeatRunId!, + ), + eq(environmentLeases.leasePolicy, "reuse_by_environment"), + eq( + environmentLeases.providerLeaseId, + input.providerLeaseId!, + ), + inArray(environmentLeases.status, [ + "active", + "released", + "retained", + ]), + ), + ) + .returning() + .then((rows) => rows[0] ?? null); + if (!reacquired) { + throw conflict( + "Reusable sandbox lease ownership changed during reacquisition.", + ); + } + return reacquired; + } + return tx + .insert(environmentLeases) + .values(values) + .returning() + .then((rows) => rows[0] ?? null); + }) + : await db .insert(environmentLeases) .values(values) .returning() .then((rows) => rows[0] ?? null); - }) - : await db - .insert(environmentLeases) - .values(values) - .returning() - .then((rows) => rows[0] ?? null); if (!row) { throw new Error("Failed to acquire environment lease"); } diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 94a52139ee..43ef1a643a 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -130,6 +130,7 @@ import { buildNativeRuntimeContext, cancelNativeSession, claimNativeRestartRecoveries, + closeWarmNativeSessionsForEnvironment, currentNativeControllerIdentity, dispatchNativeSessionResumptions, detachNativeSessionsForRestart, @@ -1295,7 +1296,8 @@ export async function resolveExecutionRunAdapterConfig(input: { input.trustPreset?.kind === "low_trust_review" ? (input.trustPreset.boundary.allowedSecretBindingIds ?? []) : undefined; - const allowTrustedEnvProjection = input.trustPreset?.kind !== "low_trust_review"; + const allowTrustedEnvProjection = + input.trustPreset?.kind !== "low_trust_review"; if (input.trustPreset?.kind === "low_trust_review") { assertLowTrustEnvConfigAllowed(environmentEnv, "environment.env"); assertLowTrustEnvConfigAllowed(executionRunConfig.env, "agent.env"); @@ -1306,7 +1308,8 @@ export async function resolveExecutionRunAdapterConfig(input: { const requiredScopedBindingsConfigured = requiredScopedEnvBinding ? requiredScopedEnvBinding.keys.some( (key) => - (allowTrustedEnvProjection && typeof input.trustedEnvProjection?.[key] === "string") || + (allowTrustedEnvProjection && + typeof input.trustedEnvProjection?.[key] === "string") || (requiredScopedEnvBinding.consumerScopes.includes("agent") && isConfiguredEnvBindingValue(agentEnv[key])) || (requiredScopedEnvBinding.consumerScopes.includes("project") && @@ -1567,9 +1570,9 @@ export async function resolveExecutionRunAdapterConfig(input: { } } if ( - allowTrustedEnvProjection - && input.trustedEnvProjection - && Object.keys(input.trustedEnvProjection).length > 0 + allowTrustedEnvProjection && + input.trustedEnvProjection && + Object.keys(input.trustedEnvProjection).length > 0 ) { resolvedConfig.env = { ...parseObject(resolvedConfig.env), @@ -1753,9 +1756,8 @@ export function providerResourceDispositionForTerminalRun( desired: ProviderResourceDisposition | undefined, status: string | null | undefined, ): ProviderResourceDisposition | undefined { - return desired === "keep_running" && status !== "succeeded" - ? "stop_and_retain" - : desired; + if (desired !== "keep_running") return desired; + return status === "succeeded" ? desired : "stop_and_retain"; } export interface NativeSandboxLifecycle { @@ -4089,19 +4091,22 @@ export async function buildPaperclipRuntimeMcpServers(input: { const [runIdentity] = await input.db .select({ responsibleUserId: heartbeatRuns.responsibleUserId }) .from(heartbeatRuns) - .where(and( - eq(heartbeatRuns.id, input.runId), - eq(heartbeatRuns.companyId, input.agent.companyId), - eq(heartbeatRuns.agentId, input.agent.id), - )) + .where( + and( + eq(heartbeatRuns.id, input.runId), + eq(heartbeatRuns.companyId, input.agent.companyId), + eq(heartbeatRuns.agentId, input.agent.id), + ), + ) .limit(1); - const resolvedInstalledConnections = await filterResolvedGitHubConnectionsForRun({ - db: input.db, - companyId: input.agent.companyId, - agentId: input.agent.id, - responsibleUserId: runIdentity?.responsibleUserId ?? null, - connections: effective.installedConnections, - }); + const resolvedInstalledConnections = + await filterResolvedGitHubConnectionsForRun({ + db: input.db, + companyId: input.agent.companyId, + agentId: input.agent.id, + responsibleUserId: runIdentity?.responsibleUserId ?? null, + connections: effective.installedConnections, + }); const permittedConnectionIds = new Set([ ...effective.entries .filter((entry) => entry.effect === "include" && entry.connectionId) @@ -4572,30 +4577,36 @@ export async function createManagedMcpRunConfig(input: { const [runIdentity] = await input.db .select({ responsibleUserId: heartbeatRuns.responsibleUserId }) .from(heartbeatRuns) - .where(and( - eq(heartbeatRuns.id, input.runId), - eq(heartbeatRuns.companyId, input.agent.companyId), - eq(heartbeatRuns.agentId, input.agent.id), - )) + .where( + and( + eq(heartbeatRuns.id, input.runId), + eq(heartbeatRuns.companyId, input.agent.companyId), + eq(heartbeatRuns.agentId, input.agent.id), + ), + ) .limit(1); - const resolvedAvailableInstalls = await filterResolvedGitHubConnectionsForRun({ - db: input.db, - companyId: input.agent.companyId, - agentId: input.agent.id, - responsibleUserId: runIdentity?.responsibleUserId ?? null, - connections: installRows.filter( - (install) => - install.enabled && - install.status === "active" && - !["degraded", "failed", "error", "missing_secret"].includes( - install.healthStatus, - ), - ).map((install) => ({ - id: install.connectionId, - config: install.config, - transportConfig: install.transportConfig, - })), - }); + const resolvedAvailableInstalls = await filterResolvedGitHubConnectionsForRun( + { + db: input.db, + companyId: input.agent.companyId, + agentId: input.agent.id, + responsibleUserId: runIdentity?.responsibleUserId ?? null, + connections: installRows + .filter( + (install) => + install.enabled && + install.status === "active" && + !["degraded", "failed", "error", "missing_secret"].includes( + install.healthStatus, + ), + ) + .map((install) => ({ + id: install.connectionId, + config: install.config, + transportConfig: install.transportConfig, + })), + }, + ); const availableInstalledConnectionIds = new Set( resolvedAvailableInstalls.map((install) => install.id), ); @@ -6055,6 +6066,14 @@ function buildSessionConfigCategoryValues(input: { // the timestamp here makes every comment invalidate an otherwise reusable // task session. delete workspaceConfig.issueConfigRevisionAt; + // This row is runtime state, not requested configuration. It is absent + // before the first reusable run is realized and present on the next turn; + // fingerprinting that transition would rotate the native session exactly + // when the warm runner first becomes reusable. The requested/effective mode, + // project policy, and issue settings remain the configuration compatibility + // boundary; the reusable row and its evolving generation are state. + delete workspaceConfig.existingExecutionWorkspace; + delete workspaceConfig.reusableExecutionWorkspaceConfig; return { adapter: { adapterType: input.adapterType, @@ -8151,6 +8170,11 @@ export interface HeartbeatServiceOptions { nativeSessionBackendFactory?: ( execution: NativeExecutionInput, ) => NativeSessionBackend; + /** Test seam for observing the native-session shutdown boundary before lease destruction. */ + closeWarmNativeSessionsForRun?: (input: { + runId: string; + reason: string; + }) => Promise<{ closed: number; busy: number; failed: number }>; /** Test seam for changing a continuation issue at the final pre-dispatch boundary. */ beforeResolvedInteractionContinuationDispatchCheck?: (input: { runId: string; @@ -8731,6 +8755,57 @@ export function heartbeatService( sandboxResource: "keep_running" | "stop_and_reuse" | "destroy_after_turn"; }; }) { + if (input.providerResourceDisposition === "destroy") { + const closeResult = await ( + options.closeWarmNativeSessionsForRun ?? + (async ({ runId, reason }) => { + const environmentIds = await db + .selectDistinct({ environmentId: environmentLeases.environmentId }) + .from(environmentLeases) + .where( + and( + eq(environmentLeases.heartbeatRunId, runId), + eq(environmentLeases.status, "active"), + ), + ) + .then((rows) => + rows.flatMap((row) => + typeof row.environmentId === "string" && + row.environmentId.length > 0 + ? [row.environmentId] + : [], + ), + ); + const aggregate = { closed: 0, busy: 0, failed: 0 }; + for (const environmentId of environmentIds) { + const result = await closeWarmNativeSessionsForEnvironment({ + environmentId, + reason, + }); + aggregate.closed += result.closed; + aggregate.busy += result.busy; + aggregate.failed += result.failed; + } + return aggregate; + }) + )({ + runId: input.runId, + reason: "terminal heartbeat run destroyed its environment lease", + }).catch((err) => { + logger.warn( + { err, runId: input.runId }, + "failed to close warm native sessions before environment lease destruction", + ); + return { closed: 0, busy: 0, failed: 1 }; + }); + if (closeResult.busy > 0 || closeResult.failed > 0) { + logger.warn( + { runId: input.runId, warmNativeSessions: closeResult }, + "deferred environment lease destruction until warm native sessions close", + ); + return; + } + } const releaseResult = await envOrchestrator .releaseForRun({ heartbeatRunId: input.runId, @@ -10408,8 +10483,9 @@ export function heartbeatService( readNonEmptyString(context.issueId) ?? readNonEmptyString(context.taskId); const resolveGitAuth = createGitRemoteAuthProvider(db, agent.companyId, { issueId, - responsibleUserId: readNonEmptyString(context.responsibleUserId) - ?? readNonEmptyString(context.responsible_user_id), + responsibleUserId: + readNonEmptyString(context.responsibleUserId) ?? + readNonEmptyString(context.responsible_user_id), agentId: agent.id, }); const contextProjectId = readNonEmptyString(context.projectId); @@ -10668,8 +10744,9 @@ export function heartbeatService( readNonEmptyString(context.issueId) ?? readNonEmptyString(context.taskId); const resolveGitAuth = createGitRemoteAuthProvider(db, agent.companyId, { issueId, - responsibleUserId: readNonEmptyString(context.responsibleUserId) - ?? readNonEmptyString(context.responsible_user_id), + responsibleUserId: + readNonEmptyString(context.responsibleUserId) ?? + readNonEmptyString(context.responsible_user_id), agentId: agent.id, }); const { additionalWorkspaces, warnings, failures } = @@ -18760,12 +18837,16 @@ export function heartbeatService( issueId, explicitRunScopedSkillKeys: runScopedMentionedSkillKeys, }); - const githubRunAuth = await createGitRemoteAuthProvider(db, agent.companyId, { - issueId, - heartbeatRunId: run.id, - responsibleUserId, - agentId: agent.id, - })("https://github.com/paperclipai/credential-probe.git"); + const githubRunAuth = await createGitRemoteAuthProvider( + db, + agent.companyId, + { + issueId, + heartbeatRunId: run.id, + responsibleUserId, + agentId: agent.id, + }, + )("https://github.com/paperclipai/credential-probe.git"); const { resolvedConfig, secretKeys, secretManifest } = await resolveExecutionRunAdapterConfig({ companyId: agent.companyId, @@ -18784,10 +18865,16 @@ export function heartbeatService( routineEnv: routineEnvContext.env, secretsSvc, trustPreset, - ...(githubRunAuth ? { - trustedEnvProjection: githubRunAuth.env, - trustedEnvSecretKeys: ["GH_TOKEN", "GITHUB_TOKEN", GIT_CREDENTIAL_TOKEN_ENV_KEY], - } : {}), + ...(githubRunAuth + ? { + trustedEnvProjection: githubRunAuth.env, + trustedEnvSecretKeys: [ + "GH_TOKEN", + "GITHUB_TOKEN", + GIT_CREDENTIAL_TOKEN_ENV_KEY, + ], + } + : {}), requiredScopedEnvBinding: pushCapabilityPreflightRequired ? { keys: [...PUSH_CAPABILITY_ENV_KEYS], @@ -19223,6 +19310,63 @@ export function heartbeatService( const resolvedProjectWorkspaceId = issueRef?.projectWorkspaceId ?? resolvedWorkspace.workspaceId ?? null; let persistedExecutionWorkspace: ExecutionWorkspace | null = null; + let issueExecutionWorkspaceIdForRun = + issueRef?.executionWorkspaceId ?? null; + let issueProjectWorkspaceIdForRun = issueRef?.projectWorkspaceId ?? null; + let issueExecutionWorkspacePreferenceForRun = + issueRef?.executionWorkspacePreference ?? null; + let issueExecutionWorkspaceModeForRun = + issueExecutionWorkspaceSettings?.mode ?? null; + const warmReusableExecutionWorkspace = + selectedEnvironmentForConfig?.driver === "sandbox" && + selectedEnvironmentConfigForFingerprint.reuseLease === true && + selectedEnvironmentConfigForFingerprint.runnerLifecycleMode === "warm"; + const bindIssueToPersistedExecutionWorkspace = async ( + workspace: ExecutionWorkspace | null, + ) => { + if (!issueId || !workspace || nativeRecoveryExecutionWorkspaceId) { + return; + } + const nextIssueWorkspaceMode = + issueExecutionWorkspaceModeForPersistedWorkspace(workspace.mode) ?? + "agent_default"; + const shouldSwitchIssueToExistingWorkspace = + issueRef?.executionWorkspacePreference === "reuse_existing" || + requestedExecutionWorkspaceMode === "isolated_workspace" || + requestedExecutionWorkspaceMode === "operator_branch" || + warmReusableExecutionWorkspace; + const nextIssuePatch: Record = {}; + if (issueExecutionWorkspaceIdForRun !== workspace.id) { + nextIssuePatch.executionWorkspaceId = workspace.id; + } + if ( + resolvedProjectWorkspaceId && + issueProjectWorkspaceIdForRun !== resolvedProjectWorkspaceId + ) { + nextIssuePatch.projectWorkspaceId = resolvedProjectWorkspaceId; + } + if ( + shouldSwitchIssueToExistingWorkspace && + (issueExecutionWorkspacePreferenceForRun !== "reuse_existing" || + issueExecutionWorkspaceModeForRun !== nextIssueWorkspaceMode) + ) { + nextIssuePatch.executionWorkspacePreference = "reuse_existing"; + nextIssuePatch.executionWorkspaceSettings = { + ...(issueExecutionWorkspaceSettings ?? {}), + mode: nextIssueWorkspaceMode, + }; + } + if (Object.keys(nextIssuePatch).length > 0) { + await issuesSvc.update(issueId, nextIssuePatch); + issueExecutionWorkspaceIdForRun = workspace.id; + issueProjectWorkspaceIdForRun = + resolvedProjectWorkspaceId ?? issueProjectWorkspaceIdForRun; + if (shouldSwitchIssueToExistingWorkspace) { + issueExecutionWorkspacePreferenceForRun = "reuse_existing"; + issueExecutionWorkspaceModeForRun = nextIssueWorkspaceMode; + } + } + }; const baseExecutionWorkspaceMetadata = mergeExecutionWorkspaceMetadataForPersistence({ existingMetadata: @@ -19440,40 +19584,7 @@ export function heartbeatService( }, ); } - if ( - issueId && - persistedExecutionWorkspace && - !nativeRecoveryExecutionWorkspaceId - ) { - const nextIssueWorkspaceMode = - issueExecutionWorkspaceModeForPersistedWorkspace( - persistedExecutionWorkspace.mode, - ); - const shouldSwitchIssueToExistingWorkspace = - issueRef?.executionWorkspacePreference === "reuse_existing" || - requestedExecutionWorkspaceMode === "isolated_workspace" || - requestedExecutionWorkspaceMode === "operator_branch"; - const nextIssuePatch: Record = {}; - if (issueRef?.executionWorkspaceId !== persistedExecutionWorkspace.id) { - nextIssuePatch.executionWorkspaceId = persistedExecutionWorkspace.id; - } - if ( - resolvedProjectWorkspaceId && - issueRef?.projectWorkspaceId !== resolvedProjectWorkspaceId - ) { - nextIssuePatch.projectWorkspaceId = resolvedProjectWorkspaceId; - } - if (shouldSwitchIssueToExistingWorkspace) { - nextIssuePatch.executionWorkspacePreference = "reuse_existing"; - nextIssuePatch.executionWorkspaceSettings = { - ...(issueExecutionWorkspaceSettings ?? {}), - mode: nextIssueWorkspaceMode, - }; - } - if (Object.keys(nextIssuePatch).length > 0) { - await issuesSvc.update(issueId, nextIssuePatch); - } - } + await bindIssueToPersistedExecutionWorkspace(persistedExecutionWorkspace); if (persistedExecutionWorkspace) { context.executionWorkspaceId = persistedExecutionWorkspace.id; await db @@ -19612,6 +19723,11 @@ export function heartbeatService( }; persistedExecutionWorkspace = realizationResult.persistedExecutionWorkspace; + // A sandbox realization may materialize or replace the durable workspace + // after the host-side provisioning boundary above. Bind that final ID to + // the issue before dispatch so warm turns reuse the exact same workspace + // and lease scope instead of silently creating a per-run replacement. + await bindIssueToPersistedExecutionWorkspace(persistedExecutionWorkspace); const workspaceRealization = realizationResult.workspaceRealization; const executionTarget = realizationResult.executionTarget; const remoteExecution = realizationResult.remoteExecution; @@ -20861,6 +20977,7 @@ export function heartbeatService( target: executionTarget, lease: activeEnvironmentLease.lease, restartRecovery: runOptions.nativeRestartRecovery, + sameRunRecovery: Boolean(runOptions.nativeLeaseOwner), resourceDisposition: providerResourceDispositionForRun, }); } else { @@ -22207,6 +22324,7 @@ export function heartbeatService( .select({ nextAttemptAt: nativeRunFinalizations.nextAttemptAt, attempt: nativeRunFinalizations.attempt, + failureDetail: nativeRunFinalizations.failureDetail, }) .from(nativeRunFinalizations) .where(eq(nativeRunFinalizations.runId, run.id)) @@ -22225,6 +22343,12 @@ export function heartbeatService( nextAttemptAt: coordinator?.nextAttemptAt?.toISOString() ?? null, fallbackSuppressed: true, retryReasonCode, + // The executor has already redacted and bounded this diagnostic + // before persisting it. Retain it on the immutable transition + // event as well: a same-run retry reopens the log stream, so the + // first attempt's stderr must not be the only explanation for + // why a live warm runner was replaced. + failureDetail: coordinator?.failureDetail ?? null, }, }).catch(() => undefined); if (coordinator?.nextAttemptAt) { @@ -22415,7 +22539,13 @@ export function heartbeatService( livenessRun, agent, ); - await releaseIssueExecutionAndPromote(livenessRun); + await releaseIssueExecutionAndPromote(livenessRun, { + // Native recovery owns the original heartbeat run through + // exhaustion. Once its durable coordinator has classified a + // terminal failure, generic issue recovery must not create a + // replacement retryOfRunId chain for the same provider work. + suppressImmediateRecovery: nativeTerminalFailureCode !== null, + }); await handleIssueReviewPathDisposition(livenessRun); await updateRuntimeState( diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 5c26081ab3..290adaa6b5 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -1989,14 +1989,31 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti "Interaction has already been resolved", ); } - if (isNativeCompletionReview(lockedCurrent)) { - await issueService(db).update(args.issue.id, { - status: "todo", - assigneeAgentId: issueContext.assigneeAgentId, - assigneeUserId: null, - actorAgentId: args.actor.agentId ?? null, - actorUserId: args.actor.userId ?? null, - }, tx); + const rejectedPlanNeedsRevision = + lockedCurrent.kind === "request_confirmation" && + readAcceptedPlanConfirmationTarget( + lockedCurrent.payload, + issueContext.id, + )?.key === "plan"; + const shouldResumeReviewedIssue = + issueContext.status === "in_review" && + (lockedCurrent.continuationPolicy === "wake_assignee" || + rejectedPlanNeedsRevision); + if ( + isNativeCompletionReview(lockedCurrent) || + shouldResumeReviewedIssue + ) { + await issueService(db).update( + args.issue.id, + { + status: "todo", + assigneeAgentId: issueContext.assigneeAgentId, + assigneeUserId: null, + actorAgentId: args.actor.agentId ?? null, + actorUserId: args.actor.userId ?? null, + }, + tx, + ); } else { await touchIssue(tx, args.issue.id); } diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 8d363ab17d..fe9a2fd325 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -8963,7 +8963,7 @@ export function issueService(db: Db) { }); }, - addComment: async ( + addComment: async function addComment( issueId: string, body: string, actor: { @@ -8981,7 +8981,21 @@ export function issueService(db: Db) { createdAt?: Date | string | null; }, dbOrTx: any = db, - ) => { + ): Promise { + if (dbOrTx === db && actor.runId) { + return db.transaction(async (tx) => { + // Serialize run-authored comments on the issue so a provider retry + // cannot publish the same visible result twice. This needs no schema + // change: the issue row is the transaction fence, and the recursive + // call below performs the lookup and insert while holding it. + await tx + .select({ id: issues.id }) + .from(issues) + .where(eq(issues.id, issueId)) + .for("update"); + return addComment(issueId, body, actor, options, tx); + }); + } const issue = await dbOrTx .select({ companyId: issues.companyId }) .from(issues) @@ -9016,11 +9030,45 @@ export function issueService(db: Db) { actor.onBehalfOfUserId, ) : null; - const metadata = issueCommentMetadataSchema.nullable().parse( - actor.agentId - ? withAgentCommentAuthorizationMetadata(options?.metadata ?? null, options?.authorizationReason) - : options?.metadata ?? null, - ); + const metadata = issueCommentMetadataSchema + .nullable() + .parse( + actor.agentId + ? withAgentCommentAuthorizationMetadata( + options?.metadata ?? null, + options?.authorizationReason, + ) + : (options?.metadata ?? null), + ); + if (createdByRunId) { + const existing = await dbOrTx + .select() + .from(issueComments) + .where( + and( + eq(issueComments.companyId, issue.companyId), + eq(issueComments.issueId, issueId), + eq(issueComments.createdByRunId, createdByRunId), + eq(issueComments.authorType, authorType), + actor.agentId + ? eq(issueComments.authorAgentId, actor.agentId) + : isNull(issueComments.authorAgentId), + eq(issueComments.body, redactedBody), + isNull(issueComments.deletedAt), + ), + ) + .orderBy(issueComments.createdAt, issueComments.id) + .limit(1) + .then( + (rows: Array) => rows[0] ?? null, + ); + if (existing) { + return redactIssueComment( + existing, + currentUserRedactionOptions.enabled, + ); + } + } const [comment] = await dbOrTx .insert(issueComments) .values({ diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 10a981df18..686001f88f 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -167,7 +167,9 @@ import { buildNativeProviderEnvironment, buildNativeHarnessBackupManifest, cancelNativeSession, + closeWarmNativeSessionsForEnvironment, createGovernedWaitEventObservation, + createRemoteRunnerProcessLauncher, createRunnerdBackend, executePaperclipNativeSession, getNativeSessionSteeringState, @@ -181,6 +183,7 @@ import { mayUsePreinstalledRunnerArtifact, nativeUsageCostUsd, normalizeNativeUsage, + parseRemoteRunnerProcessIdentity, readRemoteProviderPackManifest, providerSessionIdentityFromDurableProviderState, providerSessionIdentityTransitionIsAllowed, @@ -202,6 +205,285 @@ import { shouldRestoreNativeHarnessBackupIntoSandbox, } from "./native-session-executor.js"; +describe("remote runner process supervision", () => { + it("detaches runnerd from the provider RPC and monitors its durable identity", async () => { + let launchNonce = ""; + const execute = vi.fn( + async (input: { + command?: string; + args?: string[]; + timeoutMs?: number; + useSession?: boolean; + bypassSession?: boolean; + }) => { + const label = input.args?.[2]; + if (label === "paperclip-runner-launch") { + launchNonce = input.args?.[4] ?? ""; + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + }; + } + if (label === "paperclip-runner-process-identity") { + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: `${launchNonce}\n4321\n2026-09-06T00:00:00.000Z\nrunner-remote\n`, + stderr: "", + }; + } + if (label === "paperclip-runner-monitor") { + return { + exitCode: 3, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + }; + } + if (label === "paperclip-runner-diagnostics") { + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: "paperclip-runnerd: provider transport closed", + stderr: "", + }; + } + if (input.command === "sh" && input.args?.[1]?.includes("base64")) { + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: Buffer.from( + JSON.stringify({ + lifecycle: "ready", + diagnostics: ["last durable diagnostic"], + }), + ).toString("base64"), + stderr: "", + }; + } + if (label === "paperclip-runner-signal") { + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + }; + } + throw new Error(`unexpected remote command: ${label ?? "missing"}`); + }, + ); + const onSpawn = vi.fn(async () => undefined); + const launcher = createRemoteRunnerProcessLauncher({ + target: { + kind: "remote", + transport: "sandbox", + environmentId: "environment-remote", + leaseId: "lease-remote", + remoteCwd: "/workspace", + }, + runner: { execute } as never, + remoteBinary: "/runtime/paperclip-runnerd", + processIdentityPath: "/runtime/runner-process.identity", + stateDirectory: "/runtime", + diagnosticsDirectory: "/runtime/diagnostics", + runnerInstanceId: "runner-remote", + onSpawn, + }); + + const handle = launcher({ + command: "/controller/paperclip-runnerd", + args: ["--runner-id", "runner-remote"], + cwd: "/controller", + environment: {}, + }); + await expect(handle.completion).resolves.toMatchObject({ + code: null, + stderr: "paperclip-runnerd: provider transport closed", + }); + + const launch = execute.mock.calls.find( + ([input]) => input.args?.[2] === "paperclip-runner-launch", + )?.[0]; + expect(launch).toMatchObject({ + timeoutMs: 20_000, + bypassSession: true, + }); + expect(launch?.useSession).toBeUndefined(); + expect(launch?.args?.[1]).toContain("nohup setsid"); + expect(launch?.args?.[6]).toContain('"$$"'); + expect(launch?.args?.[6]).toContain('exec "$@"'); + expect(launch?.args?.[7]).toBe("/runtime/diagnostics"); + expect(launch?.args).toContain("/runtime/diagnostics"); + expect(onSpawn).toHaveBeenCalledExactlyOnceWith({ + pid: 4321, + processGroupId: null, + startedAt: "2026-09-06T00:00:00.000Z", + }); + expect(handle.child.pid).toBe(4321); + + expect(handle.child.kill("SIGKILL")).toBe(true); + await vi.waitFor(() => + expect( + execute.mock.calls.some( + ([input]) => + input.args?.[2] === "paperclip-runner-signal" && + input.args?.[1]?.includes('kill -KILL "$expected_pid"'), + ), + ).toBe(true), + ); + }); + + it("terminates a detached runner when its process identity cannot be adopted", async () => { + vi.useFakeTimers(); + try { + let launchNonce = ""; + const execute = vi.fn( + async (input: { command?: string; args?: string[] }) => { + const label = input.args?.[2]; + if (label === "paperclip-runner-launch") { + launchNonce = input.args?.[4] ?? ""; + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + }; + } + if (label === "paperclip-runner-process-identity") { + return { + exitCode: 3, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + }; + } + if (label === "paperclip-runner-identity-failure-cleanup") { + expect(input.args?.[4]).toBe(launchNonce); + return { + exitCode: 0, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + }; + } + throw new Error(`unexpected remote command: ${label ?? "missing"}`); + }, + ); + const launcher = createRemoteRunnerProcessLauncher({ + target: { + kind: "remote", + transport: "sandbox", + environmentId: "environment-remote", + leaseId: "lease-remote", + remoteCwd: "/workspace", + }, + runner: { execute } as never, + remoteBinary: "/runtime/paperclip-runnerd", + processIdentityPath: "/runtime/runner-process.identity", + stateDirectory: "/runtime", + diagnosticsDirectory: "/runtime/diagnostics", + runnerInstanceId: "runner-remote", + }); + + const handle = launcher({ + command: "/controller/paperclip-runnerd", + args: ["--runner-id", "runner-remote"], + cwd: "/controller", + environment: {}, + }); + const completion = expect(handle.completion).rejects.toThrow( + "runner_remote_process_identity_unavailable", + ); + await vi.advanceTimersByTimeAsync(20_100); + await completion; + + const cleanup = execute.mock.calls.find( + ([call]) => + call.args?.[2] === "paperclip-runner-identity-failure-cleanup", + )?.[0]; + expect(cleanup).toMatchObject({ + bypassSession: true, + timeoutMs: 20_000, + }); + expect(cleanup?.args?.[1]).toContain('test "$nonce" = "$expected_nonce"'); + expect(cleanup?.args?.[1]).toContain('grep -Fqx -- "--runner-id"'); + expect(cleanup?.args?.[1]).toContain('kill -TERM -- "$signal_target"'); + expect(cleanup?.args?.[1]).toContain('kill -KILL -- "$signal_target"'); + expect(cleanup?.args?.[1]?.indexOf("rm -f --")).toBeGreaterThan( + cleanup?.args?.[1]?.indexOf('kill -0 "$pid" 2>/dev/null && exit 5') ?? + Number.MAX_SAFE_INTEGER, + ); + } finally { + vi.useRealTimers(); + } + }); + + it("reports cleanup failure when a detached runner cannot be safely identified", async () => { + vi.useFakeTimers(); + try { + const execute = vi.fn( + async (input: { command?: string; args?: string[] }) => { + const label = input.args?.[2]; + return { + exitCode: + label === "paperclip-runner-launch" + ? 0 + : label === "paperclip-runner-process-identity" + ? 3 + : label === "paperclip-runner-identity-failure-cleanup" + ? 4 + : 1, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + }; + }, + ); + const launcher = createRemoteRunnerProcessLauncher({ + target: { + kind: "remote", + transport: "sandbox", + environmentId: "environment-remote", + leaseId: "lease-remote", + remoteCwd: "/workspace", + }, + runner: { execute } as never, + remoteBinary: "/runtime/paperclip-runnerd", + processIdentityPath: "/runtime/runner-process.identity", + stateDirectory: "/runtime", + diagnosticsDirectory: "/runtime/diagnostics", + runnerInstanceId: "runner-remote", + }); + + const handle = launcher({ + command: "/controller/paperclip-runnerd", + args: ["--runner-id", "runner-remote"], + cwd: "/controller", + environment: {}, + }); + const completion = expect(handle.completion).rejects.toThrow( + "runner_remote_process_identity_unavailable_cleanup_failed", + ); + await vi.advanceTimersByTimeAsync(20_100); + await completion; + } finally { + vi.useRealTimers(); + } + }); +}); + describe("native incomplete-bootstrap evidence", () => { it("requires zero connections, zero events, and only untouched bootstrap commands", async () => { const root = await mkdtemp(join(tmpdir(), "native-bootstrap-evidence-")); @@ -1346,6 +1628,40 @@ describe("remote preinstalled executable discovery", () => { }); describe("remote runner build metadata", () => { + it("accepts only an exact remote runner process identity marker", () => { + const expected = { + nonce: "launch-nonce", + runnerInstanceId: "runner-remote-process", + }; + expect( + parseRemoteRunnerProcessIdentity( + "launch-nonce\n4102\n2026-09-06T04:20:30.123Z\nrunner-remote-process\n", + expected, + ), + ).toEqual({ + pid: 4102, + startedAt: "2026-09-06T04:20:30.123Z", + }); + expect( + parseRemoteRunnerProcessIdentity( + "stale-nonce\n4102\n2026-09-06T04:20:30.123Z\nrunner-remote-process\n", + expected, + ), + ).toBeNull(); + expect( + parseRemoteRunnerProcessIdentity( + "launch-nonce\n4102\n2026-09-06T04:20:30.123Z\nwrong-runner\n", + expected, + ), + ).toBeNull(); + expect( + parseRemoteRunnerProcessIdentity( + "launch-nonce\nnot-a-pid\n2026-09-06T04:20:30.123Z\nrunner-remote-process\n", + expected, + ), + ).toBeNull(); + }); + const current = { schema: "paperclip-runner/runnerd-build-metadata/v1", binaryName: "paperclip-runnerd", @@ -2716,6 +3032,66 @@ describe("native session same-turn steering", () => { }); describe("native warm session supervision", () => { + it("closes an idle warm session before its remote environment is destroyed", async () => { + const close = vi.fn(async () => undefined); + const warmExecution = { + ...execution, + binding: { + ...execution.binding, + runId: "run-warm-environment-delete", + executionWorkspaceId: "workspace-warm-environment-delete", + }, + session: { + ...execution.session, + normalizedSessionId: "session-warm-environment-delete", + lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 60_000 }, + }, + } as NativeExecutionInputV1; + state.execute.mockReset().mockImplementationOnce(async (options) => { + options.onSession?.({ close }); + return { + result: { summary: "completed" }, + terminal: { runTerminalState: "succeeded" }, + turnId: "turn-warm-environment-delete", + normalizedSessionId: warmExecution.session.normalizedSessionId, + providerSessionId: "provider-warm-environment-delete", + driverKind: "test", + driverVersion: "1", + nativeEventCount: 1, + highestContiguousSourceSeq: 1, + usage: null, + }; + }); + + await executePaperclipNativeSession({ + db: leaseDb(warmExecution), + execution: warmExecution, + runnerInstanceId: "runner-warm-environment-delete", + runnerExecutionTarget: { + kind: "remote", + transport: "sandbox", + environmentId: "environment-warm-delete", + remoteCwd: "/tmp/warm-environment-delete", + }, + }); + + await expect( + closeWarmNativeSessionsForEnvironment({ + environmentId: "other-environment", + reason: "environment deleted", + }), + ).resolves.toEqual({ closed: 0, busy: 0, failed: 0 }); + await expect( + closeWarmNativeSessionsForEnvironment({ + environmentId: "environment-warm-delete", + reason: "environment deleted", + }), + ).resolves.toEqual({ closed: 1, busy: 0, failed: 0 }); + expect(close).toHaveBeenCalledExactlyOnceWith({ + reason: "environment deleted", + }); + }); + it("preserves the active turn when a warm checkpoint resumes the same run", async () => { const stateBase = await mkdtemp( join(tmpdir(), "paperclip-warm-same-run-recovery-"), @@ -2852,11 +3228,13 @@ describe("native warm session supervision", () => { .mockReset() .mockImplementationOnce(async (options) => { expect(options.existingSession).toBeUndefined(); + expect(options.semanticResultTerminalGraceMs).toBe(30_000); options.onSession?.(sharedSession); return result; }) .mockImplementationOnce(async (options) => { expect(options.existingSession).toBe(sharedSession); + expect(options.semanticResultTerminalGraceMs).toBe(30_000); return result; }); @@ -2880,6 +3258,67 @@ describe("native warm session supervision", () => { ); }); + it("does not offer a quarantined warm session to the next run", async () => { + const close = vi.fn(async () => undefined); + const quarantinedSession = { close }; + const first = { + ...execution, + binding: { + ...execution.binding, + runId: "run-native-warm-quarantined-first", + executionWorkspaceId: "workspace-native-warm-quarantined", + }, + session: { + ...execution.session, + normalizedSessionId: "session-native-warm-quarantined", + lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 60_000 }, + }, + } as NativeExecutionInputV1; + const second = { + ...first, + binding: { + ...first.binding, + runId: "run-native-warm-quarantined-second", + }, + } as NativeExecutionInputV1; + const result = { + result: { summary: "completed" }, + terminal: { runTerminalState: "succeeded" }, + turnId: "turn", + normalizedSessionId: first.session.normalizedSessionId, + providerSessionId: "provider-native-warm-quarantined", + driverKind: "test", + driverVersion: "1", + nativeEventCount: 1, + highestContiguousSourceSeq: 1, + usage: null, + }; + state.execute + .mockReset() + .mockImplementationOnce(async (options) => { + expect(options.existingSession).toBeUndefined(); + options.onSession?.(quarantinedSession); + options.onSession?.(null); + return result; + }) + .mockImplementationOnce(async (options) => { + expect(options.existingSession).toBeUndefined(); + return result; + }); + + await executePaperclipNativeSession({ + db: leaseDb(first), + execution: first, + runnerInstanceId: "runner-native-warm-quarantined", + }); + await executePaperclipNativeSession({ + db: leaseDb(second), + execution: second, + runnerInstanceId: "runner-native-warm-quarantined", + }); + expect(close).not.toHaveBeenCalled(); + }); + it("reattaches a live runnerd warm session under a fresh run authority", async () => { const stateBase = await mkdtemp( join(tmpdir(), "paperclip-runnerd-warm-authority-"), @@ -2907,13 +3346,20 @@ describe("native warm session supervision", () => { normalizedSessionId: "session-runnerd-warm-authority", driverKind: "codex_app_server" as const, protocolVersion: 1 as const, - lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 20 }, + lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 500 }, }, } as NativeExecutionInputV1; const second = { ...first, binding: { ...first.binding, runId: "run-runnerd-warm-second" }, } as NativeExecutionInputV1; + const remoteTarget = { + kind: "remote" as const, + transport: "sandbox" as const, + environmentId: "environment-runnerd-warm-authority", + remoteCwd: "/home/daytona/paperclip-workspace", + runner: { execute: vi.fn() }, + } as never; const result = { result: { summary: "completed" }, terminal: { runTerminalState: "succeeded" }, @@ -2956,6 +3402,7 @@ describe("native warm session supervision", () => { execution: first, runnerInstanceId: "runner-runnerd-warm", useRunnerd: true, + runnerExecutionTarget: remoteTarget, }); const scopedRoots = (await readdir(stateBase, { withFileTypes: true })) .filter( @@ -2971,15 +3418,10 @@ describe("native warm session supervision", () => { environmentLeaseId: first.binding.executionWorkspaceId, }; await mkdir(join(durableRoot, "control-plane"), { recursive: true }); - await mkdir(join(durableRoot, "runner"), { recursive: true }); await writeFile( join(durableRoot, "control-plane", "control-plane-state.json"), JSON.stringify(durableControlPlaneState(durableIdentity)), ); - await writeFile( - join(durableRoot, "runner", "runner-state.json"), - JSON.stringify(durableRunnerState(durableIdentity, "suspended")), - ); const continuationDb = { ...leaseDb(second), select: () => ({ @@ -3001,13 +3443,18 @@ describe("native warm session supervision", () => { execution: second, runnerInstanceId: "runner-runnerd-warm", useRunnerd: true, + runnerExecutionTarget: remoteTarget, }); expect(firstClose).not.toHaveBeenCalled(); - await vi.waitFor(() => expect(firstClose).toHaveBeenCalledWith({ - reason: "warm native session idle timeout", - }), { - timeout: 500, - }); + await vi.waitFor( + () => + expect(firstClose).toHaveBeenCalledWith({ + reason: "warm native session idle timeout", + }), + { + timeout: 1_500, + }, + ); } finally { if (previousStateDirectory === undefined) { delete process.env.PAPERCLIP_RUNNER_STATE_DIR; @@ -3760,6 +4207,7 @@ describe("runnerd provider runtime wiring", () => { state.createBackend.mock.calls[0]![1].codexTransportFactory!(); expect(state.createTransport).toHaveBeenCalledWith( expect.objectContaining({ + externallySandboxed: true, environment: expect.objectContaining({ PAPERCLIP_RUNNER_EXTERNAL_SANDBOX: "1", }), diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index a9094a4606..80d850afbe 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -186,6 +186,12 @@ const TERMINAL_HEARTBEAT_RUN_STATUSES = new Set([ const NATIVE_SESSION_EXECUTION_LEASE_TTL_MS = 20 * 60_000; const NATIVE_SESSION_EXECUTION_LEASE_RENEW_INTERVAL_MS = 5 * 60_000; const NATIVE_SESSION_CANCELLATION_CLEANUP_GRACE_MS = 2_000; +// A reusable provider must publish its terminal suffix before the next run can +// rotate PRP authority. Remote Codex can take more than the ordinary five-second +// result grace to flush its final answer over Daytona, so retain the bounded +// turn long enough to reach a naturally quiescent, reusable state. This adds no +// delay when the provider terminates normally. +const NATIVE_WARM_SEMANTIC_RESULT_TERMINAL_GRACE_MS = 30_000; const NATIVE_RUNTIME_REQUEST_RESOLUTION_CACHE_MAX = 256; type NativeRuntimeRequestResolution = { runId: string; @@ -258,7 +264,10 @@ function clearNativeRuntimeRequestResolutions(runId: string): void { type WarmNativeSession = { session: NativeSession; + ownerToken: symbol; configDigest: string; + companyId: string; + environmentId: string | null; busy: boolean; idleTimer: ReturnType | null; lastActivityAt: string; @@ -266,6 +275,42 @@ type WarmNativeSession = { const warmNativeSessions = new Map(); +/** + * Close idle native sessions before an operator destroys their remote + * environment. A warm runner owns a long-lived sandbox command stream, so the + * provider cannot safely delete that sandbox until the session has closed the + * stream. Busy sessions are reported instead of interrupted; the environment + * delete guard can then fail closed while their heartbeat run is still live. + */ +export async function closeWarmNativeSessionsForEnvironment(input: { + environmentId: string; + reason: string; +}): Promise<{ closed: number; busy: number; failed: number }> { + let closed = 0; + let busy = 0; + let failed = 0; + for (const [sessionId, entry] of [...warmNativeSessions]) { + if (entry.environmentId !== input.environmentId) { + continue; + } + if (entry.busy) { + busy += 1; + continue; + } + if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); + // Remove ownership before awaiting close so a racing continuation cannot + // adopt a session whose transport is already shutting down. + warmNativeSessions.delete(sessionId); + try { + await entry.session.close({ reason: input.reason }); + closed += 1; + } catch { + failed += 1; + } + } + return { closed, busy, failed }; +} + function readBoundedNativeFile( path: string, maxBytes: number, @@ -1453,6 +1498,7 @@ function runnerdAuthorityLifecycleWithVerifiedBackup(input: { type PriorRunnerdStateVerification = | "verified" + | "retained_warm_runner" | "active" | "authority_indeterminate" | "scope_mismatch" @@ -1465,6 +1511,7 @@ async function verifyPriorRunnerdStateForSessionScope(input: { identity: RunnerdDurableIdentity; execution: NativeExecutionInput; allowVerifiedBackup: boolean; + allowRetainedWarmRunner: boolean; }): Promise { let priorRun: { status: string; @@ -1505,9 +1552,24 @@ async function verifyPriorRunnerdStateForSessionScope(input: { nativeSessionScopeKey(input.execution); if (!sameScope) return "scope_mismatch"; const lifecycle = runnerdAuthorityLifecycleWithVerifiedBackup(input); - return lifecycle === "suspended" - ? "verified" - : "terminal_state_indeterminate"; + if (lifecycle === "suspended") return "verified"; + const directLifecycle = runnerdAuthorityLifecycle( + input.root, + input.identity, + ); + if ( + input.allowRetainedWarmRunner && + (lifecycle === "not_suspended" || + // A remote runner keeps runner-state.json in the sandbox rather than + // beside the controller's PRP journal. During a genuinely warm handoff + // there is intentionally no suspended failover backup yet. The exact + // idle in-memory session owner is the authority for this one case; + // after a restart that owner is absent and this remains fail-closed. + (input.allowVerifiedBackup && directLifecycle === "absent")) + ) { + return "retained_warm_runner"; + } + return "terminal_state_indeterminate"; } catch { return "authority_indeterminate"; } @@ -1517,6 +1579,7 @@ async function migrateRunnerdStateRootForExecution(input: { db: Db; execution: NativeExecutionInput; allowVerifiedBackup: boolean; + allowRetainedWarmRunner: boolean; restartRecovery?: NativeRestartRecoveryClaim; }): Promise { const scoped = scopedRunnerdStateRoot(input.execution); @@ -1568,8 +1631,12 @@ async function migrateRunnerdStateRootForExecution(input: { identity, execution: input.execution, allowVerifiedBackup: input.allowVerifiedBackup, + allowRetainedWarmRunner: input.allowRetainedWarmRunner, }); - if (verification !== "verified") { + if ( + verification !== "verified" && + verification !== "retained_warm_runner" + ) { if (verification !== "active" && verification !== "unavailable") { quarantineRunnerdStateRoot( scoped, @@ -1606,8 +1673,12 @@ async function migrateRunnerdStateRootForExecution(input: { identity, execution: input.execution, allowVerifiedBackup: input.allowVerifiedBackup, + allowRetainedWarmRunner: input.allowRetainedWarmRunner, }); - if (verification !== "verified") { + if ( + verification !== "verified" && + verification !== "retained_warm_runner" + ) { if (verification === "terminal_state_indeterminate") { // The database proves this ambiguous legacy path belongs to the same // full session scope and its owner is terminal, so it is now safe to @@ -1802,6 +1873,24 @@ function nativeSessionConfigDigest(execution: NativeExecutionInput): string { .digest("hex")}`; } +function hasIdleWarmNativeSessionOwner(input: { + execution: NativeExecutionInput; + runnerExecutionTarget?: AdapterExecutionTarget | null; +}): boolean { + if (input.execution.session.lifecyclePolicy.mode !== "warm") return false; + const entry = warmNativeSessions.get(nativeSessionScopeKey(input.execution)); + if (!entry || entry.busy) return false; + const environmentId = + input.runnerExecutionTarget?.kind === "remote" + ? (input.runnerExecutionTarget.environmentId ?? null) + : null; + return ( + entry.companyId === input.execution.binding.companyId && + entry.environmentId === environmentId && + entry.configDigest === nativeSessionConfigDigest(input.execution) + ); +} + function nativeHarnessEnvironmentFingerprint( execution: NativeExecutionInput, ): string { @@ -2447,11 +2536,14 @@ function loadWarmNativeCheckpoint( async function releaseWarmNativeSession( sessionId: string, + ownerToken: symbol, idleTimeoutMs: number, failed: boolean, ): Promise { const entry = warmNativeSessions.get(sessionId); - if (!entry) return; + // A late completion or cleanup callback from an older execution must never + // release a replacement that has since claimed the same logical session. + if (!entry || entry.ownerToken !== ownerToken) return; entry.busy = false; entry.lastActivityAt = new Date().toISOString(); if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); @@ -2464,9 +2556,13 @@ async function releaseWarmNativeSession( } entry.idleTimer = setTimeout(() => { const current = warmNativeSessions.get(sessionId); - if (!current || current.busy) return; + // clearTimeout cannot revoke an already-queued callback. The entry object + // is the idle timer's ownership fence across a later warm acquisition. + if (current !== entry || current.busy) return; warmNativeSessions.delete(sessionId); - void current.session.close({ reason: "warm native session idle timeout" }); + void current.session + .close({ reason: "warm native session idle timeout" }) + .catch(() => undefined); }, idleTimeoutMs); entry.idleTimer.unref(); } @@ -2626,7 +2722,9 @@ export async function nativeProviderRecoveryEvidence(input: { .where( and( eq(heartbeatRunEvents.runId, input.runId), - inArray(heartbeatRunEvents.eventType, [...PROVIDER_DURABLE_EVENT_TYPES]), + inArray(heartbeatRunEvents.eventType, [ + ...PROVIDER_DURABLE_EVENT_TYPES, + ]), ), ) .limit(1); @@ -3661,6 +3759,11 @@ async function executePaperclipNativeSessionWithinScope( allowVerifiedBackup: input.runnerExecutionTarget?.kind === "remote" && input.runnerExecutionTarget.transport === "sandbox", + // A retained warm runner is deliberately still ready rather than + // suspended. Only the exact idle in-process owner may rotate that + // prior-run authority; after a hard restart the map is empty and the + // durable-state verifier continues to require a suspended runner. + allowRetainedWarmRunner: hasIdleWarmNativeSessionOwner(input), restartRecovery: input.restartRecovery, }); } @@ -4156,6 +4259,9 @@ async function executePaperclipNativeSessionWithinScope( lifecyclePolicy.mode === "warm" ? nativeSessionConfigDigest(input.execution) : null; + const warmSessionOwnerToken = Symbol( + `native-warm-session:${input.execution.binding.runId}`, + ); let existingWarmSession: NativeSession | undefined; let persistedWarmSession: PersistedNativeSession | null | undefined; if (warmSessionId !== null && warmConfigDigest !== null) { @@ -4175,6 +4281,9 @@ async function executePaperclipNativeSessionWithinScope( } else { if (entry.busy) throw new Error("native_session_supervisor_busy"); entry.busy = true; + entry.ownerToken = warmSessionOwnerToken; + entry.environmentId = + input.runnerExecutionTarget?.environmentId ?? null; if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); entry.idleTimer = null; existingWarmSession = entry.session; @@ -4311,6 +4420,10 @@ async function executePaperclipNativeSessionWithinScope( existingSession: existingWarmSession, persistedSession: persistedWarmSession, keepSessionOpen: warmSessionId !== null, + semanticResultTerminalGraceMs: + warmSessionId === null + ? undefined + : NATIVE_WARM_SEMANTIC_RESULT_TERMINAL_GRACE_MS, requireSessionCloseBeforeReturn: runnerdBackend !== null && input.runnerExecutionTarget?.kind === "remote", @@ -4323,6 +4436,21 @@ async function executePaperclipNativeSessionWithinScope( snapshot, ) : undefined, + onPostCompletionEnrichmentFailure: async ({ stage, error }) => { + const detail = redactSensitiveText( + error instanceof Error ? error.message : String(error), + ).slice(-4_096); + await input.onLog?.( + "stderr", + `[paperclip-runner] post-completion ${stage} enrichment failed: ${detail}\n`, + ); + }, + onSessionQuarantined: async (reason) => { + await input.onLog?.( + "stderr", + `[paperclip-runner] warm native session quarantined: ${redactSensitiveText(reason).slice(-1_000)}\n`, + ); + }, onContinuityBreak: async (continuity) => { const atMs = Date.now(); await trace.record({ @@ -4354,15 +4482,34 @@ async function executePaperclipNativeSessionWithinScope( warmConfigDigest !== null ) { const existing = warmNativeSessions.get(warmSessionId); - if (existing) existing.session = session; - else + if (existing) { + if (existing.ownerToken !== warmSessionOwnerToken) { + throw new Error("native_session_supervisor_busy"); + } + existing.session = session; + } else warmNativeSessions.set(warmSessionId, { session, + ownerToken: warmSessionOwnerToken, configDigest: warmConfigDigest, + companyId: input.execution.binding.companyId, + environmentId: + input.runnerExecutionTarget?.environmentId ?? null, busy: true, idleTimer: null, lastActivityAt: new Date().toISOString(), }); + } else if (!session && warmSessionId !== null) { + const existing = warmNativeSessions.get(warmSessionId); + // onSession(null) quarantines a transport that can no longer + // be reused. Remove only this execution's generation so a + // late failure cannot evict a successor session. + if (existing?.ownerToken === warmSessionOwnerToken) { + if (existing.idleTimer !== null) { + clearTimeout(existing.idleTimer); + } + warmNativeSessions.delete(warmSessionId); + } } if (session) activeNativeSessions.set(input.execution.binding.runId, { @@ -4398,6 +4545,13 @@ async function executePaperclipNativeSessionWithinScope( } catch (error) { await leaseRenewal.stop().catch(() => undefined); const failedAtMs = Date.now(); + const executionFailureMessage = redactSensitiveText( + error instanceof Error ? error.message : String(error), + ).slice(-4_096); + await input.onLog?.( + "stderr", + `[paperclip-runner] native session execution failed: ${executionFailureMessage}\n`, + ); if (runnerSessionStartupScope) { await trace.end(runnerSessionStartupScope, { endedAtMs: failedAtMs, @@ -4422,6 +4576,7 @@ async function executePaperclipNativeSessionWithinScope( if (warmSessionId !== null && lifecyclePolicy.mode === "warm") { await releaseWarmNativeSession( warmSessionId, + warmSessionOwnerToken, lifecyclePolicy.idleTimeoutMs, true, ); @@ -4711,6 +4866,7 @@ async function executePaperclipNativeSessionWithinScope( if (warmSessionId !== null && lifecyclePolicy.mode === "warm") { await releaseWarmNativeSession( warmSessionId, + warmSessionOwnerToken, lifecyclePolicy.idleTimeoutMs, false, ); @@ -5546,10 +5702,102 @@ async function readRemoteRunnerProviderState(input: { ); } -function createRemoteRunnerProcessLauncher(input: { +const REMOTE_RUNNER_PROCESS_IDENTITY_WAIT_MS = 20_000; +const REMOTE_RUNNER_PROCESS_POLL_MS = 1_000; + +const REMOTE_RUNNER_IDENTITY_CHECK_SCRIPT = + 'set -eu; identity_path=$1; expected_nonce=$2; expected_runner_id=$3; expected_pid=$4; test -f "$identity_path" && test ! -L "$identity_path" || exit 3; { IFS= read -r nonce; IFS= read -r pid; IFS= read -r started_at; IFS= read -r runner_id; } < "$identity_path"; test "$nonce" = "$expected_nonce" && test "$runner_id" = "$expected_runner_id" && test "$pid" = "$expected_pid" && test -n "$started_at" || exit 4; kill -0 "$pid" 2>/dev/null || exit 3; if test -r "/proc/$pid/cmdline"; then command_line=$(tr "\\000" "\\n" < "/proc/$pid/cmdline"); printf "%s\\n" "$command_line" | grep -Fqx -- "--runner-id" || exit 4; printf "%s\\n" "$command_line" | grep -Fqx -- "$expected_runner_id" || exit 4; fi'; + +const REMOTE_RUNNER_CHILD_LAUNCH_SCRIPT = + 'set -eu; identity_path=$1; identity_nonce=$2; runner_instance_id=$3; diagnostics_directory=$4; shift 4; umask 077; test ! -L "$diagnostics_directory"; if test -e "$diagnostics_directory"; then test -d "$diagnostics_directory"; else mkdir -p -- "$diagnostics_directory"; fi; chmod 0700 "$diagnostics_directory"; started_at=$(date -u +"%Y-%m-%dT%H:%M:%S.%3NZ"); identity_tmp="${identity_path}.tmp.$$"; printf "%s\\n%s\\n%s\\n%s\\n" "$identity_nonce" "$$" "$started_at" "$runner_instance_id" > "$identity_tmp"; chmod 0600 "$identity_tmp"; mv -f -- "$identity_tmp" "$identity_path"; exec "$@"'; + +const REMOTE_RUNNER_FAILED_IDENTITY_CLEANUP_SCRIPT = + 'set -eu; identity_path=$1; expected_nonce=$2; expected_runner_id=$3; marker_wait=0; while { test ! -f "$identity_path" || test -L "$identity_path"; } && test "$marker_wait" -lt 50; do marker_wait=$((marker_wait + 1)); sleep 0.1; done; test -f "$identity_path" && test ! -L "$identity_path" || exit 3; { IFS= read -r nonce; IFS= read -r pid; IFS= read -r started_at; IFS= read -r runner_id; } < "$identity_path"; test "$nonce" = "$expected_nonce" && test "$runner_id" = "$expected_runner_id" && test -n "$started_at" || exit 4; case "$pid" in ""|*[!0-9]*) exit 4 ;; esac; test "$pid" -gt 0 || exit 4; if kill -0 "$pid" 2>/dev/null; then if test -r "/proc/$pid/cmdline"; then command_line=$(tr "\\000" "\\n" < "/proc/$pid/cmdline"); printf "%s\\n" "$command_line" | grep -Fqx -- "--runner-id" || exit 4; printf "%s\\n" "$command_line" | grep -Fqx -- "$expected_runner_id" || exit 4; fi; signal_target=$pid; if command -v ps >/dev/null 2>&1; then session_id=$(ps -o sid= -p "$pid" 2>/dev/null | tr -d " ") || true; if test "$session_id" = "$pid"; then signal_target="-$pid"; fi; fi; kill -TERM -- "$signal_target" 2>/dev/null || kill -TERM "$pid" 2>/dev/null || true; term_wait=0; while kill -0 "$pid" 2>/dev/null && test "$term_wait" -lt 50; do term_wait=$((term_wait + 1)); sleep 0.1; done; if kill -0 "$pid" 2>/dev/null; then kill -KILL -- "$signal_target" 2>/dev/null || kill -KILL "$pid" 2>/dev/null || true; kill_wait=0; while kill -0 "$pid" 2>/dev/null && test "$kill_wait" -lt 50; do kill_wait=$((kill_wait + 1)); sleep 0.1; done; fi; kill -0 "$pid" 2>/dev/null && exit 5; fi; test -f "$identity_path" && test ! -L "$identity_path" || exit 4; { IFS= read -r final_nonce; IFS= read -r final_pid; IFS= read -r final_started_at; IFS= read -r final_runner_id; } < "$identity_path"; test "$final_nonce" = "$nonce" && test "$final_pid" = "$pid" && test "$final_started_at" = "$started_at" && test "$final_runner_id" = "$runner_id" || exit 4; rm -f -- "$identity_path"'; + +export function parseRemoteRunnerProcessIdentity( + value: string, + expected: { nonce: string; runnerInstanceId: string }, +): { pid: number; startedAt: string } | null { + const [nonce, rawPid, startedAt, runnerInstanceId, ...remainder] = value + .trim() + .split("\n"); + const pid = Number(rawPid); + if ( + remainder.length > 0 || + nonce !== expected.nonce || + runnerInstanceId !== expected.runnerInstanceId || + !Number.isSafeInteger(pid) || + pid <= 0 || + !startedAt || + Number.isNaN(new Date(startedAt).getTime()) + ) { + return null; + } + return { pid, startedAt }; +} + +async function waitForRemoteRunnerProcessIdentity(input: { + runner: CommandManagedRuntimeRunner; + identityPath: string; + nonce: string; + runnerInstanceId: string; +}): Promise<{ pid: number; startedAt: string }> { + const deadline = Date.now() + REMOTE_RUNNER_PROCESS_IDENTITY_WAIT_MS; + while (Date.now() < deadline) { + const result = await input.runner + .execute({ + command: "sh", + args: [ + "-c", + 'test -f "$1" && test ! -L "$1" && cat -- "$1"', + "paperclip-runner-process-identity", + input.identityPath, + ], + bypassSession: true, + timeoutMs: 2_000, + }) + .catch(() => null); + const identity = + result && result.exitCode === 0 && !result.timedOut + ? parseRemoteRunnerProcessIdentity(result.stdout, input) + : null; + if (identity) return identity; + await new Promise((resolve) => setTimeout(resolve, 100)); + } + throw new Error("runner_remote_process_identity_unavailable"); +} + +async function cleanupRemoteRunnerAfterIdentityFailure(input: { + runner: CommandManagedRuntimeRunner; + identityPath: string; + nonce: string; + runnerInstanceId: string; +}): Promise { + const result = await input.runner + .execute({ + command: "sh", + args: [ + "-c", + REMOTE_RUNNER_FAILED_IDENTITY_CLEANUP_SCRIPT, + "paperclip-runner-identity-failure-cleanup", + input.identityPath, + input.nonce, + input.runnerInstanceId, + ], + bypassSession: true, + timeoutMs: 20_000, + }) + .catch(() => null); + return result?.exitCode === 0 && result.timedOut === false; +} + +export function createRemoteRunnerProcessLauncher(input: { target: Extract; runner: CommandManagedRuntimeRunner; remoteBinary: string; + processIdentityPath: string; + stateDirectory: string; + diagnosticsDirectory: string; runnerInstanceId: string; ensureArtifact?: () => Promise; onSpawn?: (meta: { @@ -5563,15 +5811,39 @@ function createRemoteRunnerProcessLauncher(input: { }): (spec: RunnerProcessLaunchSpec) => RunnerProcessHandle { const runner = input.runner; return (spec) => { + let launchedIdentity: { + nonce: string; + pid: number; + startedAt: string; + } | null = null; const child: RunnerProcessHandle["child"] = { pid: undefined, exitCode: null, signalCode: null, - kill: () => { - const pattern = `--runner-id ${input.runnerInstanceId}`; + kill: (requestedSignal) => { + const identity = launchedIdentity; + if (!identity) return false; + const signal = + requestedSignal === "SIGKILL" || requestedSignal === 9 + ? "KILL" + : requestedSignal === "SIGINT" || requestedSignal === 2 + ? "INT" + : "TERM"; + // The durable marker, nonce, exact pid, and runner-id command line are + // revalidated in the sandbox immediately before signalling. A recycled + // pid or replaced marker therefore fails closed instead of killing an + // unrelated process. void runner.execute({ - command: "pkill", - args: ["-f", pattern], + command: "sh", + args: [ + "-c", + `${REMOTE_RUNNER_IDENTITY_CHECK_SCRIPT}; kill -${signal} "$expected_pid"`, + "paperclip-runner-signal", + input.processIdentityPath, + identity.nonce, + input.runnerInstanceId, + String(identity.pid), + ], bypassSession: true, timeoutMs: 10_000, }); @@ -5604,36 +5876,164 @@ function createRemoteRunnerProcessLauncher(input: { endedAtMs: Date.now(), attributes: { target: "remote" }, }); - const result = await runner.execute({ - command: input.remoteBinary, - args: [...spec.args], + const identityNonce = randomUUID(); + const remoteArgs = [...spec.args]; + const diagnosticsArgumentIndex = remoteArgs.indexOf( + "--diagnostics-directory", + ); + if (diagnosticsArgumentIndex >= 0) { + remoteArgs[diagnosticsArgumentIndex + 1] = input.diagnosticsDirectory; + } else { + remoteArgs.push("--diagnostics-directory", input.diagnosticsDirectory); + } + // Do not keep runnerd as the foreground command of a provider RPC. Some + // sandbox command/session transports impose a provider-side lifetime on + // that RPC even when Paperclip requests a longer timeout. Detach runnerd + // into its own session instead; its own bounded diagnostics directory and + // durable PRP state remain the authorities, and the controller monitors + // the exact persisted process identity below. + const launchResult = await runner.execute({ + command: "sh", + args: [ + "-c", + 'set -eu; identity_path=$1; identity_nonce=$2; runner_instance_id=$3; child_script=$4; shift 4; umask 077; identity_dir=$(dirname -- "$identity_path"); mkdir -p -- "$identity_dir"; if command -v setsid >/dev/null 2>&1; then nohup setsid sh -c "$child_script" paperclip-runner-child "$identity_path" "$identity_nonce" "$runner_instance_id" "$@" /dev/null 2>&1 & else nohup sh -c "$child_script" paperclip-runner-child "$identity_path" "$identity_nonce" "$runner_instance_id" "$@" /dev/null 2>&1 & fi', + "paperclip-runner-launch", + input.processIdentityPath, + identityNonce, + input.runnerInstanceId, + REMOTE_RUNNER_CHILD_LAUNCH_SCRIPT, + input.diagnosticsDirectory, + input.remoteBinary, + ...remoteArgs, + ], cwd: input.target.remoteCwd, env: processEnvironment(spec.environment), - timeoutMs: 62 * 60_000, - useSession: true, + timeoutMs: 20_000, + bypassSession: true, onLog: input.onLog, - onSpawn: async (meta) => { - child.pid = meta.pid; - await input.onSpawn?.({ - pid: meta.pid, - processGroupId: null, - startedAt: new Date().toISOString(), - }); - await input.trace?.record({ - name: "runner.process.launch", - parentName: "runner.session.startup", - startedAtMs: launchStartedAtMs, - endedAtMs: Date.now(), - }); - }, }); - child.exitCode = result.exitCode; - return { - code: result.exitCode, - signal: null, - stdout: result.stdout, - stderr: result.stderr, - }; + if (launchResult.exitCode !== 0 || launchResult.timedOut) { + throw new Error( + launchResult.timedOut + ? "runner_remote_process_launch_timed_out" + : "runner_remote_process_launch_failed", + ); + } + let identity: { pid: number; startedAt: string }; + try { + identity = await waitForRemoteRunnerProcessIdentity({ + runner, + identityPath: input.processIdentityPath, + nonce: identityNonce, + runnerInstanceId: input.runnerInstanceId, + }); + } catch { + const cleanupStartedAtMs = Date.now(); + const cleaned = await cleanupRemoteRunnerAfterIdentityFailure({ + runner, + identityPath: input.processIdentityPath, + nonce: identityNonce, + runnerInstanceId: input.runnerInstanceId, + }); + await input.trace?.record({ + name: "runner.process.identity_failure_cleanup", + parentName: "runner.session.startup", + startedAtMs: cleanupStartedAtMs, + endedAtMs: Date.now(), + attributes: { cleaned }, + }); + if (!cleaned) { + throw new Error( + "runner_remote_process_identity_unavailable_cleanup_failed", + ); + } + throw new Error("runner_remote_process_identity_unavailable"); + } + launchedIdentity = { nonce: identityNonce, ...identity }; + child.pid = identity.pid; + await input.onSpawn?.({ + pid: identity.pid, + processGroupId: null, + startedAt: identity.startedAt, + }); + await input.trace?.record({ + name: "runner.process.launch", + parentName: "runner.session.startup", + startedAtMs: launchStartedAtMs, + endedAtMs: Date.now(), + attributes: { identitySource: "remote_marker", detached: true }, + }); + + while (true) { + const observed = await runner.execute({ + command: "sh", + args: [ + "-c", + REMOTE_RUNNER_IDENTITY_CHECK_SCRIPT, + "paperclip-runner-monitor", + input.processIdentityPath, + identityNonce, + input.runnerInstanceId, + String(identity.pid), + ], + bypassSession: true, + timeoutMs: 10_000, + }); + if (observed.exitCode === 0 && !observed.timedOut) { + await new Promise((resolve) => + setTimeout(resolve, REMOTE_RUNNER_PROCESS_POLL_MS), + ); + continue; + } + child.exitCode = null; + const identityMismatch = observed.exitCode === 4; + const diagnostic = await runner + .execute({ + command: "sh", + args: [ + "-c", + 'set -eu; directory=$1; file="$directory/runnerd.stderr.log"; test -d "$directory" && test ! -L "$directory" && test -f "$file" && test ! -L "$file"; tail -c 65536 -- "$file"', + "paperclip-runner-diagnostics", + input.diagnosticsDirectory, + ], + bypassSession: true, + timeoutMs: 10_000, + }) + .catch(() => null); + const diagnosticTail = + diagnostic && diagnostic.exitCode === 0 && !diagnostic.timedOut + ? redactSensitiveText(diagnostic.stdout).slice(-16_384).trim() + : ""; + const durableState = await readRemoteRunnerState({ + runner, + stateDirectory: input.stateDirectory, + }).catch(() => null); + const lifecycle = + typeof durableState?.lifecycle === "string" + ? durableState.lifecycle + : "unavailable"; + const recoverableFailure = + typeof durableState?.recoverableFailure === "string" + ? durableState.recoverableFailure + : typeof durableState?.recoverable_failure === "string" + ? durableState.recoverable_failure + : null; + const stateDiagnostics = Array.isArray(durableState?.diagnostics) + ? durableState.diagnostics + .filter((value): value is string => typeof value === "string") + .slice(-4) + .map((value) => redactSensitiveText(value).slice(-1_000)) + : []; + const stateSummary = `runner_remote_process_exited lifecycle=${lifecycle}${recoverableFailure ? ` recoverableFailure=${redactSensitiveText(recoverableFailure).slice(-1_000)}` : ""}${stateDiagnostics.length > 0 ? ` diagnostics=${JSON.stringify(stateDiagnostics)}` : ""}`; + return { + code: null, + signal: null, + stdout: "", + stderr: identityMismatch + ? "runner_remote_process_identity_mismatch" + : diagnosticTail || stateSummary, + }; + } })(); return { child, completion }; }; @@ -5721,14 +6121,25 @@ export async function createRunnerdBackend(input: { } initializingSessionToolAuthorities.add(sessionScopeId); try { - await migrateRunnerdStateRootForExecution({ - db: input.db, - execution: input.execution, - allowVerifiedBackup: - input.runnerExecutionTarget?.kind === "remote" && - input.runnerExecutionTarget.transport === "sandbox", - restartRecovery: input.restartRecovery, - }); + // executePaperclipNativeSession holds the full session-scope claim and + // verifies/migrates the durable root before it acquires the coordinator + // lease. Avoid reclassifying the same root after that path has marked its + // retained warm owner busy for this run. Direct backend construction still + // performs the complete fail-closed verification here. + if ( + executingRunnerdSessionScopes.get(sessionScopeId) !== + input.execution.binding.runId + ) { + await migrateRunnerdStateRootForExecution({ + db: input.db, + execution: input.execution, + allowVerifiedBackup: + input.runnerExecutionTarget?.kind === "remote" && + input.runnerExecutionTarget.transport === "sandbox", + allowRetainedWarmRunner: false, + restartRecovery: input.restartRecovery, + }); + } return await createRunnerdBackendWithinSessionClaim(input, sessionScopeId); } finally { initializingSessionToolAuthorities.delete(sessionScopeId); @@ -7154,11 +7565,17 @@ async function createRunnerdBackendWithinSessionClaim( : undefined; const remoteProcessLauncher = - remoteTarget && remoteCommandRunner && remoteBinary + remoteTarget && remoteCommandRunner && remoteBinary && remoteStateDirectory ? createRemoteRunnerProcessLauncher({ target: remoteTarget, runner: remoteCommandRunner, remoteBinary, + processIdentityPath: posix.join( + remoteStateDirectory, + "runner-process.identity", + ), + stateDirectory: remoteStateDirectory, + diagnosticsDirectory: posix.join(remoteSessionRoot!, "diagnostics"), runnerInstanceId: input.runnerInstanceId, ensureArtifact: ensureRemoteRunner, onSpawn: input.onSpawn, @@ -7186,8 +7603,12 @@ async function createRunnerdBackendWithinSessionClaim( const effectiveRunnerEnvironment: NodeJS.ProcessEnv = remoteRuntimeRoot ? { ...effectiveRunnerEnvironmentBase, - HOME: remoteTarget!.remoteCwd, - CODEX_HOME: posix.join(remoteTarget!.remoteCwd, ".codex"), + // The provider home is runner-owned state, not the execution workspace. + // Codex's permission profile explicitly denies HOME and CODEX_HOME. If + // either points at remoteCwd, that deny rule shadows the workspace write + // grant and the provider cannot initialize its shell sandbox or edit. + HOME: posix.join(remoteRunnerFilesystemRoot!, "codex-home"), + CODEX_HOME: posix.join(remoteRunnerFilesystemRoot!, "codex-home"), PAPERCLIP_WORKSPACE_CWD: remoteTarget!.remoteCwd, ...(remoteTarget!.transport === "sandbox" ? { PAPERCLIP_RUNNER_EXTERNAL_SANDBOX: "1" } @@ -7389,6 +7810,12 @@ async function createRunnerdBackendWithinSessionClaim( } : undefined, environment: effectiveRunnerEnvironment, + onDiagnostic: (message) => { + void input.onLog?.( + "stderr", + `[paperclip-runner] runnerd diagnostic: ${redactSensitiveText(message).slice(-4_096)}\n`, + ); + }, lifecyclePolicy: input.execution.session.lifecyclePolicy, runtimeContext: "runtimeContext" in input.execution @@ -7396,6 +7823,7 @@ async function createRunnerdBackendWithinSessionClaim( : null, runnerRuntimeContext: remoteRuntimeContext, runnerFilesystemRoot: remoteRunnerFilesystemRoot ?? undefined, + externallySandboxed: remoteTarget?.transport === "sandbox", opencodeRuntimeDirectory: remoteRunnerFilesystemRoot ? posix.join(remoteRunnerFilesystemRoot, "opencode") : undefined, @@ -7621,6 +8049,13 @@ async function createRunnerdBackendWithinSessionClaim( }, { parentName: "runner.session.startup" }, ); + // Cancellation can release the sandbox while activation is + // still waiting for the remote runner process. Attach a + // rejection observer immediately; `ready()` still awaits the + // original promise and reports the same startup failure, but + // an early teardown can no longer crash the controller with + // an unhandled plugin RPC rejection. + void activation.catch(() => undefined); }, ready: async () => { await measureNativeRunnerSpan( diff --git a/server/src/services/native-runtime/native-workspace-sync.ts b/server/src/services/native-runtime/native-workspace-sync.ts index e8f9f95884..2933cf9212 100644 --- a/server/src/services/native-runtime/native-workspace-sync.ts +++ b/server/src/services/native-runtime/native-workspace-sync.ts @@ -90,6 +90,7 @@ export type NativeWorkspaceInboundEvidence = | { kind: "existing_run"; restartRecovery: boolean; + sameRunRecovery?: boolean; sameProviderLease: boolean; } | { @@ -102,7 +103,7 @@ export function classifyNativeWorkspaceInbound( evidence: NativeWorkspaceInboundEvidence, ): WorkspaceInboundMode { if (evidence.kind === "existing_run") { - if (!evidence.restartRecovery) { + if (!evidence.restartRecovery && !evidence.sameRunRecovery) { throw new Error("native_workspace_sync_unexpected_existing_descriptor"); } return evidence.sameProviderLease ? "adopt_remote" : "durable_seed"; @@ -734,6 +735,7 @@ export async function prepareNativeWorkspaceSync(input: { target: AdapterExecutionTarget | null; lease: EnvironmentLease; restartRecovery?: NativeRestartRecoveryClaim; + sameRunRecovery?: boolean; resourceDisposition?: NativeWorkspaceResourceDisposition; }): Promise { if (input.target?.kind !== "remote" || input.target.transport !== "sandbox") { @@ -777,6 +779,7 @@ export async function prepareNativeWorkspaceSync(input: { mode = classifyNativeWorkspaceInbound({ kind: "existing_run", restartRecovery: Boolean(input.restartRecovery), + sameRunRecovery: input.sameRunRecovery, sameProviderLease, }); const durableSeed = diff --git a/server/src/services/sandbox-provider-runtime.ts b/server/src/services/sandbox-provider-runtime.ts index 74566c3ea5..ad0d690f48 100644 --- a/server/src/services/sandbox-provider-runtime.ts +++ b/server/src/services/sandbox-provider-runtime.ts @@ -328,7 +328,11 @@ function metadataMatchesPluginSandboxConfig( if (metadata.reuseLease !== true) return false; for (const [key, value] of Object.entries(config)) { if (key === "provider" || key === "reuseLease") continue; - if (value === undefined) continue; + // Null is the normalized form of an unspecified optional provider setting. + // The provider may report the concrete default it realized (for example, + // Daytona resolves a null target to "us"), which remains compatible with + // the caller's lack of a preference. + if (value === undefined || value === null) continue; if (JSON.stringify(metadata[key]) !== JSON.stringify(value)) { return false; } diff --git a/tests/runner-e2e/catalog.test.ts b/tests/runner-e2e/catalog.test.ts index 685c18add8..dbdbe995e4 100644 --- a/tests/runner-e2e/catalog.test.ts +++ b/tests/runner-e2e/catalog.test.ts @@ -101,6 +101,22 @@ describe("runner E2E catalog", () => { expect( daytonaWarmContinuityTask.buildFollowupMessages?.("nonce"), ).toHaveLength(2); + const initialPrompt = daytonaWarmContinuityTask.buildPrompt("nonce"); + const followups = + daytonaWarmContinuityTask.buildFollowupMessages?.("nonce") ?? []; + expect(initialPrompt).toContain('"kind":"request_confirmation"'); + expect(initialPrompt).toContain( + '"reviewInteractionId":""', + ); + expect(initialPrompt).toContain('"continuationPolicy":"wake_assignee"'); + expect(followups[0]).toContain('"kind":"request_confirmation"'); + expect(followups[0]).toContain( + '"reviewInteractionId":""', + ); + expect(followups[1]).toContain( + '{"status":"done","comment":"PAPERCLIP_E2E_WARM_T3_nonce"}', + ); + expect(followups[1]).not.toContain('"kind":"request_confirmation"'); const cells = runnerMatrix.filter( (entry) => entry.suite.id === "daytona-warm-continuity", ); diff --git a/tests/runner-e2e/catalog.ts b/tests/runner-e2e/catalog.ts index 586879eace..eff09ecf5b 100644 --- a/tests/runner-e2e/catalog.ts +++ b/tests/runner-e2e/catalog.ts @@ -769,7 +769,11 @@ function warmTurnMarker(turn: 1 | 2 | 3, nonce: string) { } function warmWorkspaceLine(turn: 1 | 2 | 3, nonce: string) { - return `T${turn}_${nonce}`; + // Issue descriptions are rendered through the native prompt's Markdown + // boundary, which escapes underscores. Keep the byte-level workspace + // sentinel Markdown-inert so both legacy and native providers receive the + // same literal content. + return `T${turn}-${nonce}`; } function warmTurnInstructions(turn: 1 | 2 | 3, nonce: string) { @@ -779,6 +783,9 @@ function warmTurnInstructions(turn: 1 | 2 | 3, nonce: string) { ); const marker = warmTurnMarker(turn, nonce); const finalTurn = turn === 3; + const legacyCompletion = finalTurn + ? `In a legacy runner, make exactly one public-API completion write after verification: PATCH /api/issues/$PAPERCLIP_TASK_ID with {"status":"done","comment":"${marker}"}. Include Authorization and X-Paperclip-Run-Id. Do not POST a separate comment.` + : `In a legacy runner, after verification POST exactly one request_confirmation to /api/issues/$PAPERCLIP_TASK_ID/interactions with {"kind":"request_confirmation","idempotencyKey":"daytona-warm-review-T${turn}-${nonce}","resolverPolicy":"human_only","title":"Warm continuity turn ${turn}","summary":"Review completed warm continuity turn ${turn}.","continuationPolicy":"wake_assignee","payload":{"version":1,"prompt":"Continue to warm continuity turn ${turn + 1}?","acceptLabel":"Approve completion","rejectLabel":"Continue work","rejectRequiresReason":true,"allowDeclineReason":true,"supersedeOnUserComment":false,"target":{"type":"custom","key":"daytona_warm_turn_${turn}","revisionId":"${nonce}-T${turn}","label":"Warm continuity turn ${turn}"}}}. Capture the returned interaction id. Then make exactly one issue PATCH with {"status":"in_review","comment":"${marker}","reviewInteractionId":""}. Include Authorization and X-Paperclip-Run-Id on both writes. If the issue PATCH fails, retry only that PATCH and never create another interaction. Do not POST a separate comment. After both writes succeed, end the response and heartbeat immediately; do not wait or poll because the reviewer action will start the next turn.`; return [ `This is warm Daytona continuity turn ${turn} of 3. Work only in the current execution workspace.`, turn === 1 @@ -786,7 +793,8 @@ function warmTurnInstructions(turn: 1 | 2 | 3, nonce: string) { : `Before changing anything, read ${file} and verify its content is exactly ${lines.slice(0, -1).join("\\n")} followed by a newline. Then append exactly ${lines.at(-1)} followed by a newline.`, `After the write, verify ${file} contains exactly these lines, once each and in order: ${lines.join(" | ")}.`, `In a native runner, call paperclip_finish exactly once with {reportedWorkDisposition:"${finalTurn ? "done" : "needs_review"}",summary:"${marker}",completionClaim:{contractRevision:"1",objectiveSatisfied:true,criteria:[{criterionId:"objective",status:"satisfied",evidenceRefs:[]}],remainingWork:[]},evidence:[],verification:[{commandOrCheck:"read ${file}",status:"passed"}]}. Wait for that tool call to succeed, then emit exactly ${marker} once as the complete user-facing final response.`, - `In a legacy runner, make exactly one public-API completion write after verification: PATCH /api/issues/$PAPERCLIP_TASK_ID with {"status":"${finalTurn ? "done" : "in_review"}","comment":"${marker}"}. Include Authorization and X-Paperclip-Run-Id. Do not POST a separate comment.`, + legacyCompletion, + `In a legacy runner, the PATCH comment is the complete visible response. After its 2xx response, finish silently: do not print, echo, or emit ${marker} again as assistant text.`, `Do not include ${marker} in any other visible response or write. Do not recreate, truncate, reorder, or duplicate prior lines.`, ].join("\n"); } diff --git a/tests/runner-e2e/harness-env.ts b/tests/runner-e2e/harness-env.ts index df7596992e..8d5691e9fe 100644 --- a/tests/runner-e2e/harness-env.ts +++ b/tests/runner-e2e/harness-env.ts @@ -76,8 +76,15 @@ export function resolvePaperclipRunnerBinaryForHarness( export function resolvePaperclipRemoteRunnerBinaryForHarness( executions: readonly MatrixExecution[], runnerBinary: string | undefined, + configuredPath = process.env.PAPERCLIP_RUNNER_REMOTE_BINARY_PATH, + platform: NodeJS.Platform = process.platform, ): string | undefined { + if (configuredPath?.trim()) return configuredPath; if (!runnerBinary) return undefined; + // Daytona runs Linux. A default debug binary built by a macOS developer is + // Mach-O and cannot be staged into that sandbox. Leave the remote override + // unset so the pinned Daytona image's verified runnerd is discovered instead. + if (platform !== "linux") return undefined; return executions.some( (execution) => execution.profile.generation === "native" && diff --git a/tests/runner-e2e/redaction.ts b/tests/runner-e2e/redaction.ts index e07adb402c..d1214542c6 100644 --- a/tests/runner-e2e/redaction.ts +++ b/tests/runner-e2e/redaction.ts @@ -9,6 +9,9 @@ const SECRET_SHAPES = [ /\b(?:openrouter|daytona)[-_]?(?:api)?[-_]?key["'=:\s]+[A-Za-z0-9._-]{12,}\b/gi, ] as const; +const SENSITIVE_JSON_KEY = + /^(?:api[-_]?key|access[-_]?token|refresh[-_]?token|auth(?:orization)?|bearer|client[-_]?secret|cookie|password|secret|token)$/i; + export function normalizedSecrets(values: readonly (string | undefined)[]) { return [ ...new Set( @@ -130,7 +133,9 @@ export function sanitizeJson( return Object.fromEntries( Object.entries(value).map(([key, entry]) => [ key, - sanitizeJson(entry, secrets), + SENSITIVE_JSON_KEY.test(key) + ? "[REDACTED]" + : sanitizeJson(entry, secrets), ]), ); } diff --git a/tests/runner-e2e/runner.spec.ts b/tests/runner-e2e/runner.spec.ts index f97cd952d6..e56807db00 100644 --- a/tests/runner-e2e/runner.spec.ts +++ b/tests/runner-e2e/runner.spec.ts @@ -87,6 +87,7 @@ interface RunRecord { interface EnvironmentLeaseRecord { id: string; status?: string; + cleanupStatus?: string | null; leasePolicy?: string; providerLeaseId?: string | null; executionWorkspaceId?: string | null; @@ -103,6 +104,8 @@ interface InteractionRecord { id: string; status: string; kind?: string; + sourceRunId?: string | null; + continuationPolicy?: string; payload?: { version?: number; acceptLabel?: string; @@ -349,7 +352,10 @@ async function createTaskThroughUi(input: { await input.page.getByText(input.agentName, { exact: true }).last().click(); if (input.projectName) { const dialog = input.page.getByRole("dialog"); - await dialog.getByRole("button", { name: "Project", exact: true }).click(); + // Selecting the assignee advances focus to this selector and opens it. + // Focus is idempotent here; clicking would toggle an already-open popover + // closed before the search field can be filled. + await dialog.getByRole("button", { name: "Project", exact: true }).focus(); await dialog.getByPlaceholder("Search projects...").fill(input.projectName); await dialog.getByText(input.projectName, { exact: true }).last().click(); } @@ -372,6 +378,23 @@ async function submitTaskReply(page: Page, body: string): Promise { return submittedAtMs; } +async function submitTaskRevision(page: Page, body: string): Promise { + const revise = page + .getByRole("button", { name: "Continue work", exact: true }) + .last(); + await expect(revise).toBeVisible({ timeout: 30_000 }); + await revise.click(); + const reason = page.getByPlaceholder("Add a short note").last(); + await expect(reason).toBeVisible({ timeout: 30_000 }); + await reason.fill(body); + const submittedAtMs = Date.now(); + await page + .getByRole("button", { name: "Continue work", exact: true }) + .last() + .click(); + return submittedAtMs; +} + function matchingRuns(runs: RunRecord[], issue: IssueRecord) { const explicit = new Set( [issue.executionRunId, issue.checkoutRunId].filter(Boolean), @@ -405,6 +428,15 @@ function isPendingQuestion(interaction: InteractionRecord) { ); } +function isPendingWarmConfirmation(interaction: InteractionRecord) { + return ( + interaction.kind === "request_confirmation" && + interaction.status === "pending" && + interaction.continuationPolicy === "wake_assignee" && + interaction.payload?.target?.type === "custom" + ); +} + function normalizePlanMarkdown(body: string | null | undefined) { // Provider plan renderers may defensively escape underscores in plain-text // markers. The rendered document and semantic marker are identical. @@ -583,10 +615,7 @@ for (const execution of executions) { let failureClassOverride: FailureClass | undefined; let cleanup: RunnerE2EResult["cleanup"] = "not_started"; - const capturePrivateScreenshot = async ( - id: string, - file: string, - ) => { + const capturePrivateScreenshot = async (id: string, file: string) => { const screenshotPath = path.join(privateDir, file); await page.screenshot({ path: screenshotPath, fullPage: true }); await testInfo.attach(id, { @@ -638,6 +667,40 @@ for (const execution of executions) { runtimeLeases = relevant.length > 0 ? relevant : listed; }; + const cancelActiveRunsForCleanup = async () => { + if (!issue) return; + const cleanupIssueId = issue.id; + const runs = await api.get( + `/api/issues/${cleanupIssueId}/runs`, + ); + const activeRunIds = [ + ...new Set( + runs + .filter((run) => !TERMINAL_RUN_STATUSES.has(run.status)) + .map((run) => run.id) + .filter( + (runId): runId is string => + typeof runId === "string" && runId.length > 0, + ), + ), + ]; + for (const runId of activeRunIds) { + await api.post(`/api/heartbeat-runs/${runId}/cancel`); + } + if (activeRunIds.length === 0) return; + const activeIds = new Set(activeRunIds); + await pollUntil({ + label: `cleanup cancellation for issue ${cleanupIssueId}`, + deadlineAt: Date.now() + 45_000, + load: () => api.get(`/api/issues/${cleanupIssueId}/runs`), + accept: (currentRuns) => + currentRuns + .filter((run) => activeIds.has(run.id)) + .every((run) => TERMINAL_RUN_STATUSES.has(run.status)), + intervalMs: 500, + }); + }; + const captureFailureApiState = async () => { if (!fixtures || !issue) return; const capture = async (operation: () => Promise) => @@ -739,6 +802,9 @@ for (const execution of executions) { expect(initialExperimental.enableNativeRunner).toBe(false); await api.patch("/api/instance/settings/experimental", { enableNativeRunner: true, + ...(execution.task.flow === "warm_three_turn" + ? { enableIsolatedWorkspaces: true } + : {}), ...(execution.profile.generation === "native" && execution.environment.id === "daytona" ? { enableRunnerPreviewIngress: true } @@ -1287,10 +1353,19 @@ for (const execution of executions) { label: `warm Daytona turn ${completedTurn} review state for issue ${issue.id}`, deadlineAt: turnDeadlineAt, load: loadTaskState, - accept: ({ currentIssue, taskRuns }) => - currentIssue.status === "in_review" && - taskRuns.length === completedTurn && - taskRuns.every((run) => run.status === "succeeded"), + accept: ({ currentIssue, taskRuns, interactions }) => { + const latestRun = sortRunsChronologically(taskRuns).at(-1); + const pendingConfirmations = interactions.filter( + isPendingWarmConfirmation, + ); + return ( + currentIssue.status === "in_review" && + taskRuns.length === completedTurn && + taskRuns.every((run) => run.status === "succeeded") && + pendingConfirmations.length === 1 && + pendingConfirmations[0]?.sourceRunId === latestRun?.id + ); + }, reject: ({ taskRuns }) => definitiveRunFailure(taskRuns) ?? (taskRuns.length > completedTurn @@ -1299,7 +1374,7 @@ for (const execution of executions) { }); const expectedPrefix = `${Array.from( { length: completedTurn }, - (_, index) => `T${index + 1}_${nonce}`, + (_, index) => `T${index + 1}-${nonce}`, ).join("\n")}\n`; const hostContent = await readFile(workspaceFile, "utf8"); if (hostContent !== expectedPrefix) { @@ -1317,8 +1392,11 @@ for (const execution of executions) { `Warm turn ${completedTurn} lost its project execution-workspace scope`, ); } + const chronologicalRuns = sortRunsChronologically( + waitingState.taskRuns, + ); const completedRunIds = new Set( - waitingState.taskRuns.map((candidate) => candidate.id), + chronologicalRuns.map((candidate) => candidate.id), ); const retainedTurnLeases = await pollUntil({ label: `retained Daytona leases after warm turn ${completedTurn}`, @@ -1330,7 +1408,7 @@ for (const execution of executions) { ), accept: (leases) => { const runOrder = new Map( - waitingState.taskRuns.map((candidate, index) => [ + chronologicalRuns.map((candidate, index) => [ candidate.id, index, ]), @@ -1348,9 +1426,16 @@ for (const execution of executions) { ); return ( completed.length === completedTurn && + completed + .slice(0, -1) + .every( + (lease) => + lease.status === "expired" && + lease.cleanupStatus === "success", + ) && + completed.at(-1)?.status === "retained" && completed.every( (lease) => - lease.status === "retained" && lease.leasePolicy === "reuse_by_environment" && typeof lease.providerLeaseId === "string" && record(lease.metadata).sandboxState === "started", @@ -1365,10 +1450,7 @@ for (const execution of executions) { }, }); const turnRunOrder = new Map( - waitingState.taskRuns.map((candidate, index) => [ - candidate.id, - index, - ]), + chronologicalRuns.map((candidate, index) => [candidate.id, index]), ); const completedLeases = retainedTurnLeases .filter( @@ -1392,7 +1474,7 @@ for (const execution of executions) { turnEvidence.push({ turn: completedTurn, issue: waitingState.currentIssue, - run: waitingState.taskRuns.at(-1), + run: chronologicalRuns.at(-1), hostContent, leases: completedLeases, }); @@ -1406,7 +1488,7 @@ for (const execution of executions) { `warm-turn-${completedTurn}.png`, ); turnSubmissionTimesMs.push( - await submitTaskReply(page, followups[completedTurn - 1]), + await submitTaskRevision(page, followups[completedTurn - 1]), ); } warmLifecycleEvidence = { turns: turnEvidence }; @@ -1950,7 +2032,7 @@ for (const execution of executions) { ); } const retainedLeases = await pollUntil({ - label: `retained warm Daytona lease for issue ${issue.id}`, + label: `terminal warm Daytona lease history for issue ${issue.id}`, deadlineAt: Math.min(deadlineAt, Date.now() + 30_000), intervalMs: 500, load: () => @@ -1967,13 +2049,21 @@ for (const execution of executions) { ); return ( warmLeases.length === 3 && - warmLeases.every( - (lease) => - lease.status === "retained" && + warmLeases.every((lease) => { + const runIndex = selectedRuns.findIndex( + (candidate) => candidate.id === lease.heartbeatRunId, + ); + return ( + lease.status === + (runIndex === selectedRuns.length - 1 + ? "retained" + : "expired") && + lease.cleanupStatus === "success" && lease.leasePolicy === "reuse_by_environment" && typeof lease.providerLeaseId === "string" && - record(lease.metadata).sandboxState === "started", - ) + record(lease.metadata).sandboxState === "started" + ); + }) ); }, }); @@ -2310,6 +2400,7 @@ for (const execution of executions) { }); }); try { + await cancelActiveRunsForCleanup(); await fixtures.teardown(); cleanup = "passed"; } catch (error) { diff --git a/tests/runner-e2e/support.test.ts b/tests/runner-e2e/support.test.ts index e56d9923db..d22ba6a568 100644 --- a/tests/runner-e2e/support.test.ts +++ b/tests/runner-e2e/support.test.ts @@ -107,14 +107,34 @@ describe("runner E2E local binary resolution", () => { resolvePaperclipRemoteRunnerBinaryForHarness( [remoteNativeExecution], runnerBinary, + undefined, + "linux", ), ).toBe(runnerBinary); expect( resolvePaperclipRemoteRunnerBinaryForHarness( [localNativeExecution], runnerBinary, + undefined, + "linux", ), ).toBeUndefined(); + expect( + resolvePaperclipRemoteRunnerBinaryForHarness( + [remoteNativeExecution], + runnerBinary, + undefined, + "darwin", + ), + ).toBeUndefined(); + expect( + resolvePaperclipRemoteRunnerBinaryForHarness( + [remoteNativeExecution], + runnerBinary, + "/cross-compiled/paperclip-runnerd", + "darwin", + ), + ).toBe("/cross-compiled/paperclip-runnerd"); }); }); @@ -856,6 +876,24 @@ describe("runner E2E evidence redaction", () => { expect(sanitizeJson({ nested: [secret] }, [secret])).toEqual({ nested: ["[REDACTED]"], }); + expect( + sanitizeJson( + { + metadata: { + apiKey: "opaque-provider-issued-value", + access_token: "opaque-access-token", + apiKeyRef: "DAYTONA_API_KEY", + }, + }, + [], + ), + ).toEqual({ + metadata: { + apiKey: "[REDACTED]", + access_token: "[REDACTED]", + apiKeyRef: "DAYTONA_API_KEY", + }, + }); expect(sanitizeJson("paperclip.runner-e2e.evidence/v1", [secret])).toBe( "paperclip.runner-e2e.evidence/v1", ); diff --git a/ui/src/components/InlineEntitySelector.test.tsx b/ui/src/components/InlineEntitySelector.test.tsx index dbe7354fca..668cb799eb 100644 --- a/ui/src/components/InlineEntitySelector.test.tsx +++ b/ui/src/components/InlineEntitySelector.test.tsx @@ -136,6 +136,47 @@ describe("InlineEntitySelector", () => { }); }); + it("opens on programmatic focus without toggling an open popover closed", async () => { + const root = createRoot(container); + + act(() => { + root.render( + , + ); + }); + + const trigger = container.querySelector("button") as HTMLButtonElement | null; + expect(trigger).not.toBeNull(); + + await act(async () => { + trigger?.focus(); + await Promise.resolve(); + }); + expect( + document.querySelector('input[placeholder="Search projects..."]'), + ).not.toBeNull(); + + await act(async () => { + trigger?.focus(); + await Promise.resolve(); + }); + expect( + document.querySelector('input[placeholder="Search projects..."]'), + ).not.toBeNull(); + + act(() => { + root.unmount(); + }); + }); + it("does not open the popover when disabled", async () => { const root = createRoot(container); const onChange = vi.fn();