feat(runner): resolve ACPX provider requests (#12421)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The Rust runner now owns a scoped ACPX turn and validates provider
requests before exposing them
> - A live turn can pause for semantic tool results or structured input
> - Local state and the sidecar can diverge if the runner clears a
request before the sidecar accepts its resolution
> - A mismatched or ambiguous remote acknowledgement must close the
session, while a local validation error must preserve the request for a
safe retry
> - This pull request adds those two-phase resolution paths and rejects
Codex permission events that bypass the pinned policy without selecting
ACPX in runnerd
> - The benefit is an atomic request boundary that can be wired into
durable execution in a later slice

## Linked Issues or Issue Description

Refs #12420

Refs #12419

## What Changed

- Resolve authorized semantic tool calls only for the active turn and
exact pending operation.
- Validate semantic results against the authorized response schema
before transport.
- Send a bounded generic provider error when a semantic operation fails
without exposing internal error text or payloads.
- Resolve structured input only after validating the exact persisted
question set.
- Support explicit submit, decline, and cancel input outcomes.
- Reject any Codex permission event that bypasses the pinned sidecar
policy.
- Build candidate provider and authorization state before each request.
- Commit candidate state only after the sidecar returns an affirmative
resolution acknowledgement.
- Preserve pending work after local validation errors so the caller can
retry safely.
- Terminate the session after transport failure or an invalid remote
acknowledgement because the remote effect is ambiguous.
- Bind every resolution to the exact active turn and request or call
identity.
- Extend the fake sidecar and add integration coverage for successful
commits, safe local retries, pinned-policy enforcement, redaction, and
fail-closed acknowledgement mismatch.
- Document the package-local resolution boundary.
- Do not change dependencies, lockfiles, workflows, runnerd selection,
server behavior, UI, or migrations.

## Verification

- Replay base: `3aa2065d084d6a29492aaa15e822b5d17c3a4266` (`master`
after #12420 merged).
- Exact replay head: `9507024f70c6f434c2c322385d3a9e240250b03c`.
- Stable patch ID: `a4f27d2fae606596f70b5b1c2b29dd7f250541d8`, identical
to the prepared two-commit delta.
- The exact delta is 5 files, 463 additions, and 7 deletions, all in
`packages/paperclip-runner`; it contains no lockfile, workflow, server,
UI, or migration change.
- GitHub Actions run `33368135190`, attempt 2: **PASSED** on the exact
replay head (23/23 jobs passed; a failed-job-only retry cleared one
unrelated server test environment failure where `npm` was unavailable).
- Greptile: **5/5** on the exact replay head with zero unresolved review
threads; Superagent, Socket, Snyk, and contributor-trust checks also
passed.
- No local test result is claimed. GitHub Actions is the authoritative
verification environment for this replayed revision.

## Risks

- A transport failure can happen after the sidecar applied a resolution.
The session closes instead of retrying an ambiguous effect.
- Local validation happens before transport and preserves pending state,
so a corrected answer or result can be retried.
- The sidecar transport already correlates each command response to its
request identifier. This slice also requires `resolved: true` before
local commit.
- The initial Codex sidecar owns its pinned permission policy and does
not delegate permission resolution. Any permission event therefore
terminates the session fail closed.
- No production path invokes these methods in this pull request. Durable
ACPX execution wiring remains a later slice.

> 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 (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [ ] 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:
Dotta 2026-08-31 02:39:08 -05:00 committed by GitHub
parent 3aa2065d08
commit 91d861ff69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 463 additions and 7 deletions

View File

@ -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:

View File

@ -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>,

View File

@ -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()
}

View File

@ -31,6 +31,10 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
.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<dyn std::error::Error>> {
| "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<dyn std::error::Error>> {
)?;
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<dyn std::error::Error>> {
)?;
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<dyn std::error::Error>> {
}
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}),
};

View File

@ -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());
}