diff --git a/packages/paperclip-runner/README.md b/packages/paperclip-runner/README.md index 7c91f9a034..fedf71d0eb 100644 --- a/packages/paperclip-runner/README.md +++ b/packages/paperclip-runner/README.md @@ -106,6 +106,11 @@ event authority. The Rust question-response validator checks the versioned response envelope against the exact persisted question IDs, answer modes, options, required answers, custom-answer policy, and text constraints before provider delivery. +Tool results and structured question responses then use two-phase resolution: +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. Run the complete contract gate with: 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 af272e60ea..34330e5149 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 @@ -12,8 +12,9 @@ use crate::generated_acpx_sidecar_contract::{ use crate::local_runner::LocalRunnerError; use crate::provider_bridge::{ authorized_tool_catalog_digest, AuthorizedTool, AuthorizedToolSet, ProviderToolBridge, - TOOL_SET_SCHEMA, + ToolResult, TOOL_SET_SCHEMA, }; +use crate::question_response::validate_question_response; const MAX_ID_CHARS: usize = 240; const MAX_MODEL_CHARS: usize = 240; @@ -508,6 +509,11 @@ impl AcpxProviderSession { .map(AcpxProviderStateEvent::ToolResult), ); } + AcpxProviderStateEvent::PermissionRequest { .. } => { + return Err(self.fail_closed(LocalRunnerError::invalid( + "ACPX Codex permission request violated the pinned runner policy", + ))); + } _ => {} } if expose_event { @@ -520,6 +526,72 @@ impl AcpxProviderSession { Ok(Some(reconciled_events)) } + pub fn deliver_tool_result(&mut self, result: &ToolResult) -> Result<(), LocalRunnerError> { + let turn_id = self.ensure_active_turn()?.to_owned(); + let mut next_state = self.state.clone(); + next_state.complete_tool(&result.call_id, &result.operation_id)?; + let mut next_bridge = self.tool_bridge.clone(); + next_bridge.apply_result(result.clone()).map_err(|error| { + LocalRunnerError::invalid(format!("ACPX tool result is invalid: {error}")) + })?; + let resolution = if result.is_error { + // The durable result remains authoritative for correlation and + // retry bookkeeping, but provider-facing failures expose only a + // fixed diagnostic. Internal dispatcher payloads must not cross + // the sidecar boundary on the separate success-result channel. + json!({ + "callId":result.call_id, + "turnId":turn_id, + "error":{"message":"Paperclip semantic operation failed"}, + }) + } else { + json!({ + "callId":result.call_id, + "turnId":turn_id, + "result":result.result, + "error":Value::Null, + }) + }; + let response = match self + .transport + .request(GeneratedAcpxSidecarCommand::ToolResolve, resolution) + { + Ok(response) => response, + Err(error) => return Err(self.fail_closed(error)), + }; + self.verify_resolution(&response, "tool")?; + self.state = next_state; + self.tool_bridge = next_bridge; + Ok(()) + } + + pub fn resolve_input( + &mut self, + request_id: &str, + turn_id: &str, + resolution: &Value, + ) -> Result<(), LocalRunnerError> { + self.ensure_bound_turn(turn_id)?; + validate_text(request_id, 240, "ACPX input request id")?; + let question_set = self + .state + .pending_question_set(request_id) + .ok_or_else(|| LocalRunnerError::invalid("ACPX input request is stale or unknown"))?; + validate_input_resolution(question_set, resolution)?; + let mut next_state = self.state.clone(); + next_state.complete_input(request_id)?; + let response = match self.transport.request( + GeneratedAcpxSidecarCommand::InputResolve, + json!({"requestId":request_id,"turnId":turn_id,"resolution":resolution}), + ) { + Ok(response) => response, + Err(error) => return Err(self.fail_closed(error)), + }; + self.verify_resolution(&response, "input")?; + self.state = next_state; + Ok(()) + } + pub fn shutdown(&mut self, reason: &str) -> Result<(), LocalRunnerError> { if self.closed { return self.terminate_transport(); @@ -603,6 +675,32 @@ impl AcpxProviderSession { with_cleanup_error(error, cleanup) } + fn ensure_active_turn(&self) -> Result<&str, LocalRunnerError> { + self.ensure_open()?; + self.state + .active_turn_id() + .ok_or_else(|| LocalRunnerError::invalid("ACPX provider session has no active turn")) + } + + fn ensure_bound_turn(&self, turn_id: &str) -> Result<(), LocalRunnerError> { + validate_text(turn_id, 160, "ACPX turn id")?; + if self.ensure_active_turn()? != turn_id { + return Err(LocalRunnerError::invalid( + "ACPX resolution named a stale or inactive turn", + )); + } + Ok(()) + } + + fn verify_resolution(&mut self, response: &Value, kind: &str) -> Result<(), LocalRunnerError> { + if response.get("resolved").and_then(Value::as_bool) != Some(true) { + return Err(self.fail_closed(LocalRunnerError::invalid(format!( + "ACPX sidecar did not confirm {kind} resolution" + )))); + } + Ok(()) + } + fn fail_closed(&mut self, error: LocalRunnerError) -> LocalRunnerError { self.closed = true; with_cleanup_error(error, self.terminate_transport()) @@ -901,6 +999,42 @@ fn validate_turn_message(value: &str) -> Result<(), LocalRunnerError> { Ok(()) } +fn validate_input_resolution( + question_set: &Value, + resolution: &Value, +) -> Result<(), LocalRunnerError> { + let object = resolution + .as_object() + .ok_or_else(|| LocalRunnerError::invalid("ACPX input resolution must be an object"))?; + if object + .keys() + .any(|key| !matches!(key.as_str(), "action" | "response")) + { + return Err(LocalRunnerError::invalid( + "ACPX input resolution contains an unknown field", + )); + } + let action = resolution + .get("action") + .and_then(Value::as_str) + .ok_or_else(|| LocalRunnerError::invalid("ACPX input resolution requires an action"))?; + match action { + "submit" => validate_question_response( + question_set, + resolution.get("response").ok_or_else(|| { + LocalRunnerError::invalid("ACPX submitted input resolution requires a response") + })?, + ), + "decline" | "cancel" if !object.contains_key("response") => Ok(()), + "decline" | "cancel" => Err(LocalRunnerError::invalid( + "ACPX declined input resolution cannot contain a response", + )), + _ => Err(LocalRunnerError::invalid( + "ACPX input resolution action is unsupported", + )), + } +} + fn with_cleanup_error( error: LocalRunnerError, cleanup: Result<(), LocalRunnerError>, diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_state.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_state.rs index fd223888e0..78b547f049 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_state.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_state.rs @@ -76,6 +76,7 @@ pub enum AcpxProviderStateEvent { #[derive(Clone, Debug, PartialEq)] struct PendingInput { value_bytes: usize, + question_set: Value, } /// Reduces validated sidecar events into bounded provider state. @@ -216,7 +217,13 @@ impl AcpxProviderState { self.admit_runtime_request(&request_id, value_bytes)?; if self .pending_inputs - .insert(request_id.clone(), PendingInput { value_bytes }) + .insert( + request_id.clone(), + PendingInput { + value_bytes, + question_set: question_set.clone(), + }, + ) .is_some() { return Err(LocalRunnerError::invalid( @@ -321,10 +328,9 @@ impl AcpxProviderState { "ACPX tool result operation mismatch", )); } - let pending = self - .pending_tools - .remove(call_id) - .expect("validated ACPX pending tool remains present"); + let pending = self.pending_tools.remove(call_id).ok_or_else(|| { + LocalRunnerError::invalid("ACPX pending tool disappeared during completion") + })?; self.pending_tool_input_bytes = self .pending_tool_input_bytes .saturating_sub(pending.input_bytes); @@ -352,6 +358,12 @@ impl AcpxProviderState { Ok(()) } + pub fn pending_question_set(&self, request_id: &str) -> Option<&Value> { + self.pending_inputs + .get(request_id) + .map(|pending| &pending.question_set) + } + pub fn semantic_result(&self) -> Option<&AcpxSemanticResult> { self.semantic_result.as_ref() } 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 6b03af391d..0d4feec4a4 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 @@ -31,6 +31,10 @@ fn run() -> Result<(), Box> { .get("command") .and_then(Value::as_str) .ok_or("request command is missing")?; + if command == "permission.resolve" { + write_json(&mut stdout, &bootstrap_success(id, command, &request, mode))?; + continue; + } match mode { "silent" => continue, "wrong-id" => { @@ -102,7 +106,11 @@ fn run() -> Result<(), Box> { | "turns-invalid-reserved-block-terminal" | "turns-uncorrelated-reserved-result-terminal" | "turns-mismatched-reserved-result-terminal" - | "turns-unauthorized-tool" => { + | "turns-unauthorized-tool" + | "turns-permission" + | "resolutions" + | "resolutions-error-redaction" + | "resolutions-wrong-ack" => { write_json(&mut stdout, &bootstrap_success(id, command, &request, mode))?; let params = request.get("params").unwrap_or(&Value::Null); let turn_id = params @@ -253,6 +261,21 @@ fn run() -> Result<(), Box> { )?; next_sequence += 1; } + if command == "turn.start" && mode == "turns-permission" { + write_turn_event( + &mut stdout, + next_sequence, + "runtime.permission_requested", + "run-1", + turn_id, + json!({ + "requestId":"permission-1", + "kind":"execute", + "title":"Run a command?", + }), + )?; + next_sequence += 1; + } if command == "turn.start" && matches!( mode, @@ -424,6 +447,49 @@ fn run() -> Result<(), Box> { )?; next_sequence += 1; } + if command == "turn.start" + && matches!( + mode, + "resolutions" | "resolutions-error-redaction" | "resolutions-wrong-ack" + ) + { + for (event_type, payload) in [ + ( + "runtime.tool_called", + json!({ + "callId":"call-1", + "operationId":"issues.read", + "input":{"id":"issue-1"}, + }), + ), + ( + "runtime.input_requested", + json!({ + "requestId":"input-1", + "questionSet":{ + "schema":"paperclip.question_set.v1", + "questions":[{ + "id":"target", + "prompt":"Which target?", + "required":true, + "answerMode":"single_select", + "options":[{"id":"first","label":"First"}], + }], + }, + }), + ), + ] { + write_turn_event( + &mut stdout, + next_sequence, + event_type, + "run-1", + turn_id, + payload, + )?; + next_sequence += 1; + } + } if command == "turn.cancel" && mode == "turns" { write_turn_event( &mut stdout, @@ -448,6 +514,18 @@ fn run() -> Result<(), Box> { } fn bootstrap_success(id: u64, command: &str, request: &Value, mode: &str) -> Value { + if command == "permission.resolve" { + return json!({ + "protocolVersion": GENERATED_ACPX_SIDECAR_PROTOCOL_VERSION, + "id": id, + "ok": false, + "error": { + "code": "permission_resolution_unsupported", + "message": "Codex permissions are fixed by runner policy and cannot be resolved through ACPX.", + "retryable": false, + }, + }); + } let params = request.get("params").cloned().unwrap_or_else(|| json!({})); let result = match command { "initialize" => json!({ @@ -507,6 +585,19 @@ fn bootstrap_success(id: u64, command: &str, request: &Value, mode: &str) -> Val "permissionMode": "approve-reads", }, }), + "tool.resolve" => json!({ + "resolved":if mode == "resolutions-error-redaction" { + params.get("callId").and_then(Value::as_str) == Some("call-1") + && params.get("turnId").and_then(Value::as_str) == Some("turn-1") + && params.get("result").is_none() + && params.pointer("/error/message").and_then(Value::as_str) + == Some("Paperclip semantic operation failed") + && !params.to_string().contains("violet-internal-diagnostic-4821") + } else { + mode != "resolutions-wrong-ack" + } + }), + "input.resolve" => json!({"resolved":true}), "session.close" => json!({"closed":true}), _ => json!({"command":command,"params":params}), }; diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_provider_resolutions.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_provider_resolutions.rs new file mode 100644 index 0000000000..e9a8d4b2ae --- /dev/null +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_provider_resolutions.rs @@ -0,0 +1,214 @@ +use std::io::Write; +use std::path::PathBuf; +use std::process::{Command, Stdio}; +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, AuthorizedTool, AuthorizedToolSet, ToolResult, +}; +use serde_json::json; + +fn tool_set() -> AuthorizedToolSet { + let operations = vec![AuthorizedTool { + operation_id: "issues.read".to_owned(), + version: 1, + description: "Read an issue.".to_owned(), + input_schema: json!({"type":"object"}), + response_schema: json!({"type":"object"}), + }]; + AuthorizedToolSet { + schema: "paperclip.runner.authorized-tools.v1".to_owned(), + schema_version: 1, + catalog_digest: authorized_tool_catalog_digest(&operations).unwrap(), + operations, + } +} + +fn config(mode: &str) -> AcpxProviderSessionConfig { + 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: tool_set(), + expected_identity: None, + } +} + +fn started(mode: &str) -> AcpxProviderSession { + let mut session = AcpxProviderSession::start(&config(mode)).unwrap(); + session + .start_turn("turn-1", "Please help", &std::env::temp_dir()) + .unwrap(); + for _ in 0..2 { + session.poll_event(Duration::from_secs(1)).unwrap().unwrap(); + } + session +} + +fn tool_result(operation_id: &str) -> ToolResult { + ToolResult { + call_id: "call-1".to_owned(), + operation_id: operation_id.to_owned(), + result: json!({"id":"issue-1"}), + is_error: false, + } +} + +fn input_resolution(option_id: &str) -> serde_json::Value { + json!({ + "action":"submit", + "response":{ + "schema":"paperclip.question_response.v1", + "answers":{"target":{"selectedOptionIds":[option_id]}} + } + }) +} + +#[test] +fn commits_each_resolution_only_after_sidecar_acknowledgement() { + let mut session = started("resolutions"); + session + .deliver_tool_result(&tool_result("issues.read")) + .unwrap(); + session + .resolve_input("input-1", "turn-1", &input_resolution("first")) + .unwrap(); + assert!(session.state().pending_tool("call-1").is_none()); + assert!(session.state().pending_question_set("input-1").is_none()); + assert!(session + .deliver_tool_result(&tool_result("issues.read")) + .is_err()); + session.shutdown("test complete").unwrap(); +} + +#[test] +fn local_validation_preserves_pending_requests_for_a_correct_retry() { + let mut session = started("resolutions"); + assert!(session + .deliver_tool_result(&tool_result("issues.write")) + .unwrap_err() + .to_string() + .contains("operation mismatch")); + assert!(session.state().pending_tool("call-1").is_some()); + let mut invalid_result = tool_result("issues.read"); + invalid_result.result = json!("not an object"); + assert!(session + .deliver_tool_result(&invalid_result) + .unwrap_err() + .to_string() + .contains("JSON Schema validation")); + assert!(session.state().pending_tool("call-1").is_some()); + session + .deliver_tool_result(&tool_result("issues.read")) + .unwrap(); + + assert!(session + .resolve_input("input-1", "turn-1", &input_resolution("unknown")) + .unwrap_err() + .to_string() + .contains("unknown option")); + assert!(session.state().pending_question_set("input-1").is_some()); + session + .resolve_input("input-1", "turn-1", &input_resolution("first")) + .unwrap(); + + session.shutdown("test complete").unwrap(); +} + +#[test] +fn redacts_failed_tool_payload_without_losing_correlation_or_retry() { + let mut session = started("resolutions-error-redaction"); + let mut failed = ToolResult { + call_id: "call-1".to_owned(), + operation_id: "issues.write".to_owned(), + result: json!({ + "diagnostic":"violet-internal-diagnostic-4821", + "request":{"private":"value"}, + }), + is_error: true, + }; + + assert!(session.deliver_tool_result(&failed).is_err()); + assert!(session.state().pending_tool("call-1").is_some()); + + failed.operation_id = "issues.read".to_owned(); + session.deliver_tool_result(&failed).unwrap(); + assert!(session.state().pending_tool("call-1").is_none()); + assert!(session.deliver_tool_result(&failed).is_err()); + session.shutdown("test complete").unwrap(); +} + +#[test] +fn rejects_permission_requests_that_bypass_the_pinned_codex_policy() { + let mut session = AcpxProviderSession::start(&config("turns-permission")).unwrap(); + session + .start_turn("turn-1", "Please help", &std::env::temp_dir()) + .unwrap(); + + let error = session + .poll_event(Duration::from_secs(1)) + .unwrap_err() + .to_string(); + assert!(error.contains("pinned runner policy"), "{error}"); + assert!(session.shutdown("already closed").is_ok()); +} + +#[test] +fn fake_sidecar_rejects_the_unsupported_permission_resolution_command() { + let mut child = Command::new(env!("CARGO_BIN_EXE_fake-acpx-sidecar")) + .args(["--mode", "happy"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + { + let mut stdin = child.stdin.take().unwrap(); + writeln!( + stdin, + "{}", + json!({ + "protocolVersion":"paperclip.runner.acpx-sidecar.v1", + "id":1, + "command":"permission.resolve", + "params":{"requestId":"permission-1","resolution":"approved"} + }) + ) + .unwrap(); + } + let output = child.wait_with_output().unwrap(); + assert!(output.status.success()); + let response: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(response["ok"], false); + assert_eq!( + response["error"]["code"], + "permission_resolution_unsupported" + ); +} + +#[test] +fn fails_closed_when_a_resolution_is_not_acknowledged() { + let mut session = started("resolutions-wrong-ack"); + let error = session + .deliver_tool_result(&tool_result("issues.read")) + .unwrap_err() + .to_string(); + assert!(error.contains("did not confirm tool resolution"), "{error}"); + assert!(session.shutdown("already closed").is_ok()); +}