feat(runner): suspend safe ACPX sessions (#12422)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Rust runner now owns exact ACPX request resolution and fail-closed session state > - A recoverable provider session needs an explicit suspension boundary before runnerd can safely hand work across process lifetimes > - Suspension is unsafe while a turn or provider request is active because remote effects may still be in flight > - A successful acknowledgement must preserve the exact immutable session identity > - This pull request adds only that guarded package-local lifecycle operation without selecting ACPX in runnerd > - The benefit is a small recovery primitive whose safety rules are independently reviewable ## Linked Issues or Issue Description Refs #12421 ## What Changed - Add a provider-state query for active pending tool, input, or permission requests. - Permit session suspension only when no turn or provider request is active. - Send a bounded `session.suspend` command with an operator-safe reason. - Require an affirmative suspension acknowledgement and the exact existing session identity. - Treat transport failure, rejection, omitted or malformed identity, and identity drift as fail-closed terminal errors. - Mark a successfully suspended session closed and terminate the local sidecar process while retaining cleanup ownership if termination must be retried. - Preserve a valid session after purely local unsafe-state rejection so it can settle before retry. - Extend the fake sidecar with deterministic suspension success, acknowledgement mismatch, identity mismatch, and missing-identity modes. - Add integration coverage for safe suspension, active-turn rejection, fail-closed remote mismatches, and retained cleanup. - Document the package-local suspension boundary. - Do not change dependencies, lockfiles, workflows, runnerd selection, server behavior, UI, or migrations. ## Verification - Replay base: `91d861ff69d415a3b105ae2eaad9cc56c66a9231` (`master` after #12421 merged). - Exact replay head: `085667e10c51c6c0360732f63c8fef83e806dd88`. - Stable patch ID: `b18d6b7efd1de569d3068b0a782f8aac2fbc9322`; this is the prepared suspension delta plus the focused fake-sidecar fix that consolidates mismatch modes into the existing command arm. - The exact delta is 5 files, 136 additions, and 5 deletions, all in `packages/paperclip-runner`; it contains no lockfile, workflow, server, UI, or migration change. - Exact-head GitHub Actions run `33369571343` (attempt 1): **PASSED** with 23 jobs passed and zero failures. - Greptile reviewed exact head `085667e10c51c6c0360732f63c8fef83e806dd88`: **5/5**, with zero unresolved review threads. - Superagent, contributor trust, Socket, and Snyk security checks: **PASSED**. - No local test result is claimed. GitHub Actions is the authoritative verification environment for this replayed revision. ## Risks - The sidecar may apply suspension before a transport failure is observed. The local session closes rather than retrying an ambiguous effect. - Local active-work rejection happens before transport and leaves the valid session open so the caller can settle it safely. - Identity equality is checked across provider, driver, session, thread, run, and company fields before accepting suspension. - No production path invokes suspension in this pull request. Runnerd execution and durable recovery wiring remain later slices. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex with GPT-5.6, agentic reasoning, tool use, and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either linked existing issues with `Refs #` or described the issue in-PR - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
parent
91d861ff69
commit
80639f4f69
|
|
@ -111,6 +111,9 @@ validate retained identity and schema, require the exact sidecar
|
|||
acknowledgement, and only then clear pending local state. Codex permission
|
||||
requests violate its pinned sidecar policy and terminate the session fail
|
||||
closed.
|
||||
Safe suspension is available only with no active turn or pending request. The
|
||||
sidecar must return the exact persistent session identity before runnerd
|
||||
terminates the local process.
|
||||
|
||||
Run the complete contract gate with:
|
||||
|
||||
|
|
|
|||
|
|
@ -592,6 +592,50 @@ impl AcpxProviderSession {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
pub fn suspend(
|
||||
&mut self,
|
||||
reason: &str,
|
||||
) -> Result<AcpxProviderSessionIdentity, LocalRunnerError> {
|
||||
self.ensure_open()?;
|
||||
if self.state.active_turn_id().is_some() || self.state.has_pending_requests() {
|
||||
return Err(LocalRunnerError::invalid(
|
||||
"ACPX provider session is not at a safe suspension point",
|
||||
));
|
||||
}
|
||||
let response = match self.transport.request(
|
||||
GeneratedAcpxSidecarCommand::SessionSuspend,
|
||||
json!({"reason":bounded_reason(reason)}),
|
||||
) {
|
||||
Ok(response) => response,
|
||||
Err(error) => return Err(self.fail_closed(error)),
|
||||
};
|
||||
let identity = response
|
||||
.get("identity")
|
||||
.cloned()
|
||||
.ok_or_else(|| LocalRunnerError::invalid("ACPX suspension omitted its identity"))
|
||||
.and_then(|value| {
|
||||
serde_json::from_value::<AcpxProviderSessionIdentity>(value).map_err(|error| {
|
||||
LocalRunnerError::invalid(format!(
|
||||
"ACPX suspension identity is invalid: {error}"
|
||||
))
|
||||
})
|
||||
});
|
||||
let identity = match identity {
|
||||
Ok(identity) => identity,
|
||||
Err(error) => return Err(self.fail_closed(error)),
|
||||
};
|
||||
if response.get("suspended").and_then(Value::as_bool) != Some(true)
|
||||
|| identity != self.identity
|
||||
{
|
||||
return Err(self.fail_closed(LocalRunnerError::invalid(
|
||||
"ACPX sidecar did not confirm the exact suspended session",
|
||||
)));
|
||||
}
|
||||
self.closed = true;
|
||||
self.terminate_transport()?;
|
||||
Ok(identity)
|
||||
}
|
||||
|
||||
pub fn shutdown(&mut self, reason: &str) -> Result<(), LocalRunnerError> {
|
||||
if self.closed {
|
||||
return self.terminate_transport();
|
||||
|
|
|
|||
|
|
@ -150,6 +150,12 @@ impl AcpxProviderState {
|
|||
.rotate_settled_turn_identities_after_provider_restart()
|
||||
}
|
||||
|
||||
pub fn has_pending_requests(&self) -> bool {
|
||||
!self.pending_tools.is_empty()
|
||||
|| !self.pending_permissions.is_empty()
|
||||
|| !self.pending_inputs.is_empty()
|
||||
}
|
||||
|
||||
pub fn begin_turn(&mut self, turn_id: impl Into<String>) -> Result<(), LocalRunnerError> {
|
||||
if self.scope.active_turn_id().is_some()
|
||||
|| !self.pending_tools.is_empty()
|
||||
|
|
|
|||
|
|
@ -110,7 +110,11 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
|
|||
| "turns-permission"
|
||||
| "resolutions"
|
||||
| "resolutions-error-redaction"
|
||||
| "resolutions-wrong-ack" => {
|
||||
| "resolutions-wrong-ack"
|
||||
| "suspend"
|
||||
| "suspend-wrong-ack"
|
||||
| "suspend-wrong-identity"
|
||||
| "suspend-missing-identity" => {
|
||||
write_json(&mut stdout, &bootstrap_success(id, command, &request, mode))?;
|
||||
let params = request.get("params").unwrap_or(&Value::Null);
|
||||
let turn_id = params
|
||||
|
|
@ -571,10 +575,10 @@ fn bootstrap_success(id: u64, command: &str, request: &Value, mode: &str) -> Val
|
|||
}),
|
||||
"turn.cancel" => json!({"cancelled":mode != "turns-wrong-cancel"}),
|
||||
"session.suspend" => json!({
|
||||
"suspended":true,
|
||||
"identity": {
|
||||
"suspended":mode != "suspend-wrong-ack",
|
||||
"identity": if mode == "suspend-missing-identity" { Value::Null } else { json!({
|
||||
"kind": "acpx",
|
||||
"normalizedSessionId": "session-1",
|
||||
"normalizedSessionId": if mode == "suspend-wrong-identity" { "another-session" } else { "session-1" },
|
||||
"acpxRecordId": "record-1",
|
||||
"backendSessionId": "backend-1",
|
||||
"agentSessionId": "agent-1",
|
||||
|
|
@ -583,7 +587,7 @@ fn bootstrap_success(id: u64, command: &str, request: &Value, mode: &str) -> Val
|
|||
"requestedModel": "gpt-5.6-sol",
|
||||
"effectiveModel": "gpt-5.6-sol",
|
||||
"permissionMode": "approve-reads",
|
||||
},
|
||||
})},
|
||||
}),
|
||||
"tool.resolve" => json!({
|
||||
"resolved":if mode == "resolutions-error-redaction" {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,74 @@
|
|||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use paperclip_runner_core::acpx_provider_session::{
|
||||
AcpxPermissionMode, AcpxProviderSession, AcpxProviderSessionConfig,
|
||||
};
|
||||
use paperclip_runner_core::acpx_sidecar_transport::AcpxSidecarTransportConfig;
|
||||
use paperclip_runner_core::provider_bridge::{authorized_tool_catalog_digest, AuthorizedToolSet};
|
||||
|
||||
fn config(mode: &str) -> AcpxProviderSessionConfig {
|
||||
let operations = Vec::new();
|
||||
AcpxProviderSessionConfig {
|
||||
transport: AcpxSidecarTransportConfig {
|
||||
command: PathBuf::from(env!("CARGO_BIN_EXE_fake-acpx-sidecar")),
|
||||
args: vec!["--mode".to_owned(), mode.to_owned()],
|
||||
request_timeout: Duration::from_secs(1),
|
||||
shutdown_grace: Duration::from_millis(100),
|
||||
},
|
||||
agent: "codex".to_owned(),
|
||||
model: "gpt-5.6-sol".to_owned(),
|
||||
run_id: "run-1".to_owned(),
|
||||
catalog_revision: 1,
|
||||
runtime_directory: std::env::temp_dir(),
|
||||
normalized_session_id: "session-1".to_owned(),
|
||||
working_directory: std::env::temp_dir(),
|
||||
permission_mode: AcpxPermissionMode::ApproveReads,
|
||||
permission_mode_pinned: true,
|
||||
system_instructions: "Complete the supplied task.".to_owned(),
|
||||
tool_set: AuthorizedToolSet {
|
||||
schema: "paperclip.runner.authorized-tools.v1".to_owned(),
|
||||
schema_version: 1,
|
||||
catalog_digest: authorized_tool_catalog_digest(&operations).unwrap(),
|
||||
operations,
|
||||
},
|
||||
expected_identity: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn suspends_only_after_the_exact_persistent_identity_is_confirmed() {
|
||||
let mut session = AcpxProviderSession::start(&config("suspend")).unwrap();
|
||||
let expected = session.identity().clone();
|
||||
|
||||
assert_eq!(session.suspend("worker restart").unwrap(), expected);
|
||||
assert!(session.shutdown("already suspended").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_suspension_during_an_active_turn_without_closing_the_session() {
|
||||
let mut session = AcpxProviderSession::start(&config("suspend")).unwrap();
|
||||
session
|
||||
.start_turn("turn-1", "Please help", &std::env::temp_dir())
|
||||
.unwrap();
|
||||
|
||||
let error = session.suspend("too early").unwrap_err().to_string();
|
||||
assert!(error.contains("safe suspension point"), "{error}");
|
||||
assert_eq!(session.state().active_turn_id(), Some("turn-1"));
|
||||
session.shutdown("test complete").unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fails_closed_when_the_suspension_acknowledgement_does_not_match() {
|
||||
for mode in ["suspend-wrong-ack", "suspend-wrong-identity"] {
|
||||
let mut session = AcpxProviderSession::start(&config(mode)).unwrap();
|
||||
let error = session.suspend("worker restart").unwrap_err().to_string();
|
||||
assert!(error.contains("exact suspended session"), "{mode}: {error}");
|
||||
assert!(session.shutdown("already closed").is_ok());
|
||||
}
|
||||
|
||||
let mut session = AcpxProviderSession::start(&config("suspend-missing-identity")).unwrap();
|
||||
let error = session.suspend("worker restart").unwrap_err().to_string();
|
||||
assert!(error.contains("identity is invalid"), "{error}");
|
||||
assert!(session.shutdown("already closed").is_ok());
|
||||
}
|
||||
Loading…
Reference in New Issue