Respect blocking PostToolUse hooks in code mode (#28365)

## Summary Make blocking hook behavior reliable for tools invoked from code mode. Previously, a `PostToolUse` hook could block a completed tool result, but code mode would still return the original typed result to JavaScript. The hook appeared blocked in hook telemetry while the running script continued with the result. This change: - rejects the nested JavaScript tool promise when `PostToolUse` blocks - normalizes `decision: "block"` and exit code 2 to the same blocking behavior - surfaces the hook feedback as the rejected promise's error - adds end-to-end coverage for the relevant PreToolUse and PostToolUse interactions ## Hook semantics in code mode | Hook behavior | Code-mode result | |---|---| | PreToolUse block | Reject the promise before the tool executes | | PreToolUse `updatedInput` | Execute the rewritten invocation and return its result | | PostToolUse `decision: "block"` | Execute the tool, then reject the promise with the hook reason | | PostToolUse exit code 2 | Same behavior as `decision: "block"` | | PostToolUse `continue: false` | Preserve the existing feedback-only behavior; do not reject the promise | ## Test coverage Added or strengthened end-to-end coverage proving that: - a PreToolUse block rejects the JavaScript promise before execution - a PreToolUse input rewrite executes only the rewritten command - JavaScript receives the rewritten command's result - PostToolUse `decision: "block"` rejects after the command executes - PostToolUse exit code 2 has the same behavior - the hook observes the original completed tool response - the blocked original result does not reach JavaScript - existing direct-mode replacement behavior remains intact - `continue: false` without a reason produces deterministic fallback feedback
2026-07-01 00:31:56 +08:00 · 2026-06-15 15:12:26 -07:00
parent 709f19e111
commit d7f298fe20
3 changed files with 284 additions and 72 deletions
@@ -594,7 +594,6 @@ impl ToolRegistry {
        } else {
            None
        };
-
        if let Some(outcome) = &post_tool_use_outcome {
            record_additional_contexts(
                &invocation.session,
@@ -602,32 +601,9 @@ impl ToolRegistry {
                outcome.additional_contexts.clone(),
            )
            .await;
-            let replacement_text = if outcome.should_stop {
-                Some(
-                    outcome
-                        .feedback_message
-                        .clone()
-                        .or_else(|| outcome.stop_reason.clone())
-                        .unwrap_or_else(|| "PostToolUse hook stopped execution".to_string()),
-                )
-            } else {
-                outcome.feedback_message.clone()
-            };
-            if let Some(replacement_text) = replacement_text {
-                let mut guard = response_cell.lock().await;
-                if let Some(mut result) = guard.take() {
-                    result.result = Box::new(PostToolUseFeedbackOutput {
-                        original: result.result,
-                        model_visible: FunctionToolOutput::from_text(
-                            replacement_text,
-                            /*success*/ None,
-                        ),
-                    });
-                    *guard = Some(result);
-                }
-            }
        }

+        // A PostToolUse block rejects the result, not the already-completed tool execution.
        let lifecycle_outcome = match &result {
            Ok(_) => {
                let guard = response_cell.lock().await;
@@ -654,9 +630,28 @@ impl ToolRegistry {
        match result {
            Ok(_) => {
                let mut guard = response_cell.lock().await;
-                let result = guard.take().ok_or_else(|| {
+                let mut result = guard.take().ok_or_else(|| {
                    FunctionCallError::Fatal("tool produced no output".to_string())
                })?;
+                if let Some(outcome) = post_tool_use_outcome {
+                    if outcome.should_block {
+                        let message = outcome.feedback_message.unwrap_or_else(|| {
+                            "PostToolUse hook blocked the tool result".to_string()
+                        });
+                        let err = FunctionCallError::RespondToModel(message);
+                        dispatch_trace.record_failed(&err);
+                        return Err(err);
+                    }
+                    if let Some(feedback_message) = outcome.feedback_message {
+                        result.result = Box::new(PostToolUseFeedbackOutput {
+                            original: result.result,
+                            model_visible: FunctionToolOutput::from_text(
+                                feedback_message,
+                                /*success*/ None,
+                            ),
+                        });
+                    }
+                }
                dispatch_trace.record_completed(
                    &invocation,
                    &result.call_id,
@@ -77,6 +77,23 @@ fn network_workspace_write_profile() -> PermissionProfile {
    )
 }

+fn code_mode_custom_tool_output_text(output_item: &Value) -> String {
+    match output_item.get("output") {
+        Some(Value::String(text)) => text.clone(),
+        Some(Value::Array(items)) => items
+            .iter()
+            .filter_map(|item| item.get("text").and_then(Value::as_str))
+            .collect::<Vec<_>>()
+            .join("\n"),
+        Some(Value::Object(output)) => output
+            .get("content")
+            .and_then(Value::as_str)
+            .unwrap_or_default()
+            .to_string(),
+        output => panic!("unexpected code mode custom tool output: {output:?}"),
+    }
+}
+
 fn non_openai_model_provider(server: &wiremock::MockServer) -> ModelProviderInfo {
    let mut provider =
        built_in_model_providers(/* openai_base_url */ /*openai_base_url*/ None)["openai"].clone();
@@ -2738,10 +2755,17 @@ async fn pre_tool_use_rewrites_code_mode_nested_exec_command_before_execution()

    let server = start_mock_server().await;
    let call_id = "pretooluse-code-mode-rewrite";
-    let original_marker = std::env::temp_dir().join("pretooluse-code-mode-original-marker");
-    let rewritten_marker = std::env::temp_dir().join("pretooluse-code-mode-rewritten-marker");
-    let original_command = format!("printf original > {}", original_marker.display());
-    let rewritten_command = format!("printf rewritten > {}", rewritten_marker.display());
+    let marker_dir = TempDir::new().context("create pre tool rewrite marker directory")?;
+    let original_marker = marker_dir.path().join("original");
+    let rewritten_marker = marker_dir.path().join("rewritten");
+    let original_command = format!(
+        "printf original > {}; printf original-result",
+        original_marker.display()
+    );
+    let rewritten_command = format!(
+        "printf rewritten > {}; printf rewritten-result",
+        rewritten_marker.display()
+    );
    let original_command_json =
        serde_json::to_string(&original_command).context("serialize original command")?;
    let code = format!(
@@ -2781,13 +2805,6 @@ text(output.output);
        });
    let test = builder.build(&server).await?;

-    if original_marker.exists() {
-        fs::remove_file(&original_marker).context("remove stale original pre tool marker")?;
-    }
-    if rewritten_marker.exists() {
-        fs::remove_file(&rewritten_marker).context("remove stale rewritten pre tool marker")?;
-    }
-
    test.submit_turn_with_permission_profile(
        "run the rewritten shell command from code mode",
        PermissionProfile::Disabled,
@@ -2796,7 +2813,16 @@ text(output.output);

    let requests = responses.requests();
    assert_eq!(requests.len(), 2);
-    requests[1].custom_tool_call_output(call_id);
+    let output_item = requests[1].custom_tool_call_output(call_id);
+    let output = code_mode_custom_tool_output_text(&output_item);
+    assert!(
+        output.contains("rewritten-result"),
+        "code mode should receive the rewritten command result"
+    );
+    assert!(
+        !output.contains("original-result"),
+        "code mode should not receive the original command result"
+    );
    assert!(
        !original_marker.exists(),
        "original nested shell command should not execute after rewrite"
@@ -2814,6 +2840,186 @@ text(output.output);
    Ok(())
 }

+#[tokio::test]
+async fn pre_tool_use_block_rejects_code_mode_tool_promise_before_execution() -> Result<()> {
+    skip_if_no_network!(Ok(()));
+
+    let server = start_mock_server().await;
+    let call_id = "pretooluse-code-mode-block";
+    let marker_dir = TempDir::new().context("create pre tool block marker directory")?;
+    let marker = marker_dir.path().join("blocked");
+    let command = format!("printf blocked > {}", marker.display());
+    let command_json = serde_json::to_string(&command).context("serialize blocked command")?;
+    let code = format!(
+        r#"
+try {{
+  const result = await tools.exec_command({{ cmd: {command_json} }});
+  text(JSON.stringify({{ kind: "unexpected-success", result }}));
+}} catch (error) {{
+  text(JSON.stringify({{ kind: "caught", error: String(error) }}));
+}}
+"#
+    );
+    let responses = mount_sse_sequence(
+        &server,
+        vec![
+            sse(vec![
+                ev_response_created("resp-1"),
+                ev_custom_tool_call(call_id, "exec", &code),
+                ev_completed("resp-1"),
+            ]),
+            sse(vec![
+                ev_response_created("resp-2"),
+                ev_assistant_message("msg-1", "pre hook block observed"),
+                ev_completed("resp-2"),
+            ]),
+        ],
+    )
+    .await;
+
+    let reason = "blocked nested command";
+    let mut builder = test_codex()
+        .with_model("test-gpt-5.1-codex")
+        .with_pre_build_hook(move |home| {
+            if let Err(error) = write_pre_tool_use_hook(home, Some("^Bash$"), "json_deny", reason) {
+                panic!("failed to write blocking pre tool use hook fixture: {error}");
+            }
+        })
+        .with_config(|config| {
+            let _ = config.features.enable(Feature::CodeMode);
+            trust_discovered_hooks(config);
+        });
+    let test = builder.build(&server).await?;
+
+    test.submit_turn_with_permission_profile(
+        "run the blocked shell command from code mode",
+        PermissionProfile::Disabled,
+    )
+    .await?;
+
+    let requests = responses.requests();
+    assert_eq!(requests.len(), 2);
+    let output_item = requests[1].custom_tool_call_output(call_id);
+    let output = code_mode_custom_tool_output_text(&output_item);
+    assert!(output.contains(r#""kind":"caught""#));
+    assert!(output.contains(reason));
+    assert!(!output.contains("unexpected-success"));
+    assert!(
+        !marker.exists(),
+        "PreToolUse-blocked nested command should not execute"
+    );
+
+    let hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?;
+    assert_eq!(hook_inputs.len(), 1);
+    assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
+
+    Ok(())
+}
+
+async fn assert_post_tool_use_blocks_code_mode_tool_result(
+    hook_mode: &'static str,
+    reason: &'static str,
+) -> Result<()> {
+    skip_if_no_network!(Ok(()));
+
+    let server = start_mock_server().await;
+    let call_id = format!("posttooluse-code-mode-{hook_mode}");
+    let marker_dir = TempDir::new().context("create post tool block marker directory")?;
+    let marker = marker_dir.path().join(hook_mode);
+    let command = format!(
+        "printf executed > {}; printf original-post-tool-result",
+        marker.display()
+    );
+    let command_json = serde_json::to_string(&command).context("serialize post hook command")?;
+    let code = format!(
+        r#"
+try {{
+  const result = await tools.exec_command({{ cmd: {command_json} }});
+  text(JSON.stringify({{ kind: "unexpected-success", result }}));
+}} catch (error) {{
+  text(JSON.stringify({{ kind: "caught", error: String(error) }}));
+}}
+"#
+    );
+    let responses = mount_sse_sequence(
+        &server,
+        vec![
+            sse(vec![
+                ev_response_created("resp-1"),
+                ev_custom_tool_call(&call_id, "exec", &code),
+                ev_completed("resp-1"),
+            ]),
+            sse(vec![
+                ev_response_created("resp-2"),
+                ev_assistant_message("msg-1", "post hook block observed"),
+                ev_completed("resp-2"),
+            ]),
+        ],
+    )
+    .await;
+
+    let mut builder = test_codex()
+        .with_model("test-gpt-5.1-codex")
+        .with_pre_build_hook(move |home| {
+            if let Err(error) = write_post_tool_use_hook(home, Some("^Bash$"), hook_mode, reason) {
+                panic!("failed to write blocking post tool use hook fixture: {error}");
+            }
+        })
+        .with_config(|config| {
+            let _ = config.features.enable(Feature::CodeMode);
+            trust_discovered_hooks(config);
+        });
+    let test = builder.build(&server).await?;
+
+    test.submit_turn_with_permission_profile(
+        "run the shell command blocked after execution from code mode",
+        PermissionProfile::Disabled,
+    )
+    .await?;
+
+    let requests = responses.requests();
+    assert_eq!(requests.len(), 2);
+    let output_item = requests[1].custom_tool_call_output(&call_id);
+    let output = code_mode_custom_tool_output_text(&output_item);
+    assert!(output.contains(r#""kind":"caught""#));
+    assert!(output.contains(reason));
+    assert!(!output.contains("unexpected-success"));
+    assert!(
+        !output.contains("original-post-tool-result"),
+        "blocked post tool result should not reach code mode"
+    );
+    assert_eq!(
+        fs::read_to_string(&marker).context("read blocking post tool marker")?,
+        "executed",
+        "PostToolUse should run after the nested command executes"
+    );
+
+    let hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?;
+    assert_eq!(hook_inputs.len(), 1);
+    assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
+    assert_eq!(
+        hook_inputs[0]["tool_response"],
+        Value::String("original-post-tool-result".to_string())
+    );
+
+    Ok(())
+}
+
+#[tokio::test]
+async fn post_tool_use_block_decision_rejects_code_mode_tool_promise() -> Result<()> {
+    assert_post_tool_use_blocks_code_mode_tool_result(
+        "decision_block",
+        "blocked nested result by decision",
+    )
+    .await
+}
+
+#[tokio::test]
+async fn post_tool_use_exit_two_rejects_code_mode_tool_promise() -> Result<()> {
+    assert_post_tool_use_blocks_code_mode_tool_result("exit_2", "blocked nested result by exit two")
+        .await
+}
+
 #[tokio::test]
 async fn plugin_pre_tool_use_blocks_shell_command_before_execution() -> Result<()> {
    skip_if_no_network!(Ok(()));
@@ -38,16 +38,14 @@ pub struct PostToolUseRequest {
 #[derive(Debug)]
 pub struct PostToolUseOutcome {
    pub hook_events: Vec<HookCompletedEvent>,
-    pub should_stop: bool,
-    pub stop_reason: Option<String>,
+    pub should_block: bool,
    pub additional_contexts: Vec<String>,
    pub feedback_message: Option<String>,
 }

 #[derive(Debug, Default, PartialEq, Eq)]
 struct PostToolUseHandlerData {
-    should_stop: bool,
-    stop_reason: Option<String>,
+    should_block: bool,
    additional_contexts_for_model: Vec<String>,
    feedback_messages_for_model: Vec<String>,
 }
@@ -83,8 +81,7 @@ pub(crate) async fn run(
    if matched.is_empty() {
        return PostToolUseOutcome {
            hook_events: Vec::new(),
-            should_stop: false,
-            stop_reason: None,
+            should_block: false,
            additional_contexts: Vec::new(),
            feedback_message: None,
        };
@@ -118,10 +115,7 @@ pub(crate) async fn run(
            .iter()
            .map(|result| result.data.additional_contexts_for_model.as_slice()),
    );
-    let should_stop = results.iter().any(|result| result.data.should_stop);
-    let stop_reason = results
-        .iter()
-        .find_map(|result| result.data.stop_reason.clone());
+    let should_block = results.iter().any(|result| result.data.should_block);
    let feedback_message = common::join_text_chunks(
        results
            .iter()
@@ -136,8 +130,7 @@ pub(crate) async fn run(
                common::hook_completed_for_tool_use(result.completed, &request.tool_use_id)
            })
            .collect(),
-        should_stop,
-        stop_reason,
+        should_block,
        additional_contexts,
        feedback_message,
    }
@@ -175,8 +168,7 @@ fn parse_completed(
 ) -> dispatcher::ParsedHandler<PostToolUseHandlerData> {
    let mut entries = Vec::new();
    let mut status = HookRunStatus::Completed;
-    let mut should_stop = false;
-    let mut stop_reason = None;
+    let mut should_block = false;
    let mut additional_contexts_for_model = Vec::new();
    let mut feedback_messages_for_model = Vec::new();

@@ -212,8 +204,6 @@ fn parse_completed(
                    }
                    if !parsed.universal.continue_processing {
                        status = HookRunStatus::Stopped;
-                        should_stop = true;
-                        stop_reason = parsed.universal.stop_reason.clone();
                        let stop_text = parsed
                            .universal
                            .stop_reason
@@ -242,6 +232,7 @@ fn parse_completed(
                        });
                    } else if parsed.should_block {
                        status = HookRunStatus::Blocked;
+                        should_block = true;
                        if let Some(reason) = parsed.reason {
                            entries.push(HookOutputEntry {
                                kind: HookOutputEntryKind::Feedback,
@@ -260,6 +251,8 @@ fn parse_completed(
            }
            Some(2) => {
                if let Some(reason) = common::trimmed_non_empty(&run_result.stderr) {
+                    status = HookRunStatus::Blocked;
+                    should_block = true;
                    entries.push(HookOutputEntry {
                        kind: HookOutputEntryKind::Feedback,
                        text: reason.clone(),
@@ -298,8 +291,7 @@ fn parse_completed(
    dispatcher::ParsedHandler {
        completed,
        data: PostToolUseHandlerData {
-            should_stop,
-            stop_reason,
+            should_block,
            additional_contexts_for_model,
            feedback_messages_for_model,
        },
@@ -310,8 +302,7 @@ fn parse_completed(
 fn serialization_failure_outcome(hook_events: Vec<HookCompletedEvent>) -> PostToolUseOutcome {
    PostToolUseOutcome {
        hook_events,
-        should_stop: false,
-        stop_reason: None,
+        should_block: false,
        additional_contexts: Vec::new(),
        feedback_message: None,
    }
@@ -364,8 +355,7 @@ mod tests {
        assert_eq!(
            parsed.data,
            PostToolUseHandlerData {
-                should_stop: false,
-                stop_reason: None,
+                should_block: true,
                additional_contexts_for_model: Vec::new(),
                feedback_messages_for_model: vec!["bash output looked sketchy".to_string()],
            }
@@ -388,8 +378,7 @@ mod tests {
        assert_eq!(
            parsed.data,
            PostToolUseHandlerData {
-                should_stop: false,
-                stop_reason: None,
+                should_block: false,
                additional_contexts_for_model: vec!["Remember the bash cleanup note.".to_string()],
                feedback_messages_for_model: Vec::new(),
            }
@@ -418,8 +407,7 @@ mod tests {
        assert_eq!(
            parsed.data,
            PostToolUseHandlerData {
-                should_stop: false,
-                stop_reason: None,
+                should_block: false,
                additional_contexts_for_model: Vec::new(),
                feedback_messages_for_model: Vec::new(),
            }
@@ -435,7 +423,7 @@ mod tests {
    }

    #[test]
-    fn exit_two_surfaces_feedback_to_model_without_blocking() {
+    fn exit_two_blocks_with_feedback() {
        let parsed = parse_completed(
            &handler(),
            run_result(Some(2), "", "post hook says pause"),
@@ -445,13 +433,12 @@ mod tests {
        assert_eq!(
            parsed.data,
            PostToolUseHandlerData {
-                should_stop: false,
-                stop_reason: None,
+                should_block: true,
                additional_contexts_for_model: Vec::new(),
                feedback_messages_for_model: vec!["post hook says pause".to_string()],
            }
        );
-        assert_eq!(parsed.completed.run.status, HookRunStatus::Completed);
+        assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked);
    }

    #[test]
@@ -469,8 +456,7 @@ mod tests {
        assert_eq!(
            parsed.data,
            PostToolUseHandlerData {
-                should_stop: true,
-                stop_reason: Some("halt after bash output".to_string()),
+                should_block: false,
                additional_contexts_for_model: Vec::new(),
                feedback_messages_for_model: vec!["post-tool hook says stop".to_string()],
            }
@@ -485,6 +471,32 @@ mod tests {
        );
    }

+    #[test]
+    fn continue_false_without_reason_synthesizes_feedback() {
+        let parsed = parse_completed(
+            &handler(),
+            run_result(Some(0), r#"{"continue":false}"#, ""),
+            Some("turn-1".to_string()),
+        );
+
+        assert_eq!(
+            parsed.data,
+            PostToolUseHandlerData {
+                should_block: false,
+                additional_contexts_for_model: Vec::new(),
+                feedback_messages_for_model: vec!["PostToolUse hook stopped execution".to_string()],
+            }
+        );
+        assert_eq!(parsed.completed.run.status, HookRunStatus::Stopped);
+        assert_eq!(
+            parsed.completed.run.entries,
+            vec![HookOutputEntry {
+                kind: HookOutputEntryKind::Stop,
+                text: "PostToolUse hook stopped execution".to_string(),
+            }]
+        );
+    }
+
    #[test]
    fn plain_stdout_is_ignored_for_post_tool_use() {
        let parsed = parse_completed(
@@ -496,8 +508,7 @@ mod tests {
        assert_eq!(
            parsed.data,
            PostToolUseHandlerData {
-                should_stop: false,
-                stop_reason: None,
+                should_block: false,
                additional_contexts_for_model: Vec::new(),
                feedback_messages_for_model: Vec::new(),
            }