diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index c6beace861d..47ea15a7dd6 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -1933,10 +1933,32 @@ judged through that boot's row: - no row (a boot from before the lease): the record stays unprovable. A record with a recorded process group stays unprovable across hosts, because -agent processes may outlive their server. The lease is off on a Railway -deployment with an exclusive volume, which proves a replaced deployment from its -own identity. A stateless Railway service keeps it. -`PAPERCLIP_CONTROLLER_INSTANCE_LEASE=on|off` overrides this. +agent processes may outlive their server. The lease is enabled on volume-backed +Railway services too: a process can crash and restart inside the same deployment, +and some saved ownership records do not contain a deployment identity. +`PAPERCLIP_CONTROLLER_INSTANCE_LEASE=off` explicitly disables it. + +Native execution and restart claims register held preparation with this lease before +a runner PID or checkpoint exists. Passive identity reads do not register work. +Restart recovery uses the recorded controller +boot and last write to prove its owner stopped or to fence it after the existing +grace period when the PID space is foreign or unknown. Within the same readable +PID space, the existing process and start-time proof still governs takeover. +A live owner or a grace period keeps the preparation held, with a +scheduled recovery check. Repeating the same evidence wait preserves its last-write +timestamp so the recovery check cannot keep extending its own grace period. +After owner proof, the normal launch, checkpoint, provider-evidence and admission +checks still govern resumption; controller fencing alone does not prove an agent +process stopped or authorize repeating its work. + +Older native controller records with no verifiable boot lease remain held. Recovery +exposes the existing board stop-confirmation action for an unknown or foreign PID +space. It requires `runtime:confirm_owner_stop` and confirmation of the exact run, +boot, PID, process start and recorded host. A changed owner rejects the confirmation. +The receipt only permits normal recovery to inspect and resume the saved work. +Scoped recovery wakes also select fenced preparations with no runner process or +checkpoint yet. They use the existing finite expired-run inventory and normal +restart classifier; a missing checkpoint cannot silently discard that wake. A record saved before its owner was fully recorded (`no_owner_stamp`, `missing_identity`, `process_space_unrecorded`) stays unprovable and is never @@ -4901,6 +4923,19 @@ Upgrade diagnostics retain their existing `ready` meaning and add bounded retain header evidence. That inventory is preliminary, reports partial coverage, and does not certify workspace durability or volume removal. +The upgrade census separates current execution from an old, unowned native +preparation that recovery explicitly retained as `awaiting_evidence`. This requires +an observed preparation with no result, failure, process identity, or checkpoint +header, and an expired, recorded owner from a different controller boot. Such a +run is counted as `retainedBlockedRuns`, rather than `activeRuns` or +`recoverableRuns`. It remains unchanged and held for its normal recovery path; +the observation does not prove that its old controller stopped or authorize +replay, resource release, or volume removal. A run registered in the current +process still blocks unless it has positive recovery evidence. Before/after +execution counters, pending wake operations, drain state, and a truncated census +continue to prevent readiness. Persisted queued wakes are not deleted by an +upgrade; the pending wake counter measures in-process admission operations. + `importNativeControllerRecovery` is an internal one-source operation for the migration owner while old controller files remain available. It requires the admitted configuration digest and exact supervisor snapshot, locks the issue, diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_park_preparation.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_park_preparation.rs index 1b6f91c5bfa..26d4afb8638 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_park_preparation.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_park_preparation.rs @@ -1,18 +1,15 @@ use std::time::Instant; use super::{ - await_provider_lifetime_fence, AcpxCommandExecutor, AcpxProviderDescriptor, AcpxProviderSession, + await_provider_lifetime_fence, goal_is_dormant, AcpxCommandExecutor, AcpxProviderDescriptor, + AcpxProviderSession, }; use crate::durable::DurableRunnerError; use crate::provider_bridge::AuthorizedToolSet; impl AcpxCommandExecutor { - pub(super) fn prepare_session_for_park( - &mut self, - deadline: Instant, - ) -> Result { - self.ensure_persistence_healthy()?; - let eligible = self.candidate_admission().is_none() + fn generation_restartable(&self, session_ready: fn(&AcpxProviderSession) -> bool) -> bool { + self.candidate_admission().is_none() && self.state.as_ref().is_some_and(|state| { state.descriptor.agent == "claude" && state.lifecycle == "session_open" @@ -22,12 +19,47 @@ impl AcpxCommandExecutor { && !state.provider_exit_unconfirmed && state.pending_events.is_empty() && state.pending_runtime_requests.is_empty() + && (state.goal_projection.is_null() || goal_is_dormant(&state.goal_projection)) }) - && self - .session - .as_ref() - .is_some_and(AcpxProviderSession::can_prepare_next_generation); - if !eligible { + && self.session.as_ref().is_some_and(session_ready) + } + + pub(super) fn restart_session_after_turn(&mut self) -> Result<(), DurableRunnerError> { + self.ensure_persistence_healthy()?; + if !self.generation_restartable(AcpxProviderSession::can_restart_after_turn) { + return Ok(()); + } + let session = self.session.as_mut().expect("checked ACPX session"); + if session.settle_park_observation().is_err() + || !self.generation_restartable(AcpxProviderSession::can_restart_after_turn) + { + return Ok(()); + } + let restarted = self + .session + .as_mut() + .expect("checked ACPX session") + .restart_after_turn(None); + let Err(error) = restarted else { + return Ok(()); + }; + let state = self + .state + .as_mut() + .expect("ACPX state remains available after a turn-end restart"); + state.lifecycle = "closed".to_owned(); + self.context.provider_turn_id = None; + self.session = None; + self.failed_turn_end_restart = Some(error.to_string()); + self.save_state() + } + + pub(super) fn prepare_session_for_park( + &mut self, + deadline: Instant, + ) -> Result { + self.ensure_persistence_healthy()?; + if !self.generation_restartable(AcpxProviderSession::can_prepare_next_generation) { return Ok(false); } let prepared = self @@ -36,7 +68,7 @@ impl AcpxCommandExecutor { .expect("checked ACPX session") .prepare_next_generation(deadline); match prepared { - Ok(()) => Ok(true), + Ok(restarted) => Ok(restarted), Err(error) => { self.recover_failed_park_preparation()?; Err(DurableRunnerError::invalid(format!( diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs index 9063b2da0e1..94c33a1ee44 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs @@ -966,6 +966,7 @@ pub struct AcpxCommandExecutor { fail_after_state_rename: bool, launch_profile: Option, recovery_pending_effects: Option, + failed_turn_end_restart: Option, } // The durable command's turn and the live provider callback have separate IDs. @@ -1005,6 +1006,7 @@ impl AcpxCommandExecutor { session: None, restore_checked: false, recovery_pending_effects: None, + failed_turn_end_restart: None, restore_error: None, persistence_error: None, #[cfg(test)] @@ -1927,6 +1929,18 @@ impl AcpxCommandExecutor { fn start_turn(&mut self, payload: &Value) -> Result { turn_start_request(payload)?; + if let Some(error) = self.failed_turn_end_restart.take() { + if self.session.is_none() + && self + .state + .as_ref() + .is_some_and(|state| state.lifecycle == "closed") + { + return Err(DurableRunnerError::invalid(format!( + "ACPX turn start failed closed: {error}" + ))); + } + } let reopenable = self.session.is_none() && self.state.as_ref().is_some_and(|state| { state.identity.is_some() @@ -1997,7 +2011,7 @@ impl AcpxCommandExecutor { let (provider_process_will_be_replaced, previous_process_id) = self .session .as_ref() - .map(|session| match session.prepared_from_process_id() { + .map(|session| match session.restarted_after_turn_from() { Some(process_id) => (true, process_id), None => (session.generation_has_run_turn(), session.process_id()), }) @@ -2611,7 +2625,7 @@ impl AcpxCommandExecutor { || self .session .as_ref() - .is_none_or(AcpxProviderSession::awaits_run_rebind) + .is_none_or(AcpxProviderSession::awaits_next_turn) { return Ok(()); } @@ -2622,7 +2636,7 @@ impl AcpxCommandExecutor { if self .session .as_ref() - .is_some_and(AcpxProviderSession::awaits_run_rebind) + .is_some_and(AcpxProviderSession::awaits_next_turn) { return Ok(()); } @@ -2909,6 +2923,10 @@ impl CommandExecutor for AcpxCommandExecutor { self.prepare_session_for_park(deadline) } + fn prepare_next_turn(&mut self) -> Result<(), DurableRunnerError> { + self.restart_session_after_turn() + } + fn execute(&mut self, command: &Command) -> Result { self.restore()?; if self diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_generation.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_generation.rs index cd826f6df07..6d7cd78c6ba 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_generation.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_generation.rs @@ -54,18 +54,20 @@ impl AcpxProviderSession { self.generation_has_run_turn } + pub fn restarted_after_turn_from(&self) -> Option { + self.restarted_after_turn_from + } + pub fn awaits_run_rebind(&self) -> bool { self.prepared_generation.is_some() } - pub fn prepared_from_process_id(&self) -> Option { - self.prepared_generation - .as_ref() - .map(|(_, process_id)| *process_id) + pub fn awaits_next_turn(&self) -> bool { + self.prepared_generation.is_some() || self.restarted_after_turn_from.is_some() } pub(super) fn park_tail_is_quiet(&self) -> bool { - if self.prepared_generation.is_none() { + if !self.awaits_next_turn() { return self.transport.buffered_events().len() == 0; } self.transport.buffered_events().all(|event| { @@ -78,65 +80,70 @@ impl AcpxProviderSession { }) } - pub fn can_prepare_next_generation(&self) -> bool { + fn at_settled_turn_boundary(&self) -> bool { !self.closed && self.failure.is_none() && !self.transport_terminated && !self.runtime_retired - && self.generation_has_run_turn && self.completed_turn_observed && self.prepared_generation.is_none() && self.config.candidate_admission.is_none() && self.state.active_turn_id().is_none() && !self.state.has_pending_requests() && !self.state.has_pending_tools() - && self.transport.buffered_events().len() == 0 + && self.park_tail_is_quiet() } - pub fn prepare_next_generation(&mut self, deadline: Instant) -> Result<(), LocalRunnerError> { + pub fn can_restart_after_turn(&self) -> bool { + self.generation_has_run_turn && self.at_settled_turn_boundary() + } + + pub fn can_prepare_next_generation(&self) -> bool { + (self.generation_has_run_turn || self.restarted_after_turn_from.is_some()) + && self.at_settled_turn_boundary() + } + + pub fn restart_after_turn( + &mut self, + deadline: Option, + ) -> Result<(), LocalRunnerError> { + if !self.can_restart_after_turn() { + return Err(LocalRunnerError::invalid( + "ACPX provider is not at a settled turn boundary", + )); + } + let previous_process_id = self.process_id(); + if let Err(error) = self.restart_idle_provider(deadline) { + return Err(self.fail_closed(error)); + } + self.restarted_after_turn_from = Some(previous_process_id); + Ok(()) + } + + pub fn prepare_next_generation(&mut self, deadline: Instant) -> Result { if !self.can_prepare_next_generation() { return Err(LocalRunnerError::invalid( "ACPX provider is not at a reusable generation boundary", )); } - let previous_process_id = self.process_id(); - let mut tool_bridge = self.tool_bridge.clone(); - let mut reserved_tool_bridge = self.reserved_tool_bridge.clone(); - tool_bridge - .prepare_turn_after_provider_restart() - .and_then(|()| reserved_tool_bridge.prepare_turn_after_provider_restart()) - .map_err(|error| { - LocalRunnerError::invalid(format!( - "ACPX tool receipt rotation before park failed: {error}" - )) - })?; - if let Err(error) = self.restart_idle_provider(Some(deadline)) { - return Err(self.fail_closed(error)); - } - if self.state.settled_turn_identity_capacity_reached() { - if let Err(error) = self - .state - .rotate_settled_turn_identities_after_provider_restart() - { + let restarted = self.generation_has_run_turn; + if restarted { + self.restart_after_turn(Some(deadline))?; + if let Err(error) = self.verify_dormant_session() { return Err(self.fail_closed(error)); } + self.transport.bound_requests_by(None); } - self.tool_bridge = tool_bridge; - self.reserved_tool_bridge = reserved_tool_bridge; self.completed_turn_observed = false; - if let Err(error) = self.verify_dormant_session() { - return Err(self.fail_closed(error)); - } - self.transport.bound_requests_by(None); - self.prepared_generation = Some((generation_digest(&self.config), previous_process_id)); - Ok(()) + self.prepared_generation = Some(generation_digest(&self.config)); + Ok(restarted) } pub fn rebind_run( &mut self, next: &AcpxProviderSessionConfig, ) -> Result { - let Some((prepared, _)) = self.prepared_generation.as_ref() else { + let Some(prepared) = self.prepared_generation.as_ref() else { return Ok(false); }; if self.closed @@ -194,6 +201,7 @@ impl AcpxProviderSession { self.config = next.clone(); self.catalog_revision = next.catalog_revision; self.working_directory = next.working_directory.clone(); + self.restarted_after_turn_from = None; self.prepared_generation = None; if let Err(error) = self.verify_dormant_session() { return Err(self.fail_closed(error)); diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs index a81d3db3742..e588b0a4d68 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs @@ -357,7 +357,8 @@ pub struct AcpxProviderSession { runtime_retired: bool, completed_turn_observed: bool, generation_has_run_turn: bool, - prepared_generation: Option<(String, u32)>, + restarted_after_turn_from: Option, + prepared_generation: Option, } pub(crate) fn goal_is_dormant(projection: &Value) -> bool { @@ -417,6 +418,7 @@ impl AcpxProviderSession { runtime_retired: false, completed_turn_observed: false, generation_has_run_turn: false, + restarted_after_turn_from: None, prepared_generation: None, }) } @@ -660,12 +662,13 @@ impl AcpxProviderSession { // provider turn originated a late HTTP callback. Reap the old sidecar // and provider before releasing its call-ID tombstones, then resume // the same verified persistent session in a fresh process generation. - let provider_restarted = self.generation_has_run_turn; - if provider_restarted { + let restart_now = self.generation_has_run_turn; + if restart_now { if let Err(error) = self.restart_idle_provider(None) { return Err(self.fail_closed(error)); } } + let provider_restarted = restart_now || self.restarted_after_turn_from.is_some(); if rotate_turn_identity_ledger { if let Err(error) = self .state @@ -700,6 +703,7 @@ impl AcpxProviderSession { )))); } self.generation_has_run_turn = true; + self.restarted_after_turn_from = None; self.prepared_generation = None; let response = match self.transport.request_turn_start( json!({"turnId":turn_id,"message":message,"performance":performance}), @@ -2256,6 +2260,7 @@ mod mode_tests { runtime_retired: false, completed_turn_observed: false, generation_has_run_turn: false, + restarted_after_turn_from: None, prepared_generation: None, } } diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-acpx-sidecar.rs b/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-acpx-sidecar.rs index 76a6a309f09..cfba5cc3ba9 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-acpx-sidecar.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-acpx-sidecar.rs @@ -299,6 +299,20 @@ fn run() -> Result<(), Box> { source_pending_completion = Some(turn.to_owned()); continue; } + if fault == "pending-input" { + write_turn_event( + &mut stdout, + next_sequence, + "runtime.input_requested", + &source_run, + turn, + json!({"requestId":"input-at-terminal","questionSet":{ + "schema":"paperclip.question_set.v1", + "questions":[{"id":"target","prompt":"Which target?","required":true, + "answerMode":"single_select","options":[{"id":"first","label":"First"}]}]}}), + )?; + next_sequence += 1; + } write_turn_event( &mut stdout, next_sequence, 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 5a45df27369..d786da4a324 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 @@ -323,6 +323,10 @@ pub trait CommandExecutor { Ok(false) } + fn prepare_next_turn(&mut self) -> Result<(), DurableRunnerError> { + Ok(()) + } + fn execute(&mut self, command: &Command) -> Result; fn observe_recovery_state(&mut self, _state: &DurableState) -> Result<(), DurableRunnerError> { @@ -2168,7 +2172,11 @@ fn poll_executor_events_after_controller_ack( if state.acked_source_seq < sent_source_seq { return executor.maintain_backpressured_provider(); } - poll_executor_events(state, store, config, executor) + poll_executor_events(state, store, config, executor)?; + if state.acked_source_seq >= state.highest_source_seq() { + executor.prepare_next_turn()?; + } + Ok(()) } fn poll_executor_events_when_control_idle( @@ -5075,6 +5083,98 @@ mod tests { fs::remove_dir_all(directory).unwrap(); } + #[test] + fn turn_boundary_preparation_waits_until_the_controller_acks_the_terminal_event() { + struct BoundaryExecutor { + events: VecDeque, + calls: Vec<&'static str>, + } + impl CommandExecutor for BoundaryExecutor { + fn execute(&mut self, _: &Command) -> Result { + panic!("the poll gate cannot execute a command") + } + fn poll_events(&mut self) -> Result, DurableRunnerError> { + self.calls.push("poll"); + Ok(self.events.iter().cloned().collect()) + } + fn acknowledge_events(&mut self, count: usize) -> Result<(), DurableRunnerError> { + self.events.drain(..count); + Ok(()) + } + fn maintain_backpressured_provider(&mut self) -> Result<(), DurableRunnerError> { + self.calls.push("maintain"); + Ok(()) + } + fn prepare_next_turn(&mut self) -> Result<(), DurableRunnerError> { + self.calls.push("prepare"); + Ok(()) + } + } + let directory = std::env::temp_dir().join(format!( + "paperclip-runner-turn-boundary-{}", + uuid::Uuid::new_v4() + )); + let config = config(directory.clone()); + let store = DurableStateStore::new(&directory).unwrap(); + let (mut state, _) = store.load_or_create(&config).unwrap(); + let mut executor = BoundaryExecutor { + events: VecDeque::from([PolledEvent { + executor_event_id: "provider-terminal".to_owned(), + event_type: "turn.completed".to_owned(), + priority: EventPriority::P0, + payload: json!({"status": "completed"}), + }]), + calls: Vec::new(), + }; + poll_executor_events_after_controller_ack(&mut state, &store, &config, &mut executor, 0) + .unwrap(); + assert_eq!(executor.calls, ["poll"]); + assert_eq!(state.highest_source_seq(), 1); + poll_executor_events_after_controller_ack(&mut state, &store, &config, &mut executor, 0) + .unwrap(); + assert_eq!( + executor.calls, + ["poll", "poll"], + "an unsent terminal is not out" + ); + poll_executor_events_after_controller_ack(&mut state, &store, &config, &mut executor, 1) + .unwrap(); + assert_eq!(executor.calls[2..], ["maintain"]); + state.apply_ack(1, 2).unwrap(); + store.save(&state).unwrap(); + poll_executor_events_when_control_idle( + &mut state, + &store, + &config, + &mut executor, + 1, + &Ok(Some(json!({"kind": "command"}))), + ) + .unwrap(); + assert_eq!(executor.calls[3..], ["maintain"], "control frames go first"); + poll_executor_events_when_control_idle( + &mut state, + &store, + &config, + &mut executor, + 1, + &Err(DurableRunnerError::invalid("disconnected")), + ) + .unwrap(); + assert_eq!(executor.calls.len(), 4); + poll_executor_events_when_control_idle( + &mut state, + &store, + &config, + &mut executor, + 1, + &Ok(None), + ) + .unwrap(); + assert_eq!(executor.calls[4..], ["poll", "prepare"]); + fs::remove_dir_all(directory).unwrap(); + } + #[test] fn controller_ack_debt_runs_cleanup_without_moving_event_or_ack_cursors() { struct MaintenanceExecutor { diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs index f1f593634d9..13551eb72b9 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/native_provider_backend.rs @@ -344,6 +344,13 @@ impl CommandExecutor for NativeProviderCommandExecutor { } } + fn prepare_next_turn(&mut self) -> Result<(), DurableRunnerError> { + match self.selected.as_mut() { + Some(SelectedExecutor::Acpx(executor)) => executor.prepare_next_turn(), + _ => Ok(()), + } + } + fn observe_recovery_state( &mut self, state: &crate::durable::DurableState, diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_preparation.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_preparation.rs index 0f7fcf96fc7..f9d9a3072ca 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_preparation.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_preparation.rs @@ -14,6 +14,9 @@ use paperclip_runner_core::provider_bridge::{authorized_tool_catalog_digest, Aut use serde_json::{json, Value}; use sha2::{Digest, Sha256}; +#[path = "support/acpx_turn_end_restart.rs"] +mod turn_end_restart; + struct Fixture(PathBuf); impl Drop for Fixture { fn drop(&mut self) { @@ -267,8 +270,9 @@ fn last_process_start(journal: &[Value]) -> usize { fn prepared_park(harness: &mut Harness) -> Value { harness.complete_turn("first"); + harness.executor.prepare_next_turn().unwrap(); assert!(harness.executor.host_park_ready().unwrap()); - assert!(harness.executor.prepare_for_park(park_deadline()).unwrap()); + assert!(!harness.executor.prepare_for_park(park_deadline()).unwrap()); harness.fixture.process_starts().last().unwrap()["processStart"].clone() } diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_readiness.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_readiness.rs index 4b56a5abc4c..f1994f3b21a 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_readiness.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_park_readiness.rs @@ -265,8 +265,12 @@ fn hosted_park_acpx_requires_completion_in_this_lifetime_and_exact_authority() { )) .unwrap(); settle(&mut executor); - assert!(executor.host_park_ready().unwrap()); - assert!(executor + executor.prepare_next_turn().unwrap(); + assert!( + executor.host_park_ready().unwrap(), + "a generation restarted at the turn boundary is park-ready" + ); + assert!(!executor .prepare_for_park(Instant::now() + Duration::from_secs(20)) .unwrap()); assert!( @@ -479,3 +483,45 @@ fn hosted_park_acpx_does_not_enable_an_unqualified_park_provider() { .unwrap()); executor.shutdown().unwrap(); } + +#[test] +fn a_turn_end_restart_waits_until_an_abandoned_park_observation_tail_is_delivered() { + let (fixture, _config, mut executor, opened) = fixture("claude"); + executor + .execute(&command( + 3, + "turn.start", + json!({"turnId":"first", "text":"Complete first work."}), + )) + .unwrap(); + settle(&mut executor); + fs::write(fixture.0.join("fault"), "slow-snapshot").unwrap(); + assert!(!executor.host_park_ready().unwrap()); + fs::write(fixture.0.join("fault"), "").unwrap(); + let starts = || { + fs::read_to_string(fixture.0.join("sidecar.jsonl")) + .unwrap() + .lines() + .filter(|line| line.contains("processStart")) + .count() + }; + executor.prepare_next_turn().unwrap(); + assert_eq!( + starts(), + 1, + "the old generation's tail is not yet delivered" + ); + assert!(std::process::Command::new("kill") + .args(["-0", &opened["processId"].to_string()]) + .status() + .unwrap() + .success()); + let events = executor.poll_events().unwrap(); + assert!(events + .iter() + .any(|event| event.payload["code"] == "slow_park_observation")); + executor.acknowledge_events(events.len()).unwrap(); + executor.prepare_next_turn().unwrap(); + assert_eq!(starts(), 2); + executor.shutdown().unwrap(); +} diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_provider_turns.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_provider_turns.rs index 1294ea1adf5..f31bb5e1da5 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_provider_turns.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_provider_turns.rs @@ -934,3 +934,83 @@ fn prepared_generation_never_admits_the_reaped_generation_late_callback() { session.shutdown("test complete").unwrap(); std::fs::remove_dir_all(directory).unwrap(); } + +#[test] +fn a_turn_end_restart_reaps_the_late_callback_and_releases_receipts_at_the_next_turn() { + let mut session = AcpxProviderSession::start(&config("turns-late-tool-after-suspend")).unwrap(); + session + .start_turn("turn-1", "Please continue", &std::env::temp_dir()) + .unwrap(); + session.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + session.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + assert!(!session.can_restart_after_turn()); + session + .deliver_tool_result(&late_result("call-reused")) + .unwrap(); + let first_pid = session.process_id(); + session.restart_after_turn(None).unwrap(); + let fresh_pid = session.process_id(); + assert_ne!(fresh_pid, first_pid); + assert_eq!(session.restarted_after_turn_from(), Some(first_pid)); + assert!(!session.generation_has_run_turn() && !session.can_restart_after_turn()); + assert!(session + .poll_event(Duration::from_millis(200)) + .unwrap() + .is_none()); + + session + .start_turn("turn-2", "Use the fresh provider", &std::env::temp_dir()) + .unwrap(); + assert_eq!(session.process_id(), fresh_pid); + assert_eq!(session.restarted_after_turn_from(), None); + let fresh = session.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + assert!(matches!( + &fresh[0], + AcpxProviderStateEvent::ToolCall { call_id, operation_id, input } + if call_id == "call-reused" + && operation_id == "issues.read" + && input["id"] == "issue-1" + )); + let terminal = session.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + assert!(matches!( + terminal.last().unwrap(), + AcpxProviderStateEvent::TurnTerminal { turn_id, .. } if turn_id == "turn-2" + )); + session.shutdown("test complete").unwrap(); +} + +#[test] +fn an_interrupted_or_retired_turn_restarts_at_the_next_turn_start() { + let mut session = AcpxProviderSession::start(&config("turns")).unwrap(); + session + .start_turn("turn-1", "Please help", &std::env::temp_dir()) + .unwrap(); + session.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + assert!(!session.can_restart_after_turn()); + session + .interrupt_turn("turn-1", "Paperclip interruption") + .unwrap(); + session.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + let interrupted_pid = session.process_id(); + assert!(!session.can_restart_after_turn()); + assert!(session.restart_after_turn(None).is_err()); + assert_eq!(session.process_id(), interrupted_pid); + session + .start_turn("turn-2", "Start again", &std::env::temp_dir()) + .unwrap(); + assert_ne!(session.process_id(), interrupted_pid); + session.shutdown("test complete").unwrap(); + + let mut retired = AcpxProviderSession::start(&config("turns-retired")).unwrap(); + retired + .start_turn("turn-1", "Please help", &std::env::temp_dir()) + .unwrap(); + retired + .interrupt_turn("turn-1", "Paperclip cancellation") + .unwrap(); + retired.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + assert!(retired.state().active_turn_id().is_none()); + assert!(retired.runtime_retired()); + assert!(!retired.can_restart_after_turn()); + retired.shutdown("test complete").unwrap(); +} diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/support/acpx_turn_end_restart.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/support/acpx_turn_end_restart.rs new file mode 100644 index 00000000000..181dc19fd06 --- /dev/null +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/support/acpx_turn_end_restart.rs @@ -0,0 +1,333 @@ +use super::*; + +const WARM_TURN: [&str; 2] = ["turn.start", "session.goal.get"]; + +fn live_providers(harness: &Harness) -> Vec { + harness + .fixture + .process_starts() + .into_iter() + .map(|line| line["processStart"].clone()) + .filter(alive) + .collect() +} + +fn settle(harness: &mut Harness, turn: &str) -> Vec { + let events = harness.complete_turn(turn); + harness.executor.prepare_next_turn().unwrap(); + events +} + +fn start_follow_up(harness: &mut Harness, turn: &str) -> (Vec, Vec, Duration) { + let before = harness.fixture.journal().len(); + let started = Instant::now(); + let execution = harness.execute( + "turn.start", + json!({"turnId": turn, "text": "Complete the work."}), + ); + let elapsed = started.elapsed(); + let mut events = execution + .events + .iter() + .map(|(kind, _, payload)| json!({"type":kind,"payload":payload})) + .collect::>(); + let deadline = Instant::now() + Duration::from_secs(5); + while !events + .iter() + .any(|event: &Value| event["type"] == "turn.completed") + { + assert!(Instant::now() < deadline, "no completed provider turn"); + events.extend(harness.drain()); + } + events.extend(harness.drain()); + let commands = commands_after(&harness.fixture.journal(), before); + (events, commands, elapsed) +} + +fn reconciled(events: &[Value]) -> &Value { + &events + .iter() + .find(|event| event["type"] == "session.reconciled") + .expect("the turn reports the replaced provider process")["payload"] +} + +#[test] +fn a_settled_turn_restarts_its_provider_and_the_next_turn_starts_without_one() { + let mut harness = Harness::new(); + let old_pid = harness.opened["processId"].clone(); + harness.complete_turn("first"); + let before = harness.fixture.journal().len(); + let started = Instant::now(); + harness.executor.prepare_next_turn().unwrap(); + let restart = started.elapsed(); + let journal = harness.fixture.journal(); + let spawned = last_process_start(&journal); + assert_eq!( + commands_after(&journal[..spawned], before), + ["session.suspend"] + ); + assert_eq!(journal[spawned]["livePredecessors"], json!([])); + assert_eq!( + commands_after(&journal, spawned), + ["initialize", "session.open", "run.attach"] + ); + assert_eq!(journal[spawned + 3]["params"]["runId"], "run-1"); + let fresh_pid = journal[spawned]["processStart"].clone(); + assert!(!alive(&old_pid)); + assert_eq!(live_providers(&harness), [fresh_pid.clone()]); + assert_eq!( + harness.drain(), + Vec::::new(), + "the fresh generation's output waits for its first turn" + ); + harness.executor.prepare_next_turn().unwrap(); + assert_eq!(harness.fixture.process_starts().len(), 2); + + let (events, commands, turn_start) = start_follow_up(&mut harness, "second"); + eprintln!("turn-end restart={restart:?} follow-up turn.start={turn_start:?}"); + assert_eq!(commands, WARM_TURN); + assert_eq!(reconciled(&events)["previousProcessId"], old_pid); + assert_eq!(reconciled(&events)["processId"], fresh_pid); + assert!(events + .iter() + .any(|event| event["type"] == "harness.diagnostic" + && event["payload"]["details"]["pid"] == fresh_pid)); + assert_eq!(harness.fixture.process_starts().len(), 2); + harness.executor.prepare_next_turn().unwrap(); + assert_eq!(harness.fixture.process_starts().len(), 3); + assert_eq!(live_providers(&harness).len(), 1); + let (_, commands, _) = start_follow_up(&mut harness, "third"); + assert_eq!(commands, WARM_TURN); + harness.executor.shutdown().unwrap(); + assert_eq!(live_providers(&harness), Vec::::new()); +} + +#[test] +fn park_reuses_the_generation_a_settled_turn_restarted() { + let mut harness = Harness::new(); + settle(&mut harness, "first"); + let fresh_pid = harness.fixture.process_starts().last().unwrap()["processStart"].clone(); + assert!(harness.executor.host_park_ready().unwrap()); + let before = harness.fixture.journal().len(); + let started = Instant::now(); + assert!( + !harness.executor.prepare_for_park(park_deadline()).unwrap(), + "park neither restarts nor asks for a second proof" + ); + eprintln!( + "park preparation after a turn-end restart={:?}", + started.elapsed() + ); + assert_eq!( + commands_after(&harness.fixture.journal(), before), + Vec::::new() + ); + assert!(harness.executor.host_park_ready().unwrap()); + assert_eq!(harness.fixture.process_starts().len(), 2); + + let (attached, _) = harness.attach("run-2", |_| {}); + assert_eq!(attached.result["processId"], fresh_pid); + let events = settle(&mut harness, "second"); + assert!(events + .iter() + .any(|event| event["type"] == "harness.diagnostic" + && event["payload"]["details"]["pid"] == fresh_pid)); + assert_eq!(harness.fixture.process_starts().len(), 3); + let (events, commands, _) = start_follow_up(&mut harness, "third"); + assert_eq!(commands, WARM_TURN); + assert_eq!(reconciled(&events)["previousProcessId"], fresh_pid); + assert_eq!(harness.fixture.process_starts().len(), 3); + harness.executor.shutdown().unwrap(); +} + +#[test] +fn the_reaped_generation_late_callback_never_reaches_the_next_turn() { + let mut harness = Harness::new(); + harness.complete_turn("first"); + harness.fixture.fault("late-tool-after-suspend"); + harness.executor.prepare_next_turn().unwrap(); + harness.fixture.fault(""); + assert_eq!(harness.drain(), Vec::::new()); + let (events, commands, _) = start_follow_up(&mut harness, "second"); + assert_eq!(commands, WARM_TURN); + assert!(!format!("{events:?}").contains("late-call")); + assert!(!format!("{events:?}").contains("late-old-generation")); + assert_eq!(harness.fixture.process_starts().len(), 2); + harness.executor.shutdown().unwrap(); +} + +#[test] +fn a_turn_that_arrives_before_the_turn_end_restart_restarts_at_its_start() { + let mut harness = Harness::new(); + let old_pid = harness.opened["processId"].clone(); + harness.complete_turn("first"); + let (events, commands, _) = start_follow_up(&mut harness, "second"); + assert_eq!(commands[..2], ["session.suspend", "initialize"]); + assert_eq!(reconciled(&events)["previousProcessId"], old_pid); + assert_eq!(harness.fixture.process_starts().len(), 2); + assert_eq!(live_providers(&harness).len(), 1); + harness.executor.prepare_next_turn().unwrap(); + assert_eq!(harness.fixture.process_starts().len(), 3); + assert_eq!(live_providers(&harness).len(), 1); + let (_, commands, _) = start_follow_up(&mut harness, "third"); + assert_eq!(commands, WARM_TURN); + harness.executor.shutdown().unwrap(); +} + +#[test] +fn a_park_before_the_turn_end_restart_restarts_once() { + let mut harness = Harness::new(); + harness.complete_turn("first"); + assert!(harness.executor.host_park_ready().unwrap()); + assert!(harness.executor.prepare_for_park(park_deadline()).unwrap()); + assert!(harness.executor.host_park_ready().unwrap()); + harness.executor.prepare_next_turn().unwrap(); + assert_eq!(harness.fixture.process_starts().len(), 2); + assert_eq!(live_providers(&harness).len(), 1); + let (attached, _) = harness.attach("run-2", |_| {}); + assert_eq!(attached.result["processId"], live_providers(&harness)[0]); + harness.executor.shutdown().unwrap(); +} + +#[test] +fn a_failed_turn_end_restart_fails_the_next_turn_like_a_failed_turn_start_restart() { + let outcome = |turn_end: bool| { + let mut harness = Harness::new(); + let old_pid = harness.opened["processId"].clone(); + harness.complete_turn("first"); + harness.fixture.fault("reject-open"); + if turn_end { + harness.executor.prepare_next_turn().unwrap(); + assert!(!alive(&old_pid)); + assert!(!harness.executor.host_park_ready().unwrap()); + assert!(!harness.executor.prepare_for_park(park_deadline()).unwrap()); + assert_eq!(harness.drain(), Vec::::new()); + harness.executor.prepare_next_turn().unwrap(); + } + let first = harness + .executor + .execute(&command( + 90, + "turn.start", + json!({"turnId": "second", "text": "Complete the work."}), + )) + .unwrap_err() + .to_string(); + harness.fixture.fault(""); + let retry = harness + .executor + .execute(&command( + 91, + "turn.start", + json!({"turnId": "third", "text": "Complete the work."}), + )) + .unwrap_err() + .to_string(); + assert_eq!(live_providers(&harness), Vec::::new()); + let state = harness.fixture.state(); + let starts = harness.fixture.process_starts().len(); + harness.executor.shutdown().unwrap(); + ( + first, + retry, + state["lifecycle"].clone(), + state["providerExitUnconfirmed"].clone(), + state["activeTurnId"].clone(), + state["pendingEvents"].clone(), + starts, + ) + }; + let at_turn_end = outcome(true); + assert!( + at_turn_end.0.starts_with("ACPX turn start failed closed:") + && at_turn_end.0.contains("session.open"), + "{}", + at_turn_end.0 + ); + assert_eq!(at_turn_end, outcome(false)); +} + +#[test] +fn cancellation_after_a_turn_end_restart_is_already_settled() { + let mut harness = Harness::new(); + settle(&mut harness, "first"); + for kind in ["run.cancel", "turn.interrupt", "turn.stop"] { + let result = harness.execute(kind, json!({})).result; + assert_eq!(result["status"], "already_settled", "{kind}"); + } + let (_, commands, _) = start_follow_up(&mut harness, "second"); + assert_eq!(commands, WARM_TURN); + assert_eq!(harness.fixture.process_starts().len(), 2); + harness.executor.shutdown().unwrap(); +} + +#[test] +fn suspension_and_retirement_after_a_turn_end_restart_reap_the_fresh_generation() { + let mut harness = Harness::new(); + settle(&mut harness, "first"); + let fresh_pid = live_providers(&harness)[0].clone(); + harness.execute("runner.suspend", json!({})); + assert!(!alive(&fresh_pid)); + assert_eq!(harness.fixture.state()["lifecycle"], "suspended"); + let events = harness.complete_turn("second"); + assert!(events + .iter() + .any(|event| event["type"] == "session.resumed")); + assert_eq!(harness.fixture.process_starts().len(), 3); + harness.executor.prepare_next_turn().unwrap(); + let fresh_pid = live_providers(&harness)[0].clone(); + assert_eq!(harness.fixture.process_starts().len(), 4); + harness.executor.shutdown().unwrap(); + assert!(!alive(&fresh_pid)); +} + +#[test] +fn an_active_goal_or_a_request_at_the_terminal_keeps_today_s_restart_timing() { + let mut harness = Harness::new(); + harness.fixture.fault("goal"); + harness.complete_turn("first"); + harness.executor.prepare_next_turn().unwrap(); + assert_eq!( + harness.fixture.process_starts().len(), + 1, + "an active goal can keep working after its prompt" + ); + harness.executor.shutdown().unwrap(); + + let resolve = |turn_end: bool| { + let mut harness = Harness::new(); + harness.fixture.fault("pending-input"); + let events = harness.complete_turn("first"); + harness.fixture.fault(""); + if turn_end { + harness.executor.prepare_next_turn().unwrap(); + } + let resolved = harness + .executor + .execute(&command( + 90, + "request.resolve", + json!({"requestId": "input-at-terminal", "answers": {"target": "first"}}), + )) + .map(|execution| execution.result) + .map_err(|error| error.to_string()); + let (_, commands, _) = start_follow_up(&mut harness, "second"); + let starts = harness.fixture.process_starts().len(); + harness.executor.shutdown().unwrap(); + ( + events + .iter() + .map(|event| event["type"].clone()) + .collect::>(), + resolved, + commands.contains(&"session.suspend".to_owned()), + starts, + ) + }; + let at_turn_end = resolve(true); + let at_turn_start = resolve(false); + assert_eq!(at_turn_end.0, at_turn_start.0); + assert_eq!(at_turn_end.1, at_turn_start.1); + assert_eq!(at_turn_end.3, at_turn_start.3); + eprintln!("request at terminal: {at_turn_end:?}"); +} diff --git a/server/src/__tests__/assistant-connections-guidance.test.ts b/server/src/__tests__/assistant-connections-guidance.test.ts index 33e421c484d..563b3e04891 100644 --- a/server/src/__tests__/assistant-connections-guidance.test.ts +++ b/server/src/__tests__/assistant-connections-guidance.test.ts @@ -5,14 +5,14 @@ import { validateRunnerApiCall } from "../services/native-runtime/runner-api-cli const context = { companyId: "00000000-0000-4000-8000-000000000121", issueId: "00000000-0000-4000-8000-000000000122", runId: "00000000-0000-4000-8000-000000000123", workMode: "standard" }; it("tells agents searching for assistant disconnection that only the person can do it, and where", () => { - // The queries a chat agent used while looking for a way to disconnect a person's old assistant. for (const query of ["chat client assistant disconnect connection", "assistant connections revoke list", "mcp connection revoke REST"]) { const found = searchRunnerApi({ query, limit: 10 }).results; const revoke = found.find(row => row.operationId === "DELETE /api/mcp/connections/{id}"); expect(revoke, query).toBeDefined(); expect(revoke!.description).toContain("[Assistant connections](//apps/assistant-connection)"); expect(revoke!.description).toContain("only the signed-in person"); - expect(revoke!.description).toContain("do not search for one"); + expect(revoke!.description).toContain("Do not search for another route"); + expect(revoke!.description).toContain("OAuth token revocation sent by the connected assistant itself"); expect(revoke!.description).toMatch(/already created keep their assignees and running work/); } const list = searchRunnerApi({ query: "GET /api/mcp/connections" }).results[0]!; @@ -21,5 +21,21 @@ it("tells agents searching for assistant disconnection that only the person can it("says the call_api protocol rejection is final instead of inviting another search", () => { expect(() => validateRunnerApiCall({ operationId: "GET /api/mcp/connections" }, context)) - .toThrow(/no other API route substitutes for it\. Its catalog description says who can use it\./); + .toThrow(/no other API route substitutes for it\. Read its catalog description for details\./); +}); + +it("points at the catalog description only when the protocol endpoint has one", () => { + const undocumented = searchRunnerApi({ query: "POST /api/mcp/project-tools" }).results.find(row => row.operationId === "POST /api/mcp/project-tools"); + expect(undocumented?.description).toBe(""); + let message = ""; + try { validateRunnerApiCall({ operationId: "POST /api/mcp/project-tools" }, context); } catch (error) { message = (error as Error).message; } + expect(message).toContain("no other API route substitutes for it."); + expect(message).not.toContain("catalog description"); +}); + +it("does not claim a non-guidance protocol description says who can use the endpoint", () => { + let message = ""; + try { validateRunnerApiCall({ operationId: "GET /api/companies/{companyId}/events/ws" }, context); } catch (error) { message = (error as Error).message; } + expect(message).toContain("Read its catalog description for details."); + expect(message).not.toContain("who can use it"); }); diff --git a/server/src/__tests__/board-access-denial.test.ts b/server/src/__tests__/board-access-denial.test.ts index c545fa7831c..9518032f254 100644 --- a/server/src/__tests__/board-access-denial.test.ts +++ b/server/src/__tests__/board-access-denial.test.ts @@ -39,12 +39,15 @@ describe("board-only denial", () => { ["patch", `/api/agents/${AGENT_ID}/permissions`, `[Open the agent's permissions](//agents/${AGENT_ID}/permissions)`], ["post", `/api/agents/${AGENT_ID}/keys`, `[Open the agent's API keys](//agents/${AGENT_ID}/api-keys)`], ["post", `/api/agents/${AGENT_ID}/pause`, `[Open the agent's profile](//agents/${AGENT_ID})`], - ["post", `/api/projects/${AGENT_ID}/access-members`, `[Open the project's access settings](//projects/${AGENT_ID}/configuration)`], - ["delete", `/api/projects/${AGENT_ID}/access-members/m-1`, `[Open the project's access settings](//projects/${AGENT_ID}/configuration)`], ["get", "/api/plugins", "[Open plugin settings](//company/settings/instance/plugins)"], ["post", "/api/plugins/install", "[Open plugin settings](//company/settings/instance/plugins)"], ["post", `/api/plugins/${AGENT_ID}/config`, `[Open the plugin's settings](//company/settings/instance/plugins/${AGENT_ID})`], ["post", "/api/plugins/acme.website-preview/disable", "[Open the plugin's settings](//company/settings/instance/plugins/acme.website-preview)"], + ["post", "/api/plugins/my-plugin/config", "[Open the plugin's settings](//company/settings/instance/plugins/my-plugin)"], + ["post", "/api/plugins/tools/execute", "[Open plugin settings](//company/settings/instance/plugins)"], + ["get", "/api/plugins/ui-contributions", "[Open plugin settings](//company/settings/instance/plugins)"], + ["post", `/api/projects/${AGENT_ID}/access-members`, `[Open the project's access settings](//projects/${AGENT_ID}/configuration)`], + ["delete", `/api/projects/${AGENT_ID}/access-members/m-1`, `[Open the project's access settings](//projects/${AGENT_ID}/configuration)`], ] as const)("tells an agent %s %s is final and names the board page", async (method, path, link) => { const res = await request(createApp(agentActor))[method](path).expect(403); expect(res.body.error).toBe("Board access required"); @@ -55,6 +58,30 @@ describe("board-only denial", () => { expect(res.body.remediation).toContain(link); }); + it("links the resolved project when an agent names the project by its short reference", async () => { + const app = express(); + app.use((req, _res, next) => { + req.actor = agentActor; + next(); + }); + const router = express.Router(); + router.param("id", (req, _res, next, rawId) => { + req.params.id = rawId === "acquisition" ? AGENT_ID : rawId; + next(); + }); + router.post("/projects/:id/access-members", (req, res) => { + assertBoard(req); + res.json({ ok: true }); + }); + app.use("/api", router); + app.use(errorHandler); + const resolved = await request(app).post("/api/projects/acquisition/access-members").expect(403); + expect(resolved.body.remediation).toContain(`[Open the project's access settings](//projects/${AGENT_ID}/configuration)`); + const unresolved = await request(app).post("/api/projects/unknown-project/access-members").expect(403); + expect(unresolved.body.remediation).toContain("Link a page only if you already know its route"); + expect(unresolved.body.remediation).not.toContain("](/"); + }); + it("falls back to a general board instruction when no page is known", async () => { const res = await request(createApp(agentActor)).get(`/api/companies/${COMPANY_ID}/managed-agent-profiles`).expect(403); expect(res.body.code).toBe(BOARD_ACCESS_REQUIRED_CODE); diff --git a/server/src/__tests__/controller-instance-lease.test.ts b/server/src/__tests__/controller-instance-lease.test.ts index 7df0d46c85d..53a1eecdac5 100644 --- a/server/src/__tests__/controller-instance-lease.test.ts +++ b/server/src/__tests__/controller-instance-lease.test.ts @@ -13,6 +13,7 @@ import { ENVIRONMENT_LEASE_CONTROLLER_KEY } from "../services/environment-lease- import { proveOwnerGone, type LocalOwnerStamp } from "../services/owner-proof.js"; import { createOwnershipReportState, runOwnershipReconciliationPass } from "../services/recovery/ownership-invariant-report.js"; import { beginIdleTrackedWork, readTaskDrain } from "../services/task-admission.js"; +import { currentNativeControllerIdentity, evaluateNativeControllerTakeover } from "../services/native-runtime/native-restart-recovery.js"; const support = await getEmbeddedPostgresTestSupport(); const suite = support.supported ? describe : describe.skip; @@ -93,6 +94,24 @@ suite("controller instance lease: a replaced container's records are provable", expect(await proveOwnerGone(stampOf(live))).toEqual({ verdict: "alive", reason: "controller_lease_held" }); }); + it("tracks native preparation before a runner exists and proves a fenced foreign controller", async () => { + const replaced = await otherBoot({ expiredMsAgo: 600_000 }); + await startHere({}, false); + const owner = await currentNativeControllerIdentity(); + await tickControllerInstanceLease(); + expect(await db.select().from(controllerInstances).where(eq(controllerInstances.bootId, owner.bootId))).toEqual([]); + currentControllerProcessStamp(); + await tickControllerInstanceLease(); + expect(await db.select().from(controllerInstances).where(eq(controllerInstances.bootId, owner.bootId))).toHaveLength(1); + const decision = await evaluateNativeControllerTakeover({ now: new Date(), owner: { + controllerBootId: replaced, controllerPid: 7, controllerHostHash: "old-container", + controllerProcessStartedAt: new Date(minutesAgo(15)), updatedAt: new Date(minutesAgo(10)), + leaseOwner: "old-preparation", leaseExpiresAt: new Date(minutesAgo(5)), + }, isProcessAlive: () => { throw new Error("A foreign PID must not be inspected locally"); } }); + expect(decision).toEqual({ allowed: true, reason: "controller_fenced" }); + expect(await proveControllerInstance(db, replaced, null)).toBe("fenced"); + }); + it("keeps an owner alive (and watched) through the grace, and for work written after an idle lapse", async () => { const justExpired = await otherBoot({ expiredMsAgo: 5_000 }); const resumed = await otherBoot({ expiredMsAgo: 600_000 }); @@ -396,7 +415,7 @@ suite("controller instance lease: a replaced container's records are provable", expect(lost).toHaveBeenCalledWith("fenced"); }); - it("records a graceful stop, and is off on a volume-exclusive Railway deployment or when disabled", async () => { + it("records a graceful stop and keeps native preparation fencing on volume-exclusive deployments", async () => { await startHere(); await stopControllerInstanceLease(db); expect(await proveControllerInstance(db, currentControllerBootId(), null)).toBe("stopped"); @@ -410,7 +429,7 @@ suite("controller instance lease: a replaced container's records are provable", expect(controllerInstanceLeaseEnabled()).toBe(true); vi.stubEnv("RAILWAY_VOLUME_ID", "v"); vi.stubEnv("RAILWAY_VOLUME_MOUNT_PATH", "/data"); - expect(controllerInstanceLeaseEnabled()).toBe(false); + expect(controllerInstanceLeaseEnabled()).toBe(true); vi.stubEnv("PAPERCLIP_CONTROLLER_INSTANCE_LEASE", "on"); expect(controllerInstanceLeaseEnabled()).toBe(true); vi.unstubAllEnvs(); diff --git a/server/src/__tests__/openapi-routes.test.ts b/server/src/__tests__/openapi-routes.test.ts index 3500aa68f94..a762b955064 100644 --- a/server/src/__tests__/openapi-routes.test.ts +++ b/server/src/__tests__/openapi-routes.test.ts @@ -576,6 +576,16 @@ describe("openapi routes", () => { expect(dismiss.description).toContain("viewers may dismiss their own"); }); + it("tells agents that Agent Chat is always on instead of advertising a retired toggle", async () => { + const res = await request(createApp()).get("/api/openapi.json"); + const experimental = res.body.paths["/api/instance/settings/experimental"]; + for (const operation of [experimental.get, experimental.patch]) { + expect(operation.description).toContain("Agent Chat is always on"); + expect(operation.description).toContain("enableAgentChat is a retired compatibility key"); + expect(operation.description).toContain("tell them it is already on"); + } + }); + it("tells agents that archiving keeps a status card and where a person restores it", async () => { const res = await request(createApp()).get("/api/openapi.json"); const patch = res.body.paths["/api/status-cards/{id}"].patch; diff --git a/server/src/__tests__/plugin-config-validator.test.ts b/server/src/__tests__/plugin-config-validator.test.ts index 70648a21279..e47eb8efe84 100644 --- a/server/src/__tests__/plugin-config-validator.test.ts +++ b/server/src/__tests__/plugin-config-validator.test.ts @@ -48,3 +48,52 @@ describe("validateInstanceConfig secret-ref fields", () => { }); }); }); + +describe("validateInstanceConfig secret-ref array items", () => { + const arraySchema = { + type: "object", + properties: { + keys: { type: "array", items: { type: "string", format: "secret-ref" } }, + accounts: { + type: "array", + items: { + type: "object", + properties: { name: { type: "string" }, token: { type: "string", format: "secret-ref" } }, + required: ["name", "token"], + additionalProperties: false, + }, + }, + }, + additionalProperties: false, + } as unknown as JsonSchema; + + it("accepts bindings the settings form saves in array items and nested array objects", () => { + const configJson = { + keys: [binding, { ...binding, version: 3 }], + accounts: [{ name: "primary", token: binding }], + }; + expect(validateInstanceConfig(configJson, arraySchema)).toEqual({ valid: true }); + expect(configJson.keys[0]).toEqual(binding); + expect(configJson.accounts[0]!.token).toEqual(binding); + }); + + it("still rejects malformed bindings and wrong types inside arrays", () => { + expect(validateInstanceConfig({ keys: [binding, { type: "secret_ref", secretId: "nope" }] }, arraySchema)).toMatchObject({ + valid: false, + errors: [{ field: "/keys/1", message: "must be string" }], + }); + expect(validateInstanceConfig({ accounts: [{ name: binding, token: binding }] }, arraySchema)).toMatchObject({ + valid: false, + errors: [{ field: "/accounts/0/name", message: "must be string" }], + }); + }); +}); + +describe("validateInstanceConfig secret-ref parsing matches the secrets handler", () => { + it("rejects a padded secret id that the secrets handler would also refuse", () => { + expect(validateInstanceConfig({ apiKey: { ...binding, secretId: ` ${SECRET_ID} ` } }, schema)).toMatchObject({ + valid: false, + errors: [{ field: "/apiKey", message: "must be string" }], + }); + }); +}); diff --git a/server/src/__tests__/public-mcp.test.ts b/server/src/__tests__/public-mcp.test.ts index 808bb56847c..c5e406b360a 100644 --- a/server/src/__tests__/public-mcp.test.ts +++ b/server/src/__tests__/public-mcp.test.ts @@ -466,6 +466,8 @@ describe.skipIf(!support.supported)("public MCP OAuth and tool boundary", () => ] as Request["actor"][]) { actor = candidate; expect((await request(app).get("/api/mcp/setup")).status).toBe(401); + expect((await request(app).get("/api/mcp/connections")).status).toBe(401); + expect((await request(app).delete(`/api/mcp/connections/${randomUUID()}`).set("Origin", config.origin)).status).toBe(401); } for (const source of ["session", "cloud_tenant"] as const) { actor = { type: "board", source, userId: randomUUID() }; diff --git a/server/src/routes/authz.ts b/server/src/routes/authz.ts index 7be801f0faa..8b39815fc45 100644 --- a/server/src/routes/authz.ts +++ b/server/src/routes/authz.ts @@ -46,15 +46,19 @@ const AGENT_PROFILE_TABS: Record = { // The board page that holds the control behind a board-only API route, when one // is known. Paths keep for the agent to fill from its own task identifier. -const BOARD_CONTROL_PAGES: Array<[RegExp, (match: RegExpExecArray) => [string, string]]> = [ +const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +const BOARD_CONTROL_PAGES: Array<[RegExp, (match: RegExpExecArray, req: Request) => [string, string] | null]> = [ [/^\/(?:companies\/[^/]+\/)?secrets?(?:[-/]|$)/, () => ["company secrets", "//company/settings/secrets"]], [/^\/companies\/[^/]+\/(?:budgets|budget-incidents|costs)(?:\/|$)/, () => ["budgets", "//activity/budgets"]], [/^\/companies\/[^/]+\/(?:prompt-injection-prevention|decision-model)(?:\/|$)/, () => ["connection settings", "//company/settings/connections"]], [/^\/companies\/[^/]+\/(?:members|invites|join-requests)(?:\/|$)/, () => ["company members", "//company/settings/members"]], [/^\/companies\/[^/]+\/(?:tools\/)?gateways(?:\/|$)|^\/tool-gateway\//, () => ["tool gateways", "//apps/gateways"]], [/^\/approvals\/([^/]+)/, match => ["the approval", `//approvals/${match[1]}`]], - // Project sharing is under the project's Configuration tab (Manage access). - [/^\/projects\/([0-9a-f-]{36})\/access-members(?:\/|$)/i, match => ["the project's access settings", `//projects/${match[1]}/configuration`]], + [/^\/projects\/([^/]+)\/access-members(?:\/|$)/, (match, req) => { + const projectId = [req.params?.id, match[1]].find((id): id is string => typeof id === "string" && UUID_PATTERN.test(id)); + return projectId ? ["the project's access settings", `//projects/${projectId}/configuration`] : null; + }], [/^\/agents\/([0-9a-f-]{36})(?:\/([^/]+))?(?:\/|$)/i, match => { const tab = match[2] && Object.hasOwn(AGENT_PROFILE_TABS, match[2]) ? AGENT_PROFILE_TABS[match[2]] : undefined; return tab ? [`the agent's ${tab[0]}`, `//agents/${match[1]}/${tab[1]}`] : ["the agent's profile", `//agents/${match[1]}`]; @@ -63,13 +67,15 @@ const BOARD_CONTROL_PAGES: Array<[RegExp, (match: RegExpExecArray) => [string, s ]; // Plugin management pages. A plugin id or key segment opens that plugin's settings -// (the page accepts either); any other segment (install, tools) opens the manager. +// (the page accepts either); a plugin-manager endpoint segment opens the manager. // An operator can hide plugin management, so link it only while it is visible. -const PLUGIN_REFERENCE = /^(?:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|[a-z0-9][a-z0-9_-]*(?:\.[a-z0-9_-]+)+)$/i; +const PLUGIN_REFERENCE = /^[a-z0-9][a-z0-9._-]*$/i; +const PLUGIN_MANAGER_SEGMENTS = new Set(["examples", "install", "tools", "ui-contributions"]); function pluginControlPage(path: string): [string, string] | null { const match = /^\/plugins(?:\/([^/]+))?(?:\/|$)/.exec(path); if (!match || getHiddenSettings().has("instance.plugins")) return null; - const plugin = match[1] && PLUGIN_REFERENCE.test(match[1]) ? match[1] : null; + const segment = match[1]; + const plugin = segment && PLUGIN_REFERENCE.test(segment) && !PLUGIN_MANAGER_SEGMENTS.has(segment) ? segment : null; return plugin ? ["the plugin's settings", `//company/settings/instance/plugins/${plugin}`] : ["plugin settings", "//company/settings/instance/plugins"]; @@ -81,7 +87,7 @@ function boardControlPage(req: Request) { if (pluginPage) return pluginPage; for (const [pattern, page] of BOARD_CONTROL_PAGES) { const match = pattern.exec(path); - if (match) return page(match); + if (match) return page(match, req); } return null; } diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 254b6b64541..0ce9f81adcf 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -7130,6 +7130,7 @@ registry.registerPath({ path: "/api/instance/settings/experimental", tags: ["instance"], summary: "Get experimental instance settings", + description: "Agent Chat is always on for every person on every instance and needs no setup. enableAgentChat is a retired compatibility key: it always reads true and written values are ignored. If someone asks to enable Agent Chat, tell them it is already on, since they are using it now; do not send them to a settings toggle.", responses: { 200: r.ok(), 401: r.unauthorized }, }); @@ -7138,7 +7139,7 @@ registry.registerPath({ path: "/api/instance/settings/experimental", tags: ["instance"], summary: "Update experimental instance settings", - description: "Requires instance-admin access or a trusted Cloud stack admin. Operator-hidden settings and Cloud-managed feature restrictions still apply. This does not grant access to other instance administration operations.", + description: "Requires instance-admin access or a trusted Cloud stack admin. Operator-hidden settings and Cloud-managed feature restrictions still apply. This does not grant access to other instance administration operations. Agent Chat is always on for every person on every instance and needs no setup. enableAgentChat is a retired compatibility key: it always reads true and written values are ignored. If someone asks to enable Agent Chat, tell them it is already on, since they are using it now; do not send them to a settings toggle.", request: { body: jsonBody(patchInstanceExperimentalSettingsSchema) }, responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden }, }); @@ -12967,10 +12968,10 @@ registerCurrentRoute({ body: mcpConsentSchema.extend({ userCode: z.string().min(8).max(12) }), responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden }, }); -/** Agents read this in API search; without it they keep searching for an agent route that does not exist. */ const ASSISTANT_CONNECTIONS_PERSON_ONLY = "Personal and person-only: only the signed-in person can list or revoke their own assistant connections, in their browser on " + "[Assistant connections](//apps/assistant-connection), with this conversation's company prefix as . " - + "No agent credential, tool or other API route can do it, so do not search for one; link that page and tell the person which connection to revoke. " + + "An agent cannot do it for them: these routes accept only the person's own signed-in browser session, and the only other revocation is OAuth token revocation sent by the connected assistant itself with its own token. " + + "Do not search for another route; link that page and tell the person which connection to revoke. " + "Revoking stops that assistant's further calls; tasks it already created keep their assignees and running work."; registerCurrentRoute({ method: "get", path: "/api/mcp/connections", tags: ["tool-gateway"], diff --git a/server/src/services/controller-instance-lease.ts b/server/src/services/controller-instance-lease.ts index 36ff6f52412..48ebd317e54 100644 --- a/server/src/services/controller-instance-lease.ts +++ b/server/src/services/controller-instance-lease.ts @@ -2,7 +2,7 @@ import { and, eq, isNull, sql } from "drizzle-orm"; import { controllerInstances, type Db } from "@paperclipai/db"; import { logger } from "../middleware/logger.js"; import { - currentControllerBootId, currentControllerDeployment, currentProcessSpaceHash, onControllerStamp, + currentControllerBootId, currentProcessSpaceHash, onControllerStamp, } from "./controller-process-identity.js"; import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js"; import { DELIVERY_QUEUES } from "./delivery-work-notifications.js"; @@ -54,14 +54,8 @@ type LeaseState = { let state: LeaseState | null = null; let registeredDb: Db | null = null; -/** A Railway deployment with an exclusive volume proves a replaced deployment on its own; every - * other host (Docker, Kubernetes, Fly, a stateless Railway service, a rebooted machine) needs the - * lease to prove a replaced process. PAPERCLIP_CONTROLLER_INSTANCE_LEASE=on|off overrides this. */ export function controllerInstanceLeaseEnabled(env: NodeJS.ProcessEnv = process.env): boolean { - const setting = env.PAPERCLIP_CONTROLLER_INSTANCE_LEASE; - if (setting === "off") return false; - if (setting === "on") return true; - return currentControllerDeployment(env)?.exclusiveVolume !== true; + return env.PAPERCLIP_CONTROLLER_INSTANCE_LEASE !== "off"; } function exitOnLostLease(reason: "fenced" | "lease_lost") { diff --git a/server/src/services/heartbeat/recovery.test.ts b/server/src/services/heartbeat/recovery.test.ts index dc27c7641a5..f031b88878c 100644 --- a/server/src/services/heartbeat/recovery.test.ts +++ b/server/src/services/heartbeat/recovery.test.ts @@ -211,6 +211,36 @@ describePostgres("heartbeat recovery module ownership", () => { return (await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, issueId, invocationSource: "automation", status: "running", runtimeMode: "legacy" }).returning())[0]; } + it.each([false, true])("delivers scoped recovery to a preparation without a runner PID or checkpoint (locally tracked=%s)", async tracked => { + const run = await seedRun(), deps = callbacks(db); + await db.update(agents).set({ adapterType: "paperclip_runner" }).where(eq(agents.id, run.agentId)); + await db.update(heartbeatRuns).set({ runtimeMode: "native", nativeIssueId: run.issueId, + nativePhase: "observed", runnerProfileJson: { nativePreparationFencingVersion: 1 }, + }).where(eq(heartbeatRuns.id, run.id)); + await db.insert(nativeRunFinalizations).values({ runId: run.id, companyId: run.companyId, issueId: run.issueId!, + phase: "observed", leaseOwner: "old-preparation", leaseExpiresAt: new Date(0), controllerHostHash: "operator_attested" }); + const claim: NativeRestartRecoveryClaim = { kind: "bootstrap_incomplete", runId: run.id, leaseOwner: "recovered-owner", + controllerGeneration: 2, providerAttempt: 1, restartKind: "hard", recoveryRequestId: null }; + native.claims.mockResolvedValue([claim]); + if (tracked) deps.activeRunExecutions.add(run.id); + try { + await createHeartbeatRecovery(db, deps).recoverActiveExecutions({ companyId: run.companyId, runId: run.id }); + if (tracked) { + expect(native.claims).not.toHaveBeenCalled(); + expect(deps.executeRun).not.toHaveBeenCalled(); + } else { + expect(native.claims).toHaveBeenCalledWith(expect.objectContaining({ runIds: [run.id] })); + expect(deps.executeRun).toHaveBeenCalledExactlyOnceWith(run.id, { nativeLeaseOwner: claim.leaseOwner, nativeRestartRecovery: claim }); + } + expect(deps.setRunStatusFromLive).not.toHaveBeenCalled(); + expect(deps.terminateHeartbeatRunProcess).not.toHaveBeenCalled(); + } finally { + await Promise.all([...deps.activeRunExecutionPromises]); + await db.delete(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, run.id)); + await db.delete(heartbeatRuns).where(eq(heartbeatRuns.id, run.id)); + } + }); + it("routes an expired pre-capability startup to cleanup before generic orphan ownership holds", async () => { const run = await seedRun(), leaseId = randomUUID(), environmentId = randomUUID(), leaseOwner = randomUUID(); const deadlineAtMs = Date.now() - 1_000, contextId = randomUUID(); diff --git a/server/src/services/heartbeat/upgrade-readiness.test.ts b/server/src/services/heartbeat/upgrade-readiness.test.ts index fb605c53be1..e5f92444e3a 100644 --- a/server/src/services/heartbeat/upgrade-readiness.test.ts +++ b/server/src/services/heartbeat/upgrade-readiness.test.ts @@ -36,6 +36,17 @@ describe("upgrade readiness", () => { it("never treats a truncated census as complete", () => { const rows = Array.from({ length: 1_001 }, (_, i) => ({ id: String(i), recoverable: true })); expect(report({ rows, activeRunIds: [] })).toMatchObject({ ready: false, truncated: true }); + expect(report({ rows: rows.map(row => ({ ...row, recoverable: false, retainedUnowned: true })), + before: idle, after: idle, activeRunIds: [] })).toMatchObject({ ready: false, truncated: true, activeRuns: 0 }); + }); + it("keeps current execution and admission barriers with retained recovery work", () => { + const rows = [{ id: "retained", recoverable: false, retainedUnowned: true }, { id: "remote", recoverable: true }]; + expect(report({ rows })).toMatchObject({ ready: true, activeRuns: 1, recoverableRuns: 1, retainedBlockedRuns: 1 }); + expect(report({ rows, activeRunIds: ["remote", "retained"], after: { ...idle, activeRuns: 2 } })) + .toMatchObject({ ready: false, activeRuns: 2, blockingRuns: 1, retainedBlockedRuns: 0 }); + for (const before of [{ ...idle, activeRuns: 2 }, { ...idle, pendingWakes: 1 }, { ...idle, draining: true }]) { + expect(report({ rows, before }).ready).toBe(false); + } }); }); @@ -54,6 +65,80 @@ postgresTests("upgrade readiness persisted execution evidence", () => { db = createDb(database.connectionString); }, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS); afterAll(async () => { await db?.$client.end(); await database?.cleanup(); vi.unstubAllEnvs(); }); + it("keeps an old unowned preparation visible without making it a live upgrade blocker", async () => { + const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID(), runId = randomUUID(); + const owner = await currentNativeControllerIdentity(); + await db.insert(companies).values({ id: companyId, name: "Retained preparation", issuePrefix: `R${companyId.slice(0, 6)}` }); + await db.insert(agents).values({ id: agentId, companyId, name: "Retained preparation", role: "engineer", adapterType: "paperclip_runner" }); + await db.insert(issues).values({ id: issueId, companyId, title: "Interrupted preparation", status: "in_progress", assigneeAgentId: agentId }); + await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, issueId, nativeIssueId: issueId, + status: "running", runtimeMode: "native", nativePhase: "observed", runnerInstanceId: randomUUID(), nativeSessionId: randomUUID() }); + const retained = { controllerBootId: "previous-controller", leaseOwner: "previous-controller-lease", controllerGeneration: 1, + leaseExpiresAt: new Date(0), recoveryState: "awaiting_evidence", phase: "observed", failureCode: null }; + await db.insert(nativeRunFinalizations).values({ runId, companyId, issueId, ...retained }); + const input = { activeRunIds: new Set(), counters: () => idle }; + try { + const savedRun = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + const savedOwner = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, runId)); + expect(await readUpgradeReadiness(db, input)).toMatchObject({ ready: true, activeRuns: 0, + recoverableRuns: 0, blockingRuns: 0, retainedBlockedRuns: 1, + nativeControllerRecovery: { activeExecutionAdoption: { retainedBlockedRuns: 1 } } }); + const [{ default: express }, { default: request }, { instanceSettingsRoutes }, { errorHandler }] = await Promise.all([ + import("express"), import("supertest"), import("../../routes/instance-settings.js"), import("../../middleware/error-handler.js"), + ]); + const app = express(); + let isInstanceAdmin = false; + app.use((req, _res, next) => { + req.actor = { type: "board", userId: "readiness-test", source: "session", isInstanceAdmin }; + next(); + }); + app.use("/api", instanceSettingsRoutes(db)); + app.use(errorHandler); + await request(app).get("/api/instance/task-drain?upgradeReadiness=1").expect(403); + isInstanceAdmin = true; + const response = await request(app).get("/api/instance/task-drain?upgradeReadiness=1").expect(200); + expect(response.body.upgradeReadiness).toMatchObject({ ready: true, activeRuns: 0, blockingRuns: 0, retainedBlockedRuns: 1 }); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId))).toEqual(savedRun); + expect(await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, runId))).toEqual(savedOwner); + expect(await db.select({ status: heartbeatRuns.status }).from(heartbeatRuns).where(eq(heartbeatRuns.id, runId))) + .toEqual([{ status: "running" }]); + expect(await db.select({ state: nativeRunFinalizations.recoveryState }).from(nativeRunFinalizations) + .where(eq(nativeRunFinalizations.runId, runId))).toEqual([{ state: "awaiting_evidence" }]); + for (const patch of [ + { controllerBootId: owner.bootId }, { controllerBootId: null }, { controllerBootId: "" }, + { leaseExpiresAt: new Date(Date.now() + 60_000) }, { leaseExpiresAt: null }, + { leaseOwner: null }, { controllerGeneration: 0 }, { failureCode: "uncertain" }, + { recoveryState: null }, { phase: "workspace_finalizing" }, + ]) { + await db.update(nativeRunFinalizations).set(patch).where(eq(nativeRunFinalizations.runId, runId)); + expect(await readUpgradeReadiness(db, input)).toMatchObject({ ready: false, blockingRuns: 1, retainedBlockedRuns: 0 }); + await db.update(nativeRunFinalizations).set(retained).where(eq(nativeRunFinalizations.runId, runId)); + } + for (const patch of [{ runtimeMode: "legacy" }, { nativePhase: "running" }, { processPid: process.pid }, { processStartedAt: new Date() }]) { + await db.update(heartbeatRuns).set(patch).where(eq(heartbeatRuns.id, runId)); + expect(await readUpgradeReadiness(db, input)).toMatchObject({ ready: false, blockingRuns: 1, retainedBlockedRuns: 0 }); + await db.update(heartbeatRuns).set({ runtimeMode: "native", nativePhase: "observed", processPid: null, processStartedAt: null }) + .where(eq(heartbeatRuns.id, runId)); + } + await db.insert(nativeRunnerCheckpoints).values({ runId, companyId, issueId, part: "header", material: {} }); + expect(await readUpgradeReadiness(db, input)).toMatchObject({ ready: false, blockingRuns: 1, retainedBlockedRuns: 0 }); + await db.delete(nativeRunnerCheckpoints).where(eq(nativeRunnerCheckpoints.runId, runId)); + await db.update(agents).set({ adapterType: "process" }).where(eq(agents.id, agentId)); + expect(await readUpgradeReadiness(db, input)).toMatchObject({ ready: false, blockingRuns: 1, retainedBlockedRuns: 0 }); + await db.update(agents).set({ adapterType: "paperclip_runner" }).where(eq(agents.id, agentId)); + input.activeRunIds.add(runId); + expect(await readUpgradeReadiness(db, { ...input, counters: () => ({ ...idle, activeRuns: 1 }) })) + .toMatchObject({ ready: false, activeRuns: 1, blockingRuns: 1, retainedBlockedRuns: 0 }); + input.activeRunIds.clear(); + let reads = 0; + expect(await readUpgradeReadiness(db, { ...input, counters: () => { + if (reads++ > 0) input.activeRunIds.add(runId); + return { ...idle, activeRuns: input.activeRunIds.size }; + } })).toMatchObject({ ready: false, blockingRuns: 1, retainedBlockedRuns: 0 }); + } finally { + await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, runId)); + } + }); it("requires a bound checkpoint and current owner rather than a native label or live process alone", async () => { const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID(), runId = randomUUID(); const owner = await currentNativeControllerIdentity(); diff --git a/server/src/services/heartbeat/upgrade-readiness.ts b/server/src/services/heartbeat/upgrade-readiness.ts index e8a2d5663f3..23952159227 100644 --- a/server/src/services/heartbeat/upgrade-readiness.ts +++ b/server/src/services/heartbeat/upgrade-readiness.ts @@ -12,7 +12,7 @@ import { readNativeFinalizationUpgradeReadiness } from "../native-runtime/native const MAX_UPGRADE_RUNS = 1_000; const POST_RESULT_INSPECTION_BUDGET_MS = 2_000; type Counters = { activeRuns: number; pendingWakes: number; draining: boolean }; -type Candidate = { id: string; recoverable: boolean }; +type Candidate = { id: string; recoverable: boolean; retainedUnowned?: boolean }; /** An observation for Cloud's optional rollout policy, never execution authority. * Only explicit upgrade probes read the database; ordinary drain reads stay local. @@ -35,6 +35,13 @@ export async function readUpgradeReadiness(db: Db, input: { r.runner_profile_json->'remoteWorkspace' as "remoteWorkspace", r.runner_profile_json ? 'remoteWorkspace' as "hasRemoteWorkspace", p.id as "pluginLeaseId", f.result_id is not null as "hasResult", + (r.runtime_mode = 'native' and a.adapter_type = 'paperclip_runner' and r.native_phase = 'observed' + and f.recovery_state = 'awaiting_evidence' and f.phase = 'observed' and f.result_id is null and f.failure_code is null + and f.controller_boot_id is not null and f.controller_boot_id <> '' and f.controller_boot_id <> ${owner.bootId} + and f.controller_generation > 0 and f.lease_owner is not null and f.lease_owner <> '' + and f.lease_expires_at <= clock_timestamp() + and r.process_pid is null and r.process_started_at is null and c.run_id is null + ) is true as "retainedUnowned", case when pg_column_size(c.material) <= ${NATIVE_CHECKPOINT_HEADER_INSPECTION_MAX_BYTES} and sum(pg_column_size(c.material)) over () <= ${NATIVE_CHECKPOINT_INSPECTION_TOTAL_MAX_BYTES} then c.material else null end as material, ( r.runtime_mode = 'native' and a.adapter_type = 'paperclip_runner' @@ -79,7 +86,7 @@ export async function readUpgradeReadiness(db: Db, input: { && performance.now() < inspectionDeadline ? await readNativeFinalizationUpgradeReadiness(db, { companyId: row.companyId, runId: row.id, controller: owner, supportedOutageMs: 300_000 }).catch(() => false) : false; - candidates.push({ id: row.id, recoverable: bound && (row.recoverable || finalization) }); + candidates.push({ id: row.id, recoverable: bound && (row.recoverable || finalization), retainedUnowned: row.retainedUnowned }); } // Additive evidence does not strengthen or redefine the existing ready flag. const retained = await readNativeControllerRecoveryInventory(db).catch(() => ({ @@ -89,7 +96,8 @@ export async function readUpgradeReadiness(db: Db, input: { const summary = summarizeUpgradeReadiness({ rows: candidates, before, after: input.counters(), activeRunIds: [...new Set([...beforeIds, ...input.activeRunIds])], controllerBootId: owner.bootId }); return { ...summary, nativeControllerRecovery: { activeExecutionAdoption: { - recoverableRuns: summary.recoverableRuns, blockingRuns: summary.blockingRuns, truncated: summary.truncated, + recoverableRuns: summary.recoverableRuns, blockingRuns: summary.blockingRuns, + retainedBlockedRuns: summary.retainedBlockedRuns, truncated: summary.truncated, }, retained } }; } @@ -97,10 +105,13 @@ export function summarizeUpgradeReadiness(input: { rows: readonly Candidate[]; before: Counters; after: Counters; activeRunIds: readonly string[]; controllerBootId: string; }) { - const eligible = new Set(input.rows.filter(row => row.recoverable).map(row => row.id)); - const observedIds = new Set(input.rows.map(row => row.id)); + const activeIds = new Set(input.activeRunIds); + const retained = input.rows.filter(row => row.retainedUnowned === true && !activeIds.has(row.id)); + const executions = input.rows.filter(row => row.retainedUnowned !== true || activeIds.has(row.id)); + const eligible = new Set(executions.filter(row => row.recoverable).map(row => row.id)); + const observedIds = new Set(executions.map(row => row.id)); const unknown = input.activeRunIds.filter(id => !observedIds.has(id)).length; - const activeRuns = Math.max(input.rows.length + unknown, input.before.activeRuns, input.after.activeRuns); + const activeRuns = Math.max(executions.length + unknown, input.before.activeRuns, input.after.activeRuns); const recoverableRuns = eligible.size; const blockingRuns = activeRuns - recoverableRuns; const pendingWakes = Math.max(input.before.pendingWakes, input.after.pendingWakes); @@ -108,6 +119,6 @@ export function summarizeUpgradeReadiness(input: { const truncated = input.rows.length > MAX_UPGRADE_RUNS; return { version: 1 as const, observedAt: new Date().toISOString(), controllerBootId: input.controllerBootId, ready: !draining && !truncated && blockingRuns === 0 && pendingWakes === 0, - activeRuns, recoverableRuns, blockingRuns, pendingWakes, draining, truncated, + activeRuns, recoverableRuns, blockingRuns, retainedBlockedRuns: retained.length, pendingWakes, draining, truncated, supportedOutageMs: 300_000, recoveryGraceMs: REMOTE_RUNNER_RECONNECT_GRACE_MS }; } diff --git a/server/src/services/native-runtime/native-accepted-result-lease-settlement.ts b/server/src/services/native-runtime/native-accepted-result-lease-settlement.ts index 928fc9dcc5c..fad566bd33b 100644 --- a/server/src/services/native-runtime/native-accepted-result-lease-settlement.ts +++ b/server/src/services/native-runtime/native-accepted-result-lease-settlement.ts @@ -103,7 +103,7 @@ export async function requestNativeAcceptedResultLeaseOperatorAction(db: Db, rec if (!held) return; await requestNativeControllerStopConfirmation(db, { runId: record.recordId, companyId: record.companyId, issueId: record.issueId, - controllerBootId: stamp.bootId, controllerPid: stamp.pid, + controllerBootId: stamp.bootId, controllerPid: stamp.pid, controllerHostHash: null, controllerProcessStartedAt: stamp.processStartedAt ? new Date(stamp.processStartedAt) : null, }); } diff --git a/server/src/services/native-runtime/native-controller-host-recovery.test.ts b/server/src/services/native-runtime/native-controller-host-recovery.test.ts index 1d3c4c61e52..dec4b4ffdcf 100644 --- a/server/src/services/native-runtime/native-controller-host-recovery.test.ts +++ b/server/src/services/native-runtime/native-controller-host-recovery.test.ts @@ -77,4 +77,28 @@ describe("native controller rows saved before their host was recorded", () => { await expect(confirmNativeControllerStopped({ db, companyId, issueId, actionId: actions[0]!.id, runId, stopEvidence, actorId: "board-user" })).rejects.toMatchObject({ status: 409 }); }); + + it("binds stop confirmation to an exact foreign-host controller and rejects changed ownership", async () => { + const { companyId, issueId, runId, coordinator } = await seed(); + const owner = { ...coordinator, controllerHostHash: "previous-pid-space" }; + await db.update(nativeRunFinalizations).set({ controllerHostHash: owner.controllerHostHash }) + .where(eq(nativeRunFinalizations.runId, runId)); + await requestNativeControllerStopConfirmation(db, owner); + const [action] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, issueId)); + const input = { db, companyId, issueId, runId, actionId: action!.id, actorId: "board-user", + stopEvidence: "The hosting provider confirms the exact previous container stopped." }; + for (const patch of [{ controllerHostHash: "changed-space" }, { controllerBootId: randomUUID() }, + { controllerPid: 1_999_999_999 }, { controllerProcessStartedAt: new Date(owner.controllerProcessStartedAt!.getTime() + 60_000) }]) { + await db.update(nativeRunFinalizations).set(patch).where(eq(nativeRunFinalizations.runId, runId)); + await expect(confirmNativeControllerStopped(input)).rejects.toMatchObject({ status: 409 }); + await db.update(nativeRunFinalizations).set({ controllerHostHash: owner.controllerHostHash, + controllerBootId: owner.controllerBootId, controllerPid: owner.controllerPid, + controllerProcessStartedAt: owner.controllerProcessStartedAt }).where(eq(nativeRunFinalizations.runId, runId)); + } + await expect(confirmNativeControllerStopped(input)).resolves.toEqual({ runId, status: "queued" }); + const [attested] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, runId)); + expect(await evaluateNativeControllerTakeover({ owner: attested!, now: new Date() })) + .toEqual({ allowed: true, reason: "operator_attested_controller_stopped" }); + expect((await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)))[0]?.status).toBe("running"); + }); }); diff --git a/server/src/services/native-runtime/native-controller-host-recovery.ts b/server/src/services/native-runtime/native-controller-host-recovery.ts index 84b482dcc83..ae424e1741e 100644 --- a/server/src/services/native-runtime/native-controller-host-recovery.ts +++ b/server/src/services/native-runtime/native-controller-host-recovery.ts @@ -5,12 +5,6 @@ import { conflict } from "../../errors.js"; import { persistActivity, publishActivity } from "../activity-log.js"; import { notifyLiveRunRecovery } from "../live-run-recovery-notifications.js"; -/** - * An ephemeral (multi-host) server cannot judge a native controller row saved - * before controller_host_hash existed: its PID may name a process on another - * host. Restart recovery leaves such a row waiting and asks the board, through - * this recovery action, to confirm that the previous server stopped. - */ export const NATIVE_CONTROLLER_HOST_UNVERIFIED = "native_controller_host_unverified"; /** controller_host_hash value written once the board confirms the stop. */ export const OPERATOR_ATTESTED_CONTROLLER_HOST = "operator_attested"; @@ -20,7 +14,7 @@ const changed = () => conflict("The recorded controller is no longer current. Re { code: "native_controller_host_recovery_stale" }); type Coordinator = Pick; + "runId" | "companyId" | "issueId" | "controllerBootId" | "controllerPid" | "controllerProcessStartedAt" | "controllerHostHash">; /** Idempotent. Another active recovery action on the task keeps precedence. */ export async function requestNativeControllerStopConfirmation(tx: Db, coordinator: Coordinator) { @@ -29,9 +23,10 @@ export async function requestNativeControllerStopConfirmation(tx: Db, coordinato status: "active", ownerType: "board", cause: NATIVE_CONTROLLER_HOST_UNVERIFIED, fingerprint: fingerprint(coordinator.runId), attemptCount: 1, evidence: { runId: coordinator.runId, controller: { bootId: coordinator.controllerBootId, pid: coordinator.controllerPid, + hostHash: coordinator.controllerHostHash, processStartedAt: coordinator.controllerProcessStartedAt?.toISOString() ?? null } }, - nextAction: "This run's previous server was recorded before Paperclip stored which host it ran on, so Paperclip " - + "cannot verify that it stopped. Confirm that the previous server process has stopped (for example, its " + nextAction: "Paperclip cannot verify that this run's previous server stopped from the current host. " + + "Confirm that the previous server process has stopped (for example, its " + "container was replaced), and the run resumes from its saved state.", }).onConflictDoNothing(); } @@ -57,13 +52,15 @@ export async function confirmNativeControllerStopped(input: { const recorded = (action?.evidence?.controller ?? {}) as Record; if (!coordinator || !action || !run || action.cause !== NATIVE_CONTROLLER_HOST_UNVERIFIED || action.ownerType !== "board" || !["active", "escalated"].includes(action.status) || action.evidence?.runId !== input.runId - || coordinator.controllerHostHash !== null || coordinator.controllerPid !== recorded.pid - || coordinator.controllerBootId !== recorded.bootId) throw changed(); + || coordinator.controllerHostHash !== (recorded.hostHash ?? null) || coordinator.controllerPid !== recorded.pid + || coordinator.controllerBootId !== recorded.bootId + || (coordinator.controllerProcessStartedAt?.toISOString() ?? null) !== recorded.processStartedAt) throw changed(); // Work intent before the write it announces (sleep-aware background work). await notifyLiveRunRecovery(tx, { companyId: input.companyId, runId: input.runId, agentId: run.agentId }); const now = new Date(); const updated = await tx.update(nativeRunFinalizations).set({ controllerHostHash: OPERATOR_ATTESTED_CONTROLLER_HOST, updatedAt: now }) - .where(and(eq(nativeRunFinalizations.runId, input.runId), isNull(nativeRunFinalizations.controllerHostHash), + .where(and(eq(nativeRunFinalizations.runId, input.runId), coordinator.controllerHostHash === null + ? isNull(nativeRunFinalizations.controllerHostHash) : eq(nativeRunFinalizations.controllerHostHash, coordinator.controllerHostHash), eq(nativeRunFinalizations.controllerPid, coordinator.controllerPid!), eq(nativeRunFinalizations.controllerBootId, coordinator.controllerBootId!))) .returning({ runId: nativeRunFinalizations.runId }); diff --git a/server/src/services/native-runtime/native-restart-recovery.test.ts b/server/src/services/native-runtime/native-restart-recovery.test.ts index 3f88219608a..71406202ae5 100644 --- a/server/src/services/native-runtime/native-restart-recovery.test.ts +++ b/server/src/services/native-runtime/native-restart-recovery.test.ts @@ -1,4 +1,4 @@ -import { currentProcessSpaceHash } from "../controller-process-identity.js"; +import { currentControllerBootId, currentProcessSpaceHash } from "../controller-process-identity.js"; import { afterEach, describe, expect, it, vi } from "vitest"; import { @@ -297,6 +297,14 @@ describe("native controller takeover fencing", () => { now, isProcessAlive: () => true })).resolves.toEqual({ allowed: true, reason: "operator_attested_controller_stopped" }); }); + it("preserves same-space process proof when boot metadata is present", async () => { + const local = { ...owner(), controllerBootId: currentControllerBootId(), controllerHostHash: currentProcessSpaceHash() }; + expect(await evaluateNativeControllerTakeover({ owner: local, now, isProcessAlive: () => false })) + .toEqual({ allowed: true, reason: "controller_process_dead" }); + expect(await evaluateNativeControllerTakeover({ owner: local, now, isProcessAlive: () => true, + readProcessStartedAt: async () => recordedStart })).toEqual({ allowed: false, reason: "controller_still_alive" }); + }); + it.each([ ["another host", "another-host-pid-space"], ["an unreadable PID space", "unknown"], diff --git a/server/src/services/native-runtime/native-restart-recovery.ts b/server/src/services/native-runtime/native-restart-recovery.ts index 23f9ba7c2e6..14d3954a745 100644 --- a/server/src/services/native-runtime/native-restart-recovery.ts +++ b/server/src/services/native-runtime/native-restart-recovery.ts @@ -1,4 +1,6 @@ import { readPendingNativeRunStop } from "./native-cancellation-request.js"; +import { CONTROLLER_LEASE_RENEW_MS } from "../controller-instance-lease.js"; +import { currentControllerProcessStamp, UNKNOWN_PROCESS_SPACE } from "../controller-process-identity.js"; import { readNativeSessionResumptionAgent, restoreNativeSessionExecution } from "./native-session-resumption-admission.js"; import { notifyLiveRunRecovery } from "../live-run-recovery-notifications.js"; import { NATIVE_RETAINED_OWNER_WAIT } from "./native-controller-recovery.js"; @@ -401,6 +403,8 @@ export async function evaluateNativeControllerTakeover(input: { * existed: those keep the previous local proof for one release, outside * ephemeral mode only (see LEGACY_HOST_FALLBACK_EVENT). */ controllerHostHash?: string | null; + controllerBootId?: string | null; + updatedAt?: Date; }; now: Date; coordinatedPreviousController?: { @@ -454,15 +458,19 @@ export async function evaluateNativeControllerTakeover(input: { pidExists: input.isProcessAlive ?? base.pidExists, readStartedAt: input.readProcessStartedAt ?? base.readStartedAt, }; + const needsBootProof = owner.controllerHostHash != null + && (owner.controllerHostHash !== probe.processSpace || owner.controllerHostHash === UNKNOWN_PROCESS_SPACE); const proof = await proveOwnerGone({ kind: "local", scope: "native_controller", generation: null, host: null, processSpace: recordsHost ? owner.controllerHostHash ?? null : probe.processSpace, processSpaceUnrecorded: recordsHost && owner.controllerHostHash == null, - bootId: null, pid: priorPid, processStartedAt: owner.controllerProcessStartedAt?.toISOString() ?? null, + bootId: needsBootProof ? owner.controllerBootId ?? null : null, touchedAt: owner.updatedAt?.toISOString() ?? null, + pid: priorPid, processStartedAt: owner.controllerProcessStartedAt?.toISOString() ?? null, processGroupId: null, }, probe); const fallback = proof.legacyHostFallback ? { legacyHostFallback: true as const } : {}; if (proof.verdict === "gone") { + if (proof.reason === "controller_fenced" || proof.reason === "controller_stopped") return { allowed: true, reason: proof.reason }; if (proof.reason === "pid_reused") return { allowed: true, reason: "controller_pid_recycled", ...fallback }; return { allowed: true, @@ -473,6 +481,7 @@ export async function evaluateNativeControllerTakeover(input: { ...fallback, }; } + if (proof.reason === "controller_lease_held" || proof.reason === "controller_lease_grace") return { allowed: false, reason: proof.reason }; if (proof.reason === "process_space_unrecorded") return { allowed: false, reason: "controller_host_unrecorded" }; if (proof.reason === "different_process_space" || proof.reason === "unknown_process_space" || proof.reason === "different_host") { @@ -1187,7 +1196,7 @@ export async function claimNativeRestartRecoveries(input: { input.coordinatedPreviousController ?? null, }); await recordLegacyHostFallbackProof(tx as unknown as Db, row.run, takeover, "native_restart_recovery"); - if (!takeover.allowed && takeover.reason === "controller_host_unrecorded") { + if (!takeover.allowed && ["controller_host_unrecorded", "controller_host_unprovable"].includes(takeover.reason)) { await requestNativeControllerStopConfirmation(tx as unknown as Db, row.coordinator); } if (!takeover.allowed) { @@ -1217,7 +1226,8 @@ export async function claimNativeRestartRecoveries(input: { // Repeated scans of one ownership wait record it once, so the // bounded history keeps the drain record the next claim reads. ...(waitAlreadyRecorded ? {} : { recoveryHistory: appendBoundedRecoveryHistory(event) }), - updatedAt: now, + updatedAt: waitAlreadyRecorded && row.coordinator.recoveryState === "awaiting_evidence" + && row.coordinator.recoveryRequestId === (input.recoveryRequestId ?? null) ? row.coordinator.updatedAt : now, }) .where( and( @@ -1230,7 +1240,9 @@ export async function claimNativeRestartRecoveries(input: { kind: "awaiting_evidence", runId: row.run.id, reason: takeover.reason, - ...(durableAuthority && row.coordinator.leaseExpiresAt ? { retryAt: row.coordinator.leaseExpiresAt } : {}), + ...(durableAuthority && row.coordinator.leaseExpiresAt ? { retryAt: row.coordinator.leaseExpiresAt } + : ["controller_lease_held", "controller_lease_grace"].includes(takeover.reason) + ? { retryAt: new Date(now.getTime() + CONTROLLER_LEASE_RENEW_MS) } : {}), } as const; } @@ -1648,6 +1660,7 @@ export async function claimNativeRestartRecoveries(input: { // The attempt the executor runs this resume as (see its execution claim). const progressAttempt = nextNativeProviderAttempt(providerAttempt, claimKind, !resumeKeepsAttempt); await notifyLiveRunRecovery(tx, { companyId: row.run.companyId, runId: row.run.id, agentId: row.run.agentId }); + currentControllerProcessStamp(); const claimed = await tx .update(nativeRunFinalizations) .set({ diff --git a/server/src/services/native-runtime/native-runner-checkpoint.ts b/server/src/services/native-runtime/native-runner-checkpoint.ts index f4b47b94e9c..feb6e6832ac 100644 --- a/server/src/services/native-runtime/native-runner-checkpoint.ts +++ b/server/src/services/native-runtime/native-runner-checkpoint.ts @@ -1,4 +1,5 @@ import { readNativeCheckpointRetirement } from "./native-runner-checkpoint-retirement.js"; +import { NATIVE_PREPARATION_FENCING_VERSION } from "./native-active-runs.js"; import { notifyLiveRunRecovery } from "../live-run-recovery-notifications.js"; import { and, eq, exists, gt, isNull, lte, inArray, notExists, or, sql } from "drizzle-orm"; import { checkpointDb, heartbeatRuns, issues, nativeRunFinalizations, nativeRunnerCheckpoints, type Db } from "@paperclipai/db"; @@ -349,10 +350,12 @@ export async function listExpiredNativeRunnerAuthorities(db: Db, now: Date, runI eq(heartbeatRuns.status, "running"), eq(nativeRunFinalizations.phase, "observed"), isNull(nativeRunFinalizations.resultId), or(isNull(nativeRunFinalizations.leaseExpiresAt), lte(nativeRunFinalizations.leaseExpiresAt, now)), - exists(db.select({ runId: nativeRunnerCheckpoints.runId }).from(nativeRunnerCheckpoints).where(and( + or(exists(db.select({ runId: nativeRunnerCheckpoints.runId }).from(nativeRunnerCheckpoints).where(and( eq(nativeRunnerCheckpoints.runId, nativeRunFinalizations.runId), eq(nativeRunnerCheckpoints.issueId, nativeRunFinalizations.issueId), - eq(nativeRunnerCheckpoints.companyId, nativeRunFinalizations.companyId)))))); + eq(nativeRunnerCheckpoints.companyId, nativeRunFinalizations.companyId)))), + and(sql`${heartbeatRuns.runnerProfileJson}->'nativePreparationFencingVersion' = ${JSON.stringify(NATIVE_PREPARATION_FENCING_VERSION)}::jsonb`, + isNull(heartbeatRuns.processPid), isNull(heartbeatRuns.processGroupId), isNull(heartbeatRuns.processStartedAt))))); return rows.map(row => row.runId); } diff --git a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts index 26b9a82da58..8b8ebc17c71 100644 --- a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts +++ b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts @@ -18,6 +18,7 @@ import { agents, authUsers, companies, + controllerInstances, companyMemberships, completionContracts, environmentLeases, @@ -80,6 +81,8 @@ import { PaperclipRunnerToolAuthority } from "./paperclip-runner-tool-authority. import { RUNNER_BRIDGE_READ_OPERATIONS, RUNNER_BRIDGE_SCHEMAS } from "./runner-bridge-tools.js"; import { HttpError } from "../../errors.js"; import { legacyControllerBootId } from "../legacy-controller-lease.js"; +import { onControllerStamp } from "../controller-process-identity.js"; +import { registerControllerInstanceDb, resetControllerInstanceLeaseForTests } from "../controller-instance-lease.js"; const mockCaptureRunFailure = vi.hoisted(() => vi.fn()); vi.mock("../../sentry.js", async () => { @@ -2277,6 +2280,42 @@ process.on("SIGTERM", () => child.kill("SIGTERM")); ]); }); + it("recovers a preparation with no runner PID once its foreign controller stops", async () => { + const fixture = await seedRun("REMOTE-PREPARATION-OWNER"); + const oldBoot = randomUUID(); + await fixture.db.update(heartbeatRuns).set({ + runnerProfileJson: { ...fixture.preparedProfile, nativeWorkspaceSync: remotePreparedWorkspace() }, + }).where(eq(heartbeatRuns.id, fixture.runId)); + await fixture.db.update(nativeRunFinalizations).set({ controllerBootId: oldBoot, controllerPid: 7, + controllerProcessStartedAt: new Date(Date.now() - 600_000), controllerHostHash: "previous-pid-space", + leaseOwner: "previous-preparation", leaseExpiresAt: new Date(0), attempt: 1, controllerGeneration: 1, + }).where(eq(nativeRunFinalizations.runId, fixture.runId)); + await fixture.db.insert(controllerInstances).values({ bootId: oldBoot, processSpace: "previous-pid-space", + pid: 7, leaseExpiresAt: new Date(Date.now() + 60_000) }); + registerControllerInstanceDb(fixture.db); + const stamped = vi.fn(), unsubscribe = onControllerStamp(stamped); + try { + const [waiting] = await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, + restartKind: "hard", runIds: [fixture.runId] }); + expect(waiting).toMatchObject({ kind: "awaiting_evidence", reason: "controller_lease_held", retryAt: expect.any(Date) }); + expect(stamped).not.toHaveBeenCalled(); + const [before] = await fixture.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, fixture.runId)); + await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, restartKind: "hard", runIds: [fixture.runId] }); + const [after] = await fixture.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, fixture.runId)); + expect(after!.updatedAt).toEqual(before!.updatedAt); + await fixture.db.update(controllerInstances).set({ stoppedAt: new Date() }).where(eq(controllerInstances.bootId, oldBoot)); + const dispositions = (await Promise.all([successor, { ...successor, bootId: randomUUID() }].map(controller => + claimNativeRestartRecoveries({ db: fixture.db, controller, restartKind: "hard", runIds: [fixture.runId] })))).flat(); + expect(dispositions.filter(row => row.kind === "bootstrap_incomplete")).toHaveLength(1); + expect(dispositions.filter(row => row.kind === "awaiting_evidence")).toHaveLength(1); + expect(stamped).toHaveBeenCalledOnce(); + const [run] = await fixture.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId)); + expect(run).toMatchObject({ status: "running", processPid: null, nativeSessionId: fixture.native.normalizedSessionId }); + const [owner] = await fixture.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, fixture.runId)); + expect(owner).toMatchObject({ attempt: 1, controllerGeneration: 2 }); + } finally { unsubscribe(); resetControllerInstanceLeaseForTests(); } + }); + it("recovers an unclaimed remote startup once without requiring a provider checkpoint", async () => { const fixture = await seedRun("REMOTE-UNCLAIMED"); await fixture.db.update(heartbeatRuns).set({ diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index a7b8873c3a0..ed4ffb7cae8 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -303,7 +303,7 @@ import { noteNativeRunnerEvidence } from "./native-runner-evidence-signal.js"; import { removeNativeHarnessBackup } from "./native-harness-backup-cleanup.js"; import { registerLiveRunnerGoalController } from "../runner-goal-control-broker.js"; import { applyRunnerGoalPrpEvent } from "../runner-goals.js"; -import { currentProcessSpaceHash } from "../controller-process-identity.js"; +import { currentControllerProcessStamp, currentProcessSpaceHash } from "../controller-process-identity.js"; import { proveOwnerGone } from "../owner-proof.js"; import { NativeRunnerOwnershipUnverifiedError, @@ -9996,6 +9996,7 @@ async function executePaperclipNativeSessionWithinScope( } const leaseNow = new Date(); confirmedLeaseDeadline = performance.now() + executionLeaseTtlMs; + currentControllerProcessStamp(); const claimed = await tx .update(nativeRunFinalizations) .set({ diff --git a/server/src/services/native-runtime/runner-api-client.ts b/server/src/services/native-runtime/runner-api-client.ts index 2c34e479389..ecce7755c8f 100644 --- a/server/src/services/native-runtime/runner-api-client.ts +++ b/server/src/services/native-runtime/runner-api-client.ts @@ -54,7 +54,7 @@ export function validateRunnerApiCall(value: unknown, context: RunnerApiContext) if (!parsed.success) throw badRequest("Invalid call_api arguments", { issues: parsed.error.issues }); const input = parsed.data; const operation = runnerApiOperation(input.operationId); - if (operation.transport !== "rest") throw unprocessable("This endpoint requires its existing protocol client; call_api supports REST only and no other API route substitutes for it. Its catalog description says who can use it."); + if (operation.transport !== "rest") throw unprocessable(`This endpoint requires its existing protocol client; call_api supports REST only and no other API route substitutes for it.${operation.description.trim() ? " Read its catalog description for details." : ""}`); if (input.responseText && operation.method !== "GET") throw badRequest("responseText requires a GET operation; read the saved artifact instead of repeating a mutation"); if (!operation.allowedModes.includes(context.workMode)) throw forbidden("call_api permits only reads in Ask and Plan modes; use the permitted dedicated tools"); if (input.pathParams?.companyId && input.pathParams.companyId !== context.companyId) throw forbidden("API call belongs to another company"); diff --git a/server/src/services/plugin-config-validator.ts b/server/src/services/plugin-config-validator.ts index 285868edf7d..404cc4e0041 100644 --- a/server/src/services/plugin-config-validator.ts +++ b/server/src/services/plugin-config-validator.ts @@ -10,13 +10,7 @@ import Ajv, { type ErrorObject } from "ajv"; import addFormats from "ajv-formats"; -import type { JsonSchema } from "@paperclipai/shared"; -import { - collectSecretRefPaths, - parseSecretRefBindingObject, - readConfigValueAtPath, - writeConfigValueAtPath, -} from "./json-schema-secret-refs.js"; +import { envBindingSecretRefSchema, type JsonSchema } from "@paperclipai/shared"; export interface ConfigValidationResult { valid: boolean; @@ -28,14 +22,43 @@ export interface ConfigValidationResult { * plugin secrets API store the `{ type: "secret_ref", secretId, version }` binding * object there. Validate the binding's secret id in its place so the schema * checks the field without rejecting the only shape the secrets handler accepts. + * The walk follows object properties, array items and schema combinators, and + * accepts a binding only when the secrets handler's own parser accepts it. */ -function secretRefsAsIds(configJson: Record, schema: JsonSchema) { - let candidate = configJson; - for (const path of collectSecretRefPaths(schema as Record)) { - const binding = parseSecretRefBindingObject(readConfigValueAtPath(candidate, path)); - if (binding) candidate = writeConfigValueAtPath(candidate, path, binding.secretId); +function secretRefsAsIds(value: unknown, schema: unknown): unknown { + if (!schema || typeof schema !== "object" || Array.isArray(schema)) return value; + const node = schema as Record; + + if (node.format === "secret-ref") { + const binding = envBindingSecretRefSchema.safeParse(value); + if (binding.success) return binding.data.secretId; } - return candidate; + + let result = value; + for (const keyword of ["allOf", "anyOf", "oneOf"] as const) { + const branches = node[keyword]; + if (!Array.isArray(branches)) continue; + for (const branch of branches) result = secretRefsAsIds(result, branch); + } + + if (Array.isArray(result)) { + const items = node.items; + if (items && typeof items === "object") { + const itemSchemaAt = (index: number) => (Array.isArray(items) ? items[index] : items); + result = result.map((item, index) => secretRefsAsIds(item, itemSchemaAt(index))); + } + return result; + } + + const properties = node.properties; + if (result && typeof result === "object" && properties && typeof properties === "object") { + const record = { ...(result as Record) }; + for (const [key, propertySchema] of Object.entries(properties as Record)) { + if (key in record) record[key] = secretRefsAsIds(record[key], propertySchema); + } + result = record; + } + return result; } /** diff --git a/ui/src/pages/PluginSettings.test.tsx b/ui/src/pages/PluginSettings.test.tsx index 402869be166..d129f093637 100644 --- a/ui/src/pages/PluginSettings.test.tsx +++ b/ui/src/pages/PluginSettings.test.tsx @@ -19,6 +19,8 @@ const mockPluginsApi = vi.hoisted(() => ({ })); const mockSetBreadcrumbs = vi.hoisted(() => vi.fn()); +const mockRoute = vi.hoisted(() => ({ pluginId: "plugin-1" })); +const mockSlots = vi.hoisted(() => ({ slots: [] as Array> })); vi.mock("@/api/plugins", () => ({ pluginsApi: mockPluginsApi, @@ -40,12 +42,12 @@ vi.mock("@/context/CompanyContext", () => ({ vi.mock("@/lib/router", () => ({ Link: ({ to, children }: { to: string; children: React.ReactNode }) => {children}, Navigate: () => null, - useParams: () => ({ companyPrefix: "PAP", pluginId: "plugin-1" }), + useParams: () => ({ companyPrefix: "PAP", pluginId: mockRoute.pluginId }), })); vi.mock("@/plugins/slots", () => ({ - PluginSlotMount: () => null, - usePluginSlots: () => ({ slots: [] }), + PluginSlotMount: ({ slot }: { slot: { id: string } }) =>
{slot.id}
, + usePluginSlots: () => ({ slots: mockSlots.slots }), })); vi.mock("@/components/PageTabBar", () => ({ @@ -168,6 +170,31 @@ describe("PluginSettings", () => { container.remove(); document.body.innerHTML = ""; vi.clearAllMocks(); + mockRoute.pluginId = "plugin-1"; + mockSlots.slots = []; + }); + + it("shows a plugin's custom settings page when the route names the plugin by key", async () => { + mockRoute.pluginId = "acme.website-preview"; + mockSlots.slots = [ + { id: "settings", type: "settingsPage", pluginId: "plugin-1", pluginKey: "acme.website-preview" }, + { id: "other-settings", type: "settingsPage", pluginId: "plugin-2", pluginKey: "acme.other" }, + ]; + mockPluginsApi.get.mockResolvedValue(basePlugin({ + pluginKey: "acme.website-preview", + status: "ready", + manifestJson: { displayName: "Website Preview", version: "0.1.0", capabilities: [] }, + })); + + const root = await renderSettings(container); + + const mounted = [...container.querySelectorAll('[data-testid="settings-slot"]')].map((node) => node.textContent); + expect(mounted).toEqual(["settings"]); + expect(container.textContent).not.toContain("This plugin does not require any settings."); + + await act(async () => { + root.unmount(); + }); }); it("routes environment-provider plugins to instance environments when they have no instance config", async () => { diff --git a/ui/src/pages/PluginSettings.tsx b/ui/src/pages/PluginSettings.tsx index f48e539ab1e..86202461bca 100644 --- a/ui/src/pages/PluginSettings.tsx +++ b/ui/src/pages/PluginSettings.tsx @@ -118,8 +118,9 @@ export function PluginSettings() { enabled: !!selectedCompanyId, }); - // Filter slots to only show settings pages for this specific plugin - const pluginSlots = slots.filter((slot) => slot.pluginId === pluginId); + // Filter slots to only show settings pages for this specific plugin. The route + // segment can be the plugin's id or its key, so match either. + const pluginSlots = slots.filter((slot) => slot.pluginId === pluginId || slot.pluginKey === pluginId); // If the plugin has a custom settingsPage slot, prefer that over auto-generated form const hasCustomSettingsPage = pluginSlots.length > 0;