fix(runner): explain invalid arguments for assigned tools (#16905)

## Thinking Path > - Paperclip coordinates agents through authorized tools. > - The native runner validates arguments before it dispatches a tool call. > - Schema failures give repair guidance for only five built-in tools. > - Assigned-tool discovery receives a generic rejection, so an agent can mistake invalid arguments for missing access. > - This PR applies the existing safe schema feedback to every authorized tool. > - Agents can correct their calls while authorization, approval, and replay checks remain in force. ## Linked Issues or Issue Description **What happened?** An invalid `paperclip_search_assigned_tools` call returned only `Paperclip rejected this semantic tool call`. The call never reached the assigned-tool gateway. The same feedback gap affects arbitrary assigned app tools. **Expected behavior** Identify argument validation as the cause. Return bounded schema locations and ask the provider to correct its arguments. Do not expose submitted values or grant access to unassigned tools. **Steps to reproduce** Prepare an authorized discovery tool. Submit an invalid query or optional argument through the native runner. Observe the generic error before this fix. Correct the arguments and confirm one dispatch after this fix. **Paperclip version or commit** Reproduced on master at `4970d5ed3d`. The incident did not retain its rejected arguments, so the tests cover representative invalid inputs rather than claim to replay that exact payload. **Deployment mode** Native runner with the Codex app-server driver. Local verification uses macOS ARM64 and controlled provider fixtures. Related: #16868 added bounded monitor validation feedback. #16723 changes runtime MCP catalog loading; this PR changes the Rust runner validation response and does not depend on that work. ## What Changed - Supply generic repair guidance for any authorized tool whose arguments fail JSON Schema validation. Preserve specific built-in hints. - Keep the existing three-location and 512-byte limits. Do not include input values or unknown keys. Preserve generic authorization failures and the existing retry flag. - Cover missing fields, wrong types, null optional fields, bounds, unknown fields, unchanged rejected-call state, corrected calls, and completed-call replay protection. - Exercise rejection, correction, one dispatch, and successful completion through the real Codex driver with a controlled provider subprocess. - Document the behavior and its verification scope in execution semantics and the agent access map. ## Verification - Red: both new runner regressions failed on the original generic rejection. The existing API-validation regression passed. - Green: all seven selected validation tests passed. The new Codex transport regression passed with one rejection and one corrected dispatch. - `cargo fmt --manifest-path packages/paperclip-runner/runner/Cargo.toml --all -- --check`, `node scripts/check-maps.mjs`, and `git diff --check` passed. - Full Rust workspace tests, the repository build, and sequential repository typecheck passed. The existing assigned-tool gateway suite passed all 30 tests. All hosted test and end-to-end jobs passed on this head. The duplicate local `test:run` was interrupted after the hosted suites passed; its local result is incomplete. The hosted build and clean public npm installation check also passed. All current-head CI gates are green, including the final verify and e2e aggregates. - Greptile reviewed commit `3e2743e1e15b3b13a7d803ce4d828510e41fb22a`: 5/5, no actionable findings. - Setup evidence: master has a stale dependency lockfile. Dependencies were resolved with the pinned pnpm 9.15.4; lockfile changes are excluded. The first full typecheck overlapped the build and failed copying a generated shared-package file. The sequential rerun passed with no source change. The initial copy failure remains recorded here; the concurrent build-output race was not separately reproduced. - The provider is simulated. This proves runner validation and transport recovery, not that a live model always retries or that GitHub credentials work. ## Risks The provider now sees schema locations for all authorized tools. The existing sanitizer and bounds still apply. Unknown input keys and values remain excluded. No schema, permission, approval, automatic tool replay, database, or connection-auth changes are made. Unauthorized tools still get a generic rejection. ## Model Used OpenAI Codex (GPT-6), with reasoning, source inspection, and tool execution. The exact runtime model ID and context-window size are not exposed in this session. ## 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 (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] 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 Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-21 00:05:49 +02:00 · 2026-10-19 17:03:42 -05:00
1 parent e5927e1db9
commit 7ec6e1a237
@@ -467,6 +467,15 @@ Rejected API fallback arguments return the same bounded schema locations as
 monitor calls, with no submitted values. Correct the arguments before retrying;
 validation does not dispatch an HTTP request or relax route permissions.

+This validation feedback applies to every authorized native tool, including
+`paperclip_search_assigned_tools`, `paperclip_call_assigned_tool`, and direct app
+tools. Invalid arguments return repair guidance and at most three locations in
+the authorized input schema, within 512 bytes. Feedback excludes submitted values
+and unknown input keys. A rejected call creates no execution receipt; corrected
+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.
+
 A native provider may omit `continuation` (or send `null`) when it reports
 `yielded` after saving a monitor. The controller reads current authorized task
 context and fills the canonical `monitor` continuation only if that same task,
@@ -97,6 +97,15 @@ approvals and revocation checks remain independent. Regression coverage lives in
 [assigned-tool tests](../../server/src/services/native-runtime/assigned-mcp-tools.test.ts);
 these fixtures do not certify a live external-client journey.

+Native assigned tools return bounded input-schema repair guidance after argument
+validation fails. Rejection creates no execution receipt and discloses no submitted
+values or unknown keys. Corrected calls keep the same authorization checks.
+[Runner regressions](../../packages/paperclip-runner/runner/crates/runner-core/src/tests/provider_assigned_tool_validation.rs)
+cover discovery repair and unauthorized calls; the
+[Codex transport fixture](../../packages/paperclip-runner/runner/crates/runner-core/tests/support/codex_assigned_tool_validation.rs)
+covers rejection, corrected dispatch, and completion with a simulated provider.
+This does not establish live model or GitHub access.
+
 Remote invocation regressions preserve access to authorized sibling tools after
 a fetch failure, timeout, non-authentication HTTP error, or JSON-RPC rejection.
 HTTP 401/403 and insufficient-scope errors keep their connection gates. The
@@ -682,6 +682,9 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
        .iter()
        .any(|value| value == "--finish-turn-with-pending-tool");
    let require_dynamic_tool = args.iter().any(|value| value == "--require-dynamic-tool");
+    let repair_assigned_discovery = args
+        .iter()
+        .any(|value| value == "--repair-assigned-discovery");
    let require_completion_contract = args
        .iter()
        .any(|value| value == "--require-completion-contract");
@@ -934,6 +937,45 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
            )?;
            continue;
        }
+        if repair_assigned_discovery
+            && message.get("method").is_none()
+            && message.get("id") == Some(&json!("discovery-request"))
+        {
+            let text = message
+                .pointer("/result/contentItems/0/text")
+                .and_then(Value::as_str)
+                .ok_or("discovery 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("discovery rejection omitted guidance")?;
+                if !hint.contains("Correct the arguments and retry")
+                    || !hint.contains("/properties/query/type")
+                    || hint.contains("PRIVATE_QUERY")
+                {
+                    return Err("discovery rejection lacks safe repair guidance".into());
+                }
+                log_call(call_log.as_deref(), "discovery:repair-guidance")?;
+                send(
+                    json!({"id":"discovery-request", "method":"item/tool/call", "params":{
+                        "threadId":state.thread_id, "turnId":state.active_turn_id,
+                        "callId":"assigned-discovery", "tool":"paperclip_search_assigned_tools",
+                        "arguments":{"query":"github"}
+                    }}),
+                )?;
+            } else {
+                if message.pointer("/result/success") != Some(&json!(true))
+                    || result != json!({"tools":[{"name":"app_github_read_pull_request"}]})
+                {
+                    return Err("corrected discovery did not return assigned tools".into());
+                }
+                log_call(call_log.as_deref(), "discovery:success")?;
+                finish_turn(&state_path, &mut state, "completed")?;
+            }
+            continue;
+        }
        if message.get("method").is_none() && message.get("id") == Some(&json!("tool-request-1")) {
            if message.pointer("/result/success") == Some(&json!(false)) {
                log_call(call_log.as_deref(), "tool-response:failure")?;
@@ -1656,6 +1698,14 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
                    }))?;
                } else if exit_after_turn_start {
                    return Ok(());
+                } else if repair_assigned_discovery {
+                    send(
+                        json!({"id":"discovery-request", "method":"item/tool/call", "params":{
+                            "threadId":state.thread_id, "turnId":provider_turn_id,
+                            "callId":"assigned-discovery", "tool":"paperclip_search_assigned_tools",
+                            "arguments":{"query":{"PRIVATE_QUERY":"private search"}}
+                        }}),
+                    )?;
                } else if emit_tool_call {
                    send(json!({
                        "id": "tool-request-1",
@@ -4999,7 +4999,7 @@ mod tests {
    }

    #[test]
-    fn invalid_tool_results_expose_only_reserved_schema_guidance() {
+    fn invalid_tool_results_expose_authorized_schema_guidance() {
        let finish_result = schema_rejection(
            "paperclip_finish",
            json!({"secretSubmittedValue": "must-not-appear"}),
@@ -5052,10 +5052,9 @@ mod tests {
        assert_eq!(monitor_result.result["error"]["retryable"], false);

        let ordinary_result = schema_rejection("get_task_context", json!({}));
-        assert_eq!(
-            ordinary_result.result["error"]["message"],
-            GENERIC_INVALID_TOOL_CALL_MESSAGE
-        );
+        let ordinary_message = ordinary_result.result["error"]["message"].as_str().unwrap();
+        assert!(ordinary_message.contains("Correct the arguments and retry"));
+        assert!(ordinary_message.contains("/required (missing \"requiredField\")"));

        let unauthorized_error = ProviderToolBridge::default()
            .begin_call(
@@ -6543,3 +6542,7 @@ mod tests {
 #[cfg(test)]
 #[path = "tests/provider_api_validation.rs"]
 mod api_validation_tests;
+
+#[cfg(test)]
+#[path = "tests/provider_assigned_tool_validation.rs"]
+mod assigned_tool_validation_tests;
@@ -44,6 +44,7 @@ const QUESTION_INPUT_SCHEMA_HINT: &str = "Invalid request_human_input arguments.
 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: <future UTC timestamp>, notes: <next check>}. 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.";
+const TOOL_INPUT_SCHEMA_HINT: &str = "Invalid tool arguments. The tool was not executed. Match its authorized input schema. Correct the arguments and retry.";

 #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
 #[serde(rename_all = "camelCase")]
@@ -248,7 +249,7 @@ impl ProviderBridgeError {
            "request_human_input" => Some(QUESTION_INPUT_SCHEMA_HINT),
            "set_task_monitor" => Some(MONITOR_INPUT_SCHEMA_HINT),
            "call_api" => Some(API_INPUT_SCHEMA_HINT),
-            _ => None,
+            _ => Some(TOOL_INPUT_SCHEMA_HINT),
        };
        Self {
            message: format!("provider arguments for {operation_id} failed JSON Schema validation"),
@@ -257,8 +258,6 @@ impl ProviderBridgeError {
    }

    fn with_schema_locations(mut self, validator: &jsonschema::Validator, input: &Value) -> Self {
-        // Only the reserved tools above have a provider-safe hint. Report bounded paths
-        // into the authorized schema, never the submitted values or unknown keys.
        if let Some(message) = self.safe_provider_message.as_mut() {
            let locations = validator
                .iter_errors(input)
@@ -1612,7 +1611,9 @@ mod tests {
        );
        let other = ProviderBridgeError::input_schema_validation("some_other_tool")
            .with_schema_locations(&validator, &json!({"a": 1}));
-        assert_eq!(other.safe_provider_message(), None);
+        let message = other.safe_provider_message().unwrap();
+        assert!(message.starts_with(TOOL_INPUT_SCHEMA_HINT));
+        assert!(message.contains("/properties/a/type"));
    }

    #[test]
@@ -0,0 +1,144 @@
+use super::*;
+use crate::provider_bridge::AuthorizedTool;
+
+fn authorized_bridge(operation_id: &str, input_schema: Value) -> ProviderToolBridge {
+    let operations = vec![AuthorizedTool {
+        operation_id: operation_id.to_owned(),
+        version: 1,
+        description: "Search assigned tools.".to_owned(),
+        input_schema,
+        response_schema: json!({"type": "object"}),
+    }];
+    let mut bridge = ProviderToolBridge::default();
+    bridge
+        .prepare(AuthorizedToolSet {
+            schema: TOOL_SET_SCHEMA.to_owned(),
+            schema_version: 1,
+            catalog_digest: authorized_tool_catalog_digest(&operations).unwrap(),
+            operations,
+        })
+        .unwrap();
+    bridge
+}
+
+#[test]
+fn assigned_tool_discovery_rejection_guides_repair_without_retaining_a_receipt() {
+    let operation_id = "paperclip_search_assigned_tools";
+    let mut bridge = authorized_bridge(
+        operation_id,
+        json!({
+            "type": "object", "additionalProperties": false,
+            "properties": {
+                "query": {"type": "string", "maxLength": 200},
+                "offset": {"type": "integer", "minimum": 0},
+                "limit": {"type": "integer", "minimum": 1, "maximum": 20},
+                "schemaTool": {"type": "string"},
+                "schemaOffset": {"type": "integer", "minimum": 0}
+            },
+            "required": ["query"]
+        }),
+    );
+    let initial_state = serde_json::to_value(&bridge).unwrap();
+    for (input, location) in [
+        (json!({}), "/required (missing \"query\")"),
+        (json!({"query": null}), "/properties/query/type"),
+        (
+            json!({"query": "PRIVATE_QUERY", "offset": null}),
+            "/properties/offset/type",
+        ),
+        (
+            json!({"query": "PRIVATE_QUERY", "limit": 21}),
+            "/properties/limit/maximum",
+        ),
+        (
+            json!({"query": "PRIVATE_QUERY".repeat(100)}),
+            "/properties/query/maxLength",
+        ),
+        (
+            json!({"query": "PRIVATE_QUERY", "PRIVATE_FIELD": "PRIVATE_VALUE"}),
+            "/additionalProperties",
+        ),
+    ] {
+        let error = bridge
+            .begin_call("discovery".to_owned(), operation_id.to_owned(), input)
+            .unwrap_err();
+        let result =
+            invalid_tool_call_result("discovery".to_owned(), operation_id.to_owned(), &error);
+        let message = result.result["error"]["message"].as_str().unwrap();
+        assert!(
+            message.contains("Correct the arguments and retry"),
+            "{message}"
+        );
+        assert!(message.contains(location), "{message}");
+        for private in ["PRIVATE_QUERY", "PRIVATE_FIELD", "PRIVATE_VALUE"] {
+            assert!(!result.result.to_string().contains(private));
+        }
+        assert!(message.len() <= 512);
+        assert!(!message.chars().any(char::is_control));
+        assert!(result.is_error);
+        assert_eq!(result.result["error"]["code"], "invalid_tool_call");
+        assert_eq!(result.result["error"]["retryable"], false);
+        assert!(!bridge.has_call_receipt("discovery"));
+        assert_eq!(serde_json::to_value(&bridge).unwrap(), initial_state);
+    }
+
+    let input = json!({"query": "github"});
+    bridge
+        .begin_call(
+            "discovery".to_owned(),
+            operation_id.to_owned(),
+            input.clone(),
+        )
+        .unwrap();
+    assert_eq!(bridge.pending_calls().count(), 1);
+    let result = ToolResult {
+        call_id: "discovery".to_owned(),
+        operation_id: operation_id.to_owned(),
+        result: json!({"tools": [{"name": "app_github_read_pull_request"}]}),
+        is_error: false,
+    };
+    bridge.apply_result(result.clone()).unwrap();
+    assert_eq!(bridge.pending_calls().count(), 0);
+    assert_eq!(
+        bridge
+            .replay_result("discovery", operation_id, &input)
+            .unwrap(),
+        Some(result)
+    );
+    assert!(bridge
+        .begin_call("discovery".to_owned(), operation_id.to_owned(), input)
+        .is_err());
+}
+
+#[test]
+fn arbitrary_assigned_tool_gets_schema_guidance_only_when_authorized() {
+    let operation_id = "app_custom_connection_search";
+    let schema = json!({
+        "type": "object", "required": ["query"],
+        "properties": {"query": {"type": "string"}},
+        "additionalProperties": false
+    });
+    let mut bridge = authorized_bridge(operation_id, schema);
+    let error = bridge
+        .begin_call("custom".to_owned(), operation_id.to_owned(), json!({}))
+        .unwrap_err();
+    let result = invalid_tool_call_result("custom".to_owned(), operation_id.to_owned(), &error);
+    assert!(result.result["error"]["message"]
+        .as_str()
+        .unwrap()
+        .contains("/required (missing \"query\")"));
+
+    let denied = bridge
+        .begin_call(
+            "denied".to_owned(),
+            "app_unassigned".to_owned(),
+            json!({"PRIVATE_FIELD": "PRIVATE_VALUE"}),
+        )
+        .unwrap_err();
+    assert_eq!(
+        invalid_tool_call_result("denied".to_owned(), "app_unassigned".to_owned(), &denied).result
+            ["error"]["message"],
+        GENERIC_INVALID_TOOL_CALL_MESSAGE
+    );
+    assert!(!bridge.has_call_receipt("denied"));
+}
@@ -19,6 +19,9 @@ use paperclip_runner_core::provider_bridge::{
 use paperclip_runner_core::provider_events::normalize_codex_notification;
 use serde_json::{json, Value};

+#[path = "support/codex_assigned_tool_validation.rs"]
+mod assigned_tool_validation;
+
 static NEXT_DIRECTORY: AtomicU64 = AtomicU64::new(1);
 static RECEIPT_LIMIT_TEST_LOCK: Mutex<()> = Mutex::new(());

@@ -0,0 +1,83 @@
+use super::*;
+
+#[test]
+fn assigned_tool_discovery_repairs_invalid_arguments_through_codex_transport() {
+    let directory = temporary_directory("assigned-discovery-repair");
+    let config = provider_config(&directory, &["--repair-assigned-discovery"]);
+    let operations = vec![AuthorizedTool {
+        operation_id: "paperclip_search_assigned_tools".to_owned(),
+        version: 1,
+        description: "Search assigned tools.".to_owned(),
+        input_schema: json!({"type":"object", "required":["query"],
+            "properties":{"query":{"type":"string"}}, "additionalProperties":false}),
+        response_schema: json!({"type":"object"}),
+    }];
+    let tool_set = 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":config, "authorizedTools":tool_set
+            }),
+        ))
+        .unwrap();
+    executor
+        .execute(&command("open", 2, "session.open", json!({})))
+        .unwrap();
+    executor
+        .execute(&command(
+            "turn",
+            3,
+            "turn.start",
+            json!({"text":"Discover the assigned GitHub tools."}),
+        ))
+        .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!(event.payload["paperclipExecuted"], false);
+                    assert!(!event.payload.to_string().contains("PRIVATE_QUERY"));
+                }
+                "semantic_tool.input" => {
+                    dispatched += 1;
+                    assert_eq!(denied, 1);
+                    assert_eq!(
+                        event.payload["semantic_tool"]["input"],
+                        json!({"query":"github"})
+                    );
+                    executor.execute(&command("result", 4, "semantic_tool.result", json!({
+                        "callId":"assigned-discovery", "operationId":"paperclip_search_assigned_tools",
+                        "result":{"tools":[{"name":"app_github_read_pull_request"}]}, "isError":false
+                    }))).unwrap();
+                }
+                "turn.completed" => completed = true,
+                "turn.failed" => panic!("provider could not repair discovery: {}", 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, "discovery:repair-guidance"), 1);
+    assert_eq!(call_count(&directory, "discovery:success"), 1);
+    fs::remove_dir_all(directory).unwrap();
+}