diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 61ab5136574..6ccaec0bfe4 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -565,6 +565,13 @@ arguments can be submitted again. `retryable: false` prevents automatic replay of the invalid call. Unauthorized tools still receive a generic rejection, and corrected calls retain the same permission and approval checks. +For native `request_human_input`, `version` and `supersedeOnUserComment` belong +inside `payload`, never at the tool's root. Repair guidance names those paths +and keeps approval requests as `confirmation` or `checkbox`. A question form +collects answers; it cannot replace proposal acceptance. Invalid calls remain +rejected before dispatch, and a corrected call still waits for the normal saved +human decision before it can authorize onboarding child work. + A native provider may omit `continuation` (or send `null`) when it reports `yielded` after saving a monitor, requesting repository preparation, or publishing an authorized current-head GitHub review. The controller reads current authorized task diff --git a/doc/run-log-events.md b/doc/run-log-events.md index 3e2a709551e..1c6d900e926 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -184,6 +184,15 @@ Paperclip Telemetry export, and legacy adapters do not use this writer. ## Semantic Settlement Diagnostics and Retry Logs +When native tool arguments fail their authorized JSON Schema, the runner records +`harness.diagnostic` with code `semantic_tool_denied` and `paperclipExecuted: false`. +Its optional `inputSchemaGuidance` retains the same safe repair guidance sent to +the provider: at most three authorized schema locations within 512 bytes. +It includes no rejected arguments, submitted values, or unknown input keys. +The rejected call does not create a semantic input event or execution receipt. +Validated runner diagnostics retain this scalar metadata even when the operation +is an API or case tool. API request and response bodies keep their existing redaction. + Before interrupting a Codex turn, the runner durably records a `harness.diagnostic` event with code `provider_interrupt_requested`, the stop reason, provider turn ID, and pending tool call IDs. A harmless identical result diff --git a/maps/agent-access-map/onboarding.md b/maps/agent-access-map/onboarding.md index 4b55082465e..762ac1b5159 100644 --- a/maps/agent-access-map/onboarding.md +++ b/maps/agent-access-map/onboarding.md @@ -194,6 +194,11 @@ conversational resolution with a genuine later same-task user message supplies acceptance; an agent-only resolution does not. A plan approval must name the current same-task revision. Newer pending/rejected proposals stop new creation. Already admitted task replay and ordinary delegation retain their existing rules. +Native proposal metadata `version` and `supersedeOnUserComment` stays inside +`request_human_input.payload`. Invalid root metadata is rejected before dispatch; +repair guidance keeps the approval kind instead of replacing it with questions. +The shipped recipes are checked against the advertised schema and the real +interaction service. Idempotent proposal replay still needs a saved human decision. An accepted onboarding execution cannot create a new task blocked by its authenticated onboarding source. Native and REST creation return a recoverable input error before committing that task or its wake. The same source restriction diff --git a/maps/feature-map/onboarding.md b/maps/feature-map/onboarding.md index fb11e7001e9..e25d45fdc73 100644 --- a/maps/feature-map/onboarding.md +++ b/maps/feature-map/onboarding.md @@ -155,6 +155,11 @@ admit new work. [Admission controls](../../server/src/__tests__/onboarding-first exercise native and REST calls against disposable PostgreSQL; live model and Cloud onboarding remain separate evidence. +For a native proposal, reject root-level `version` or `supersedeOnUserComment` +without creating a card. Correct them inside `payload`, preserve the confirmation +or checkbox approval kind, and verify one pending card after idempotent replay. +An answered question form still cannot authorize the proposed child task. + After acceptance, reject a new task whose blockers include its onboarding source. The rejected command must leave no task, dependency, wake or creation receipt; a corrected retry must still create the ready child. Preserve valid blockers for diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs b/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs index 15dc55908f9..52ef2fac6b4 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/bin/fake-codex-app-server.rs @@ -709,6 +709,9 @@ fn run() -> Result<(), Box> { let repair_assigned_discovery = args .iter() .any(|value| value == "--repair-assigned-discovery"); + let repair_first_task_confirmation = args + .iter() + .any(|value| value == "--repair-first-task-confirmation"); let require_completion_contract = args .iter() .any(|value| value == "--require-completion-contract"); @@ -1007,6 +1010,53 @@ fn run() -> Result<(), Box> { } continue; } + if repair_first_task_confirmation + && message.get("method").is_none() + && message.get("id") == Some(&json!("first-task-confirmation-request")) + { + let text = message + .pointer("/result/contentItems/0/text") + .and_then(Value::as_str) + .ok_or("confirmation response omitted content")?; + let result: Value = serde_json::from_str(text)?; + if message.pointer("/result/success") == Some(&json!(false)) { + let hint = result + .pointer("/error/message") + .and_then(Value::as_str) + .ok_or("confirmation rejection omitted guidance")?; + if !hint.contains("payload.version") + || !hint.contains("payload.supersedeOnUserComment") + || !hint.contains("Keep approval") + || !hint.contains("confirmation or checkbox") + || hint.contains("payload.questionSet") + || !hint.contains("/additionalProperties") + || hint.contains("PRIVATE_CONFIRMATION_SCOPE") + { + return Err("confirmation rejection lacks metadata placement and approval-preserving guidance".into()); + } + log_call(call_log.as_deref(), "first-task:repair-guidance")?; + send( + json!({"id":"first-task-confirmation-request", "method":"item/tool/call", "params":{ + "threadId":state.thread_id, "turnId":state.active_turn_id, + "callId":"first-task-confirmation", "tool":"request_human_input", + "arguments": {"idempotencyKey":"first-task-proposal-confirmation", "interactionKind":"confirmation", + "title":"Approve child task", "prompt":"Approve PRIVATE_CONFIRMATION_SCOPE?", + "continuationPolicy":"wake_assignee", "payload":{"version":1,"supersedeOnUserComment":false, + "acceptLabel":"Approve","rejectLabel":"Revise","rejectRequiresReason":true}} + }}), + )?; + } else { + if message.pointer("/result/success") != Some(&json!(true)) + || result.pointer("/interaction/kind") != Some(&json!("request_confirmation")) + || result.pointer("/interaction/status") != Some(&json!("pending")) + { + return Err("corrected proposal did not return a pending confirmation".into()); + } + log_call(call_log.as_deref(), "first-task:confirmation-pending")?; + finish_turn(&state_path, &mut state, "completed")?; + } + continue; + } if message.get("method").is_none() && message.get("id") == Some(&json!("terminal-result-request")) { @@ -1756,6 +1806,17 @@ fn run() -> Result<(), Box> { "arguments":{"query":{"PRIVATE_QUERY":"private search"}} }}), )?; + } else if repair_first_task_confirmation { + send( + json!({"id":"first-task-confirmation-request", "method":"item/tool/call", "params":{ + "threadId":state.thread_id, "turnId":provider_turn_id, + "callId":"first-task-confirmation", "tool":"request_human_input", + "arguments": {"idempotencyKey":"first-task-proposal-confirmation", "interactionKind":"confirmation", + "title":"Approve child task", "prompt":"Approve PRIVATE_CONFIRMATION_SCOPE?", + "continuationPolicy":"wake_assignee", "payload":{"acceptLabel":"Approve","rejectLabel":"Revise","rejectRequiresReason":true}, + "version":1,"supersedeOnUserComment":false} + }}), + )?; } else if let Some(result) = &terminal_result { send(json!({ "id": "terminal-result-request", "method": "item/tool/call", diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs index 0587b9537dc..1547ce4a076 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs @@ -3533,17 +3533,21 @@ impl CodexCommandExecutor { .state .as_mut() .expect("Codex state remains available for a rejected tool call"); + let mut payload = json!({ + "provider": state.config.provider, + "code": "semantic_tool_denied", + "operationId": operation_id, + "callId": call_id, + "message": reason, + "paperclipExecuted": false, + }); + if let Some(guidance) = error.safe_provider_message() { + payload["inputSchemaGuidance"] = json!(guidance); + } let event = NormalizedProviderEvent { event_type: "harness.diagnostic".to_owned(), priority: EventPriority::P0, - payload: json!({ - "provider": state.config.provider, - "code": "semantic_tool_denied", - "operationId": operation_id, - "callId": call_id, - "message": reason, - "paperclipExecuted": false, - }), + payload, }; if state.receipt_limit_interrupt_pending { state.push_receipt_limit_cleanup_event(event)?; @@ -5032,15 +5036,29 @@ mod tests { let question_result = schema_rejection( "request_human_input", - json!({"PRIVATE_FIELD": "PRIVATE_VALUE"}), + json!({"interactionKind": "questions", "PRIVATE_FIELD": "PRIVATE_VALUE"}), ); let question_message = question_result.result["error"]["message"].as_str().unwrap(); assert!(question_message.contains("payload.questionSet")); + assert!(question_message.contains("Preserve question and option IDs")); assert!(question_message.contains("/required (missing \"requiredField\")")); assert!(!question_message.contains("PRIVATE_FIELD")); assert!(!question_result.result.to_string().contains("PRIVATE_VALUE")); assert!(question_message.len() <= 512); assert_eq!(question_result.result["error"]["retryable"], false); + for kind in ["confirmation", "checkbox"] { + let proposal = schema_rejection( + "request_human_input", + json!({"interactionKind": kind, "PRIVATE_FIELD": "PRIVATE_VALUE"}), + ); + let guidance = proposal.result["error"]["message"].as_str().unwrap(); + assert!(guidance.contains("payload.version")); + assert!(guidance.contains("payload.supersedeOnUserComment")); + assert!(guidance.contains("confirmation or checkbox")); + assert!(!guidance.contains("payload.questionSet")); + assert!(!proposal.result.to_string().contains("PRIVATE_VALUE")); + assert!(guidance.len() <= 512); + } let monitor_result = schema_rejection( "set_task_monitor", diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs b/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs index 684951199be..aeb60ac744b 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs @@ -40,7 +40,8 @@ const REPLAY_FILTER_WORDS: usize = 32_768; const ACTIVE_TURN_RECEIPT_LIMIT_MESSAGE: &str = "durable provider tool receipt limit reached for the active turn"; const COMPLETION_INPUT_SCHEMA_HINT: &str = "Invalid paperclip_finish arguments. Required fields: reportedWorkDisposition, summary, completionClaim, evidence, and verification. When reportedWorkDisposition is yielded, continuation must include kind=response_wake or monitor, summary, and idempotencyKey."; -const QUESTION_INPUT_SCHEMA_HINT: &str = "Invalid request_human_input arguments. Required fields: idempotencyKey, interactionKind, title, prompt, and continuationPolicy. For questions, use payload.version=1 and a complete payload.questionSet matching the authorized schema. Preserve the supplied question and option IDs."; +const HUMAN_INPUT_SCHEMA_HINT: &str = "Invalid request_human_input arguments. Put version and supersedeOnUserComment at payload.version and payload.supersedeOnUserComment, never at the root. For questions, use payload.version=1 and a complete payload.questionSet. Preserve question and option IDs. Correct the arguments and retry."; +const APPROVAL_INPUT_SCHEMA_HINT: &str = "Invalid request_human_input arguments. Put version and supersedeOnUserComment at payload.version and payload.supersedeOnUserComment, never at the root. Keep approval requests in their confirmation or checkbox interactionKind; do not replace them with questions. Correct the arguments and retry."; const BLOCK_INPUT_SCHEMA_HINT: &str = "Invalid paperclip_block arguments. Required fields: reportedWorkDisposition=blocked, summary, completionClaim, evidence, verification, and blocker. blocker must include reasonCode, owner, unblockAction, and scope."; const MONITOR_INPUT_SCHEMA_HINT: &str = "Invalid set_task_monitor arguments. Supply idempotencyKey and monitor: {nextCheckAt: , notes: }. Put all scheduling fields inside monitor, or use monitor: null to clear. Omit taskId for the current task. Correct the arguments and retry."; const API_INPUT_SCHEMA_HINT: &str = "Invalid call_api arguments. For responseText, use GET with offsetBytes >= 0 and responseText.limitBytes from 4 through 24576 bytes (default 24576). Follow nextOffsetBytes; null means complete. Correct the arguments and retry."; @@ -242,11 +243,16 @@ impl ProviderBridgeError { } } - fn input_schema_validation(operation_id: &str) -> Self { + fn input_schema_validation(operation_id: &str, input: &Value) -> Self { let safe_provider_message = match operation_id { "paperclip_finish" => Some(COMPLETION_INPUT_SCHEMA_HINT), "paperclip_block" => Some(BLOCK_INPUT_SCHEMA_HINT), - "request_human_input" => Some(QUESTION_INPUT_SCHEMA_HINT), + "request_human_input" => { + Some(match input.get("interactionKind").and_then(Value::as_str) { + Some("confirmation" | "checkbox") => APPROVAL_INPUT_SCHEMA_HINT, + _ => HUMAN_INPUT_SCHEMA_HINT, + }) + } "set_task_monitor" => Some(MONITOR_INPUT_SCHEMA_HINT), "call_api" => Some(API_INPUT_SCHEMA_HINT), _ => Some(TOOL_INPUT_SCHEMA_HINT), @@ -667,8 +673,10 @@ impl ProviderToolBridge { )) })?; if !validator.is_valid(&input) { - return Err(ProviderBridgeError::input_schema_validation(&operation_id) - .with_schema_locations(&validator, &input)); + return Err( + ProviderBridgeError::input_schema_validation(&operation_id, &input) + .with_schema_locations(&validator, &input), + ); } if matches!( operation_id.as_str(), @@ -678,7 +686,10 @@ impl ProviderToolBridge { .and_then(Value::as_str) .is_some_and(|summary| summary.chars().count() > MAX_COMPLETION_SUMMARY_CHARS) { - return Err(ProviderBridgeError::input_schema_validation(&operation_id)); + return Err(ProviderBridgeError::input_schema_validation( + &operation_id, + &input, + )); } bounded_json(&input, MAX_TOOL_VALUE_BYTES, "provider tool input")?; let call = PendingToolCall { @@ -1626,7 +1637,7 @@ mod tests { "c": {"type": "string"}, "d": {"type": "string"}, }}); let validator = jsonschema::validator_for(&schema).unwrap(); - let error = ProviderBridgeError::input_schema_validation("paperclip_finish") + let error = ProviderBridgeError::input_schema_validation("paperclip_finish", &json!({})) .with_schema_locations(&validator, &json!({"a": 1, "b": 2, "c": 3, "d": 4})); assert_eq!( error @@ -1636,7 +1647,7 @@ mod tests { .count(), 3 ); - let other = ProviderBridgeError::input_schema_validation("some_other_tool") + let other = ProviderBridgeError::input_schema_validation("some_other_tool", &json!({})) .with_schema_locations(&validator, &json!({"a": 1})); let message = other.safe_provider_message().unwrap(); assert!(message.starts_with(TOOL_INPUT_SCHEMA_HINT)); @@ -1662,8 +1673,9 @@ mod tests { json!({"idempotencyKey": "check-ci", "nextCheckAt": "PRIVATE_TIMESTAMP", "notes": "PRIVATE_VALUE"}), json!({"idempotencyKey": "check-ci", "monitor": {"nextCheckAt": "PRIVATE_TIMESTAMP", "notes": "PRIVATE_VALUE".repeat(100), "PRIVATE_FIELD": "PRIVATE_VALUE"}}), ] { - let error = ProviderBridgeError::input_schema_validation("set_task_monitor") - .with_schema_locations(&validator, &input); + let error = + ProviderBridgeError::input_schema_validation("set_task_monitor", &json!({})) + .with_schema_locations(&validator, &input); let message = error.safe_provider_message().unwrap(); assert!(message.contains("Put all scheduling fields inside monitor")); assert!(message.contains("/required") || message.contains("/properties/monitor/")); diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs index 5d8ccc10ab7..f4f05edc76d 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/codex_provider.rs @@ -21,6 +21,8 @@ use serde_json::{json, Value}; #[path = "support/codex_assigned_tool_validation.rs"] mod assigned_tool_validation; +#[path = "support/codex_human_input_validation.rs"] +mod human_input_validation; static NEXT_DIRECTORY: AtomicU64 = AtomicU64::new(1); static RECEIPT_LIMIT_TEST_LOCK: Mutex<()> = Mutex::new(()); diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_assigned_tool_validation.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_assigned_tool_validation.rs index fe029b8ae2d..9dfc24a3c2c 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_assigned_tool_validation.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_assigned_tool_validation.rs @@ -51,8 +51,26 @@ fn assigned_tool_discovery_repairs_invalid_arguments_through_codex_transport() { match event.event_type.as_str() { "harness.diagnostic" if event.payload["code"] == "semantic_tool_denied" => { denied += 1; + assert_eq!(dispatched, 0); + assert_eq!( + event.payload["operationId"], + "paperclip_search_assigned_tools" + ); + assert_eq!(event.payload["callId"], "assigned-discovery"); + assert_eq!( + event.payload["message"], + "provider arguments for paperclip_search_assigned_tools failed JSON Schema validation" + ); assert_eq!(event.payload["paperclipExecuted"], false); + let guidance = event.payload["inputSchemaGuidance"] + .as_str() + .expect("denied input retains the safe guidance sent to the provider"); + assert!(guidance.contains("/properties/query/type"), "{guidance}"); + assert!(guidance.len() <= 512); + assert!(event.payload.get("input").is_none()); + assert!(event.payload.get("arguments").is_none()); assert!(!event.payload.to_string().contains("PRIVATE_QUERY")); + assert!(!event.payload.to_string().contains("private search")); } "semantic_tool.input" => { dispatched += 1; diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_human_input_validation.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_human_input_validation.rs new file mode 100644 index 00000000000..5a09092158a --- /dev/null +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_human_input_validation.rs @@ -0,0 +1,107 @@ +use super::*; + +#[test] +fn first_task_confirmation_repairs_misplaced_metadata_without_becoming_a_question() { + let directory = temporary_directory("first-task-confirmation-repair"); + // Read the current generated schema at test execution instead of embedding + // test-only data outside the release crate's hashed build-input closure. + let contracts: Value = serde_json::from_str( + &fs::read_to_string( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../../generated/capability/semantic-tool-contracts.json"), + ) + .expect("read the generated semantic tool contracts"), + ) + .expect("parse the generated semantic tool contracts"); + let definition = contracts + .as_array() + .unwrap() + .iter() + .find(|tool| tool["name"] == "request_human_input") + .unwrap(); + let operations = vec![AuthorizedTool { + operation_id: "request_human_input".to_owned(), + version: 1, + description: definition["description"].as_str().unwrap().to_owned(), + input_schema: definition["inputSchema"].clone(), + response_schema: json!({"type":"object"}), + }]; + let tools = AuthorizedToolSet { + schema: TOOL_SET_SCHEMA.to_owned(), + schema_version: 1, + catalog_digest: authorized_tool_catalog_digest(&operations).unwrap(), + operations, + }; + let mut executor = + CodexCommandExecutor::with_runner_config(&directory, &durable_config(&directory)); + executor.execute(&command("prepare", 1, "run.prepare", json!({ + "provider":provider_config(&directory, &["--repair-first-task-confirmation"]), "authorizedTools":tools + }))).unwrap(); + executor + .execute(&command("open", 2, "session.open", json!({}))) + .unwrap(); + executor + .execute(&command( + "turn", + 3, + "turn.start", + json!({"text":"Propose one child task and wait for approval."}), + )) + .unwrap(); + let mut denied = 0; + let mut dispatched = 0; + let mut completed = false; + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !completed && std::time::Instant::now() < deadline { + for event in poll_and_ack(&mut executor).unwrap() { + match event.event_type.as_str() { + "harness.diagnostic" if event.payload["code"] == "semantic_tool_denied" => { + denied += 1; + assert_eq!(dispatched, 0); + assert_eq!(event.payload["paperclipExecuted"], false); + let hint = event.payload["inputSchemaGuidance"].as_str().unwrap(); + assert!(hint.contains("payload.version")); + assert!(hint.contains("payload.supersedeOnUserComment")); + assert!(hint.contains("Keep approval")); + assert!(hint.contains("confirmation or checkbox")); + assert!(!hint.contains("payload.questionSet")); + assert!(hint.contains("/additionalProperties")); + assert!(hint.len() <= 512); + assert!(!event + .payload + .to_string() + .contains("PRIVATE_CONFIRMATION_SCOPE")); + } + "semantic_tool.input" => { + dispatched += 1; + assert_eq!(denied, 1); + let input = &event.payload["semantic_tool"]["input"]; + assert_eq!(input["interactionKind"], "confirmation"); + assert!(input.get("version").is_none()); + assert!(input.get("supersedeOnUserComment").is_none()); + assert_eq!(input["payload"]["version"], 1); + assert_eq!(input["payload"]["supersedeOnUserComment"], false); + executor.execute(&command("result", 4, "semantic_tool.result", json!({ + "callId":"first-task-confirmation", "operationId":"request_human_input", + "result":{"interaction":{"id":"proposal-card","kind":"request_confirmation","status":"pending"}}, + "isError":false + }))).unwrap(); + } + "turn.completed" => completed = true, + "turn.failed" => panic!( + "proposal could not correct its arguments: {}", + event.payload + ), + _ => {} + } + } + std::thread::sleep(std::time::Duration::from_millis(1)); + } + executor.shutdown().unwrap(); + assert!(completed); + assert_eq!(denied, 1); + assert_eq!(dispatched, 1); + assert_eq!(call_count(&directory, "first-task:repair-guidance"), 1); + assert_eq!(call_count(&directory, "first-task:confirmation-pending"), 1); + fs::remove_dir_all(directory).unwrap(); +} diff --git a/server/src/__tests__/onboarding-first-task-admission.test.ts b/server/src/__tests__/onboarding-first-task-admission.test.ts index e91f88b0417..c87ef2c096a 100644 --- a/server/src/__tests__/onboarding-first-task-admission.test.ts +++ b/server/src/__tests__/onboarding-first-task-admission.test.ts @@ -1,6 +1,7 @@ import { randomUUID } from "node:crypto"; import { readFile } from "node:fs/promises"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import Ajv2020 from "ajv/dist/2020.js"; import { eq } from "drizzle-orm"; import { agentWakeupRequests, authUsers, companyMemberships, heartbeatRuns, issueComments, issueThreadInteractions, issues } from "@paperclipai/db"; import { createIssueThreadInteractionSchema } from "@paperclipai/shared"; @@ -13,6 +14,7 @@ import { initializeRunIdentity } from "../services/run-identity.js"; import { issueService } from "../services/issues.js"; import { recoveryPostgresSupport as support } from "./helpers/recovery-postgres.js"; import { startRunnerApiTestServer } from "./helpers/runner-api-server.js"; +import { CAPABILITY_SEMANTIC_TOOL_CATALOG } from "../vendor/paperclip-runner/index.js"; async function nativeProposalRecipe(kind: "confirmation" | "checkbox", planRevisionId = randomUUID()) { const skill = await readFile(new URL("../onboarding-assets/first-task/skills/first-task/SKILL.md", import.meta.url), "utf8"); @@ -25,6 +27,12 @@ async function nativeProposalRecipe(kind: "confirmation" | "checkbox", planRevis it.each(["confirmation", "checkbox"] as const)("ships a native %s recipe accepted by the production interaction schema", async kind => { const recipe = await nativeProposalRecipe(kind); + const schema = CAPABILITY_SEMANTIC_TOOL_CATALOG.find(tool => tool.operationId === "request_human_input")!.inputSchema; + const validator = new Ajv2020({ allErrors: true, strict: false }); + expect(validator.validate(schema, recipe), JSON.stringify(validator.errors)).toBe(true); + expect(recipe).not.toHaveProperty("version"); + expect(recipe).not.toHaveProperty("supersedeOnUserComment"); + expect(recipe.payload).toMatchObject({ version: 1, supersedeOnUserComment: false }); const parsed = createIssueThreadInteractionSchema.parse({ kind: kind === "confirmation" ? "request_confirmation" : "request_checkbox_confirmation", idempotencyKey: recipe.idempotencyKey, title: recipe.title, continuationPolicy: recipe.continuationPolicy, payload: { ...(recipe.payload as Record), version: 1, prompt: recipe.prompt } }); @@ -427,7 +435,10 @@ describe.skipIf(!support.supported)("onboarding first-task execution admission", title: "Approved child plan", format: "markdown", body: "Create one child for the proposed deliverable.", createdByAgentId: f.agentId, createdByRunId: f.runId })).document : null; const args = await nativeProposalRecipe(kind, plan?.latestRevisionId ?? undefined); - const { interaction } = await f.authority.execute({ tool: "request_human_input", callId: randomUUID(), arguments: args }) as { interaction: { id: string; kind: string } }; + const receipt = await f.authority.execute({ tool: "request_human_input", callId: randomUUID(), arguments: args }) as { interaction: { id: string; kind: string } }; + const { interaction } = receipt; + await expect(f.authority.execute({ tool: "request_human_input", callId: randomUUID(), arguments: args })).resolves.toEqual(receipt); + expect(await server.db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.issueId, f.issueId))).toHaveLength(1); expect(interaction.kind).toBe(kind === "confirmation" ? "request_confirmation" : "request_checkbox_confirmation"); await expect(create(f)).rejects.toThrow(/proposal.*accept/i); const selectedOptionIds = kind === "checkbox" ? ((args.payload as { options: { id: string }[] }).options.map(option => option.id)) : undefined; diff --git a/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md b/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md index e04f30dffff..9afe2d13692 100644 --- a/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md +++ b/server/src/onboarding-assets/first-task/skills/first-task/SKILL.md @@ -40,6 +40,8 @@ Work in this order. For a native single-task confirmation, call `request_human_input` with arguments like these. Replace `` with the proposed title and describe its actual scope in the prompt. Use a new idempotency key for each changed or revised proposal. Reuse a key only for the exact same proposal when reconciling an interrupted call; the example key below is for the first proposal. + The tool's root fields are `idempotencyKey`, `interactionKind`, `title`, `prompt`, `continuationPolicy`, and `payload`, with optional `targetRevisionId` and `resolverPolicy` as advertised. Put `version` and `supersedeOnUserComment` inside `payload`; neither is a root argument. If validation rejects a proposal, correct its argument placement and keep the confirmation or checkbox approval kind. Switching it to a question form cannot authorize the proposed work. Wait for a successful saved card receipt before claiming you requested approval. + ```json { "idempotencyKey": "first-task-proposal-confirmation", @@ -48,6 +50,8 @@ Work in this order. "prompt": "Approve creating exactly one child task named \"\" for the scope described above?", "continuationPolicy": "wake_assignee", "payload": { + "version": 1, + "supersedeOnUserComment": false, "acceptLabel": "Approve", "rejectLabel": "Revise", "rejectRequiresReason": true @@ -65,6 +69,8 @@ Work in this order. "prompt": "Approve the proposed scope in this saved plan?", "continuationPolicy": "wake_assignee", "payload": { + "version": 1, + "supersedeOnUserComment": false, "options": [{ "id": "scope", "label": "Approve the proposed scope for " }], "defaultSelectedOptionIds": ["scope"], "minSelected": 1, diff --git a/server/src/services/native-runtime/paperclip-control-plane-port.test.ts b/server/src/services/native-runtime/paperclip-control-plane-port.test.ts index 7e68810fc6d..57c2d3e6289 100644 --- a/server/src/services/native-runtime/paperclip-control-plane-port.test.ts +++ b/server/src/services/native-runtime/paperclip-control-plane-port.test.ts @@ -486,6 +486,49 @@ describe("PaperclipControlPlanePort conformance", () => { expect(result.data.credentials).toBe("private-history-fixture-value"); }); + it.each(["call_api", "create_case"])("retains bounded %s schema-denial guidance through persistence and replay", async operationId => { + const identity = { ...CONTROL_PLANE_CONFORMANCE_OPEN.identity, runId: randomUUID(), sessionId: randomUUID() }; + const runnerId = randomUUID(); + await db.insert(heartbeatRuns).values({ + id: identity.runId, companyId: identity.companyId, agentId: identity.agentId, + status: "running", runtimeMode: "native", nativeIssueId: identity.issueId, + nativeSessionId: identity.sessionId, runnerInstanceId: runnerId, + completionContractId: contractId, completionContractSha256: contractSha, + contextSnapshot: { issueId: identity.issueId }, + }); + const observed: PrpEvent[] = []; + const port = new PaperclipControlPlanePort(db, { + ...identity, completionContractId: contractId, completionContractSha256: contractSha, + sourceInstanceId: runnerId, controlPlaneSourceInstanceId: `api-${identity.runId}`, + }, { onCommittedEvent: async event => { observed.push(event); } }); + await port.openRun({ identity, backendKind: "mock", sourceInstanceId: runnerId }); + const payload = { + provider: "codex", code: "semantic_tool_denied", operationId, callId: "schema-denied", + message: `provider arguments for ${operationId} failed JSON Schema validation`, paperclipExecuted: false, + inputSchemaGuidance: "Invalid tool arguments. Check these input schema locations: /required (missing title).", + }; + const event: PrpEvent = { + schema: "paperclip.prp.event.v1", sourceEventId: `${runnerId}:1`, sourceSeq: 1, + sourceInstanceId: runnerId, sourceKind: "runner", runId: identity.runId, + normalizedSessionId: identity.sessionId, eventType: "harness.diagnostic", schemaVersion: 1, + priority: 0, emittedAt: new Date().toISOString(), payload, + debug: { operationId, data: { value: "private-diagnostic-debug" } }, + }; + await port.appendEvent(event); + expect(observed[0]?.payload).toEqual(payload); + expect(observed[0]?.debug).toMatchObject({ apiPayloadRedacted: true }); + expect(JSON.stringify(observed[0])).not.toContain("private-diagnostic-debug"); + expect((await port.appendEvent(event)).disposition).toBe("duplicate"); + expect(port.normalizeReplayPayload(payload)).toEqual(port.normalizeReplayPayload(observed[0]!.payload)); + const saved = await db.select().from(heartbeatRunEvents).where(eq(heartbeatRunEvents.runId, identity.runId)); + expect(saved).toHaveLength(1); + expect((saved[0]!.payload?.prpEvent as unknown as PrpEvent).payload).toEqual(payload); + expect(JSON.stringify(saved)).not.toContain("private-diagnostic-debug"); + await expect(port.appendEvent({ ...event, payload: { + ...payload, inputSchemaGuidance: "Invalid tool arguments. Check these input schema locations: /properties/title/type.", + } })).rejects.toThrow(); + }); + it("redacts identity private material before persisting native events and checkpoints", async () => { const identity = { ...CONTROL_PLANE_CONFORMANCE_OPEN.identity, runId: randomUUID(), sessionId: randomUUID() }; const runnerId = randomUUID(); diff --git a/server/src/services/native-runtime/paperclip-control-plane-port.ts b/server/src/services/native-runtime/paperclip-control-plane-port.ts index 47faa2e5231..ef0dfa235cb 100644 --- a/server/src/services/native-runtime/paperclip-control-plane-port.ts +++ b/server/src/services/native-runtime/paperclip-control-plane-port.ts @@ -29,7 +29,7 @@ import { import { appendHeartbeatRunEvent } from "../heartbeat-run-events.js"; import { publishLiveEvent } from "../live-events.js"; import { nativeSha256 } from "./canonical.js"; -import { redactRunnerApiPayloads } from "./runner-api-log-redaction.js"; +import { redactRunnerApiEvent, redactRunnerApiPayloads } from "./runner-api-log-redaction.js"; export interface PaperclipControlPlaneBinding { companyId: string; @@ -262,7 +262,7 @@ export class PaperclipControlPlanePort implements ControlPlanePort { } } } - event = redactRunnerApiPayloads(event); + event = redactRunnerApiEvent(event); const persisted = await this.#db.transaction(async tx => { this.#assertActive(options); const receipt = await appendHeartbeatRunEvent(tx as unknown as Db, { diff --git a/server/src/services/native-runtime/runner-api-log-redaction.test.ts b/server/src/services/native-runtime/runner-api-log-redaction.test.ts index 95b3d403d63..3862c53f0a1 100644 --- a/server/src/services/native-runtime/runner-api-log-redaction.test.ts +++ b/server/src/services/native-runtime/runner-api-log-redaction.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { redactRunnerApiPayloads } from "./runner-api-log-redaction.js"; +import { redactRunnerApiEvent, redactRunnerApiPayloads } from "./runner-api-log-redaction.js"; describe("API payload persistence redaction", () => { it.each(["POST /api/agents/me/secrets/{key}/value", "POST /api/companies/{companyId}/exports", "GET /api/projects/{id}"])("preserves private HTTP results on the wire, but omits %s from history", operationId => { @@ -26,6 +26,39 @@ describe("API payload persistence redaction", () => { const value = { result: { operationId: "get_task_context", data: { title: "Work" } }, text: '{ "ordinary": "output" }' }; expect(redactRunnerApiPayloads(value)).toEqual(value); }); + it.each(["call_api", "create_case"])("keeps %s denial metadata out of the API-body exception", operationId => { + const payload = { + provider: "codex", operationId, callId: "schema-denied", code: "semantic_tool_denied", + paperclipExecuted: false, message: "Provider arguments failed JSON Schema validation.", + inputSchemaGuidance: "Invalid tool arguments. Check these input schema locations: /properties/title/type.", + }; + const event = { sourceKind: "runner", eventType: "harness.diagnostic", payload }; + expect(redactRunnerApiEvent(event)).toEqual(event); + const debugEvent = { ...event, debug: { operationId, data: { value: "private-debug-value" } } }; + const savedDebug = redactRunnerApiEvent(debugEvent); + expect(savedDebug.payload).toEqual(payload); + expect(savedDebug.debug).toMatchObject({ apiPayloadRedacted: true }); + expect(JSON.stringify(savedDebug)).not.toContain("private-debug-value"); + expect(redactRunnerApiEvent(savedDebug)).toEqual(savedDebug); + for (const value of [ + { ...event, eventType: "item.completed" }, + { ...event, sourceKind: "control_plane" }, + { ...event, payload: { ...payload, arguments: { value: "private-fixture-value" } } }, + { ...event, payload: { ...payload, inputSchemaGuidance: "x".repeat(513) } }, + { ...event, payload: { ...payload, inputSchemaGuidance: "unsafe\nmetadata" } }, + { ...event, payload: { ...payload, inputSchemaGuidance: { value: "private-fixture-value" } } }, + ]) { + const saved = redactRunnerApiEvent(value); + expect(saved.payload).toMatchObject({ apiPayloadRedacted: true }); + expect(saved.payload).not.toHaveProperty("inputSchemaGuidance"); + expect(JSON.stringify(saved)).not.toContain("private-fixture-value"); + } + const nested = { sourceKind: "runner", eventType: "item.completed", payload: { + operationId, data: { ...event, payload: { ...payload, inputSchemaGuidance: "private-fixture-value" } }, + } }; + expect(JSON.stringify(redactRunnerApiEvent(nested))).not.toContain("private-fixture-value"); + expect(redactRunnerApiPayloads(payload)).not.toHaveProperty("inputSchemaGuidance"); + }); it("omits malformed API arguments and semantic result envelopes", () => { const value = { item: { type: "tool_use", name: "call_api", input: { operationId: "invalid", body: { value: "private-fixture-value" } } }, result: { operationId: "call_api", result: { data: "private-fixture-value" } } }; diff --git a/server/src/services/native-runtime/runner-api-log-redaction.ts b/server/src/services/native-runtime/runner-api-log-redaction.ts index 9be08bb9661..58091b32f18 100644 --- a/server/src/services/native-runtime/runner-api-log-redaction.ts +++ b/server/src/services/native-runtime/runner-api-log-redaction.ts @@ -4,6 +4,23 @@ import { nativeSha256 } from "./canonical.js"; const RECEIPT_FIELDS = new Set(["operationId", "apiOperationId", "callId", "ok", "status", "contentType"]); const REDACTED_FIELDS = new Set([...RECEIPT_FIELDS, "data", "apiPayloadRedacted", "apiPayloadSha256"]); const CASE_OPERATIONS = new Set(["create_case", "read_case", "update_case", "read_case_document", "write_case_document", "list_workflows", "read_workflow", "create_workflow", "update_workflow"]); +const SCHEMA_DENIAL_STRINGS = ["provider", "operationId", "callId", "message", "inputSchemaGuidance"]; +const SCHEMA_DENIAL_FIELDS = new Set([...SCHEMA_DENIAL_STRINGS, "code", "paperclipExecuted"]); + +/** Only use after the PRP event's run and source bindings have been checked. */ +export function redactRunnerApiEvent(event: T): T { + const payload = event.payload && typeof event.payload === "object" ? event.payload as Record : {}; + // These scalar fields are authored by the runner's validation boundary, not + // an API request or result. Never apply this exception inside arbitrary data. + if (event.sourceKind === "runner" && event.eventType === "harness.diagnostic" + && payload.code === "semantic_tool_denied" && payload.paperclipExecuted === false + && Object.keys(payload).every(key => SCHEMA_DENIAL_FIELDS.has(key)) + && SCHEMA_DENIAL_STRINGS.every(key => typeof payload[key] === "string" + && payload[key].length > 0 && payload[key].length <= 512 && /^[\x20-\x7e]+$/.test(payload[key]))) { + return { ...redactRunnerApiPayloads(event), payload: event.payload }; + } + return redactRunnerApiPayloads(event); +} function redactedPayload(value: unknown) { const entry = value && typeof value === "object" ? value as Record : {};