openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-29 19:08:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Support continue/stopReason universal fields

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-04-14 17:43:38 -07:00
parent 6cb8d62b37
commit 4557ab2d23
16 changed files with 464 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+52 -8
View File
@@ -315,6 +315,7 @@ use crate::tools::js_repl::resolve_compatible_node;
use crate::tools::network_approval::NetworkApprovalService;
use crate::tools::network_approval::build_blocked_request_observer;
use crate::tools::network_approval::build_network_policy_decider;
use crate::tools::parallel::ToolCallCompletion;
use crate::tools::parallel::ToolCallRuntime;
use crate::tools::router::ToolRouterParams;
use crate::tools::sandboxing::ApprovalStore;
@@ -6439,10 +6440,12 @@ pub(crate) async fn run_turn(
let SamplingRequestResult {
needs_follow_up: model_needs_follow_up,
last_agent_message: sampling_request_last_agent_message,
stop_requested,
} = sampling_request_output;
can_drain_pending_input = true;
let has_pending_input = sess.has_pending_input().await;
let needs_follow_up = model_needs_follow_up || has_pending_input;
let needs_follow_up =
!stop_requested && (model_needs_follow_up || has_pending_input);
let total_usage_tokens = sess.get_total_token_usage().await;
let token_limit_reached = total_usage_tokens >= auto_compact_limit;
@@ -6456,6 +6459,7 @@ pub(crate) async fn run_turn(
auto_compact_limit,
token_limit_reached,
model_needs_follow_up,
stop_requested,
has_pending_input,
needs_follow_up,
"post sampling token usage"
@@ -7221,6 +7225,7 @@ pub(crate) async fn built_tools(
struct SamplingRequestResult {
needs_follow_up: bool,
last_agent_message: Option<String>,
stop_requested: bool,
}
/// Ephemeral per-response state for streaming a single proposed plan.
@@ -7743,22 +7748,26 @@ async fn handle_assistant_item_done_in_plan_mode(
}
async fn drain_in_flight(
in_flight: &mut FuturesOrdered<BoxFuture<'static, CodexResult<ResponseInputItem>>>,
in_flight: &mut FuturesOrdered<BoxFuture<'static, CodexResult<ToolCallCompletion>>>,
sess: Arc<Session>,
turn_context: Arc<TurnContext>,
) -> CodexResult<()> {
) -> CodexResult<bool> {
while let Some(res) = in_flight.next().await {
match res {
Ok(response_input) => {
Ok(ToolCallCompletion::Response(response_input)) => {
sess.record_conversation_items(&turn_context, &[response_input.into()])
.await;
}
Ok(ToolCallCompletion::StopTurn { reason }) => {
debug!(?reason, "tool requested turn stop");
return Ok(true);
}
Err(err) => {
error_or_panic(format!("in-flight tool future failed during drain: {err}"));
}
}
}
Ok(())
Ok(false)
}
#[allow(clippy::too_many_arguments)]
@@ -7801,7 +7810,7 @@ async fn try_run_sampling_request(
.instrument(trace_span!("stream_request"))
.or_cancel(&cancellation_token)
.await??;
let mut in_flight: FuturesOrdered<BoxFuture<'static, CodexResult<ResponseInputItem>>> =
let mut in_flight: FuturesOrdered<BoxFuture<'static, CodexResult<ToolCallCompletion>>> =
FuturesOrdered::new();
let mut needs_follow_up = false;
let mut last_agent_message: Option<String> = None;
@@ -7884,6 +7893,25 @@ async fn try_run_sampling_request(
cancellation_token: cancellation_token.child_token(),
};
let preempt_for_mailbox_mail = match &item {
ResponseItem::Message { role, phase, .. } => {
role == "assistant" && matches!(phase, Some(MessagePhase::Commentary))
}
ResponseItem::Reasoning { .. } => true,
ResponseItem::LocalShellCall { .. }
| ResponseItem::FunctionCall { .. }
| ResponseItem::ToolSearchCall { .. }
| ResponseItem::FunctionCallOutput { .. }
| ResponseItem::CustomToolCall { .. }
| ResponseItem::CustomToolCallOutput { .. }
| ResponseItem::ToolSearchOutput { .. }
| ResponseItem::WebSearchCall { .. }
| ResponseItem::ImageGenerationCall { .. }
| ResponseItem::GhostSnapshot { .. }
| ResponseItem::Compaction { .. }
| ResponseItem::Other => false,
};
let output_result =
match handle_output_item_done(&mut ctx, item, previously_active_item)
.instrument(handle_responses)
@@ -7899,6 +7927,14 @@ async fn try_run_sampling_request(
last_agent_message = Some(agent_message);
}
needs_follow_up |= output_result.needs_follow_up;
// todo: remove before stabilizing multi-agent v2
if preempt_for_mailbox_mail && sess.mailbox_rx.lock().await.has_pending() {
break Ok(SamplingRequestResult {
needs_follow_up: true,
last_agent_message,
stop_requested: false,
});
}
}
ResponseEvent::OutputItemAdded(item) => {
if let Some(turn_item) = handle_non_tool_response_item(
@@ -7997,6 +8033,7 @@ async fn try_run_sampling_request(
break Ok(SamplingRequestResult {
needs_follow_up,
last_agent_message,
stop_requested: false,
});
}
ResponseEvent::OutputTextDelta(delta) => {
@@ -8087,7 +8124,8 @@ async fn try_run_sampling_request(
)
.await;
drain_in_flight(&mut in_flight, sess.clone(), turn_context.clone()).await?;
let stop_requested =
drain_in_flight(&mut in_flight, sess.clone(), turn_context.clone()).await?;
if cancellation_token.is_cancelled() {
return Err(CodexErr::TurnAborted);
@@ -8104,7 +8142,13 @@ async fn try_run_sampling_request(
}
}
outcome
outcome.map(|mut outcome| {
if stop_requested {
outcome.needs_follow_up = false;
outcome.stop_requested = true;
}
outcome
})
}
pub(super) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) -> Option<String> {
codex-rs/core/src/function_tool.rs
+2
View File
@@ -4,6 +4,8 @@ use thiserror::Error;
pub enum FunctionCallError {
#[error("{0}")]
RespondToModel(String),
#[error("stop turn")]
StopTurn(Option<String>),
#[error("LocalShellCall without call_id or id")]
MissingLocalShellCallId,
#[error("Fatal error: {0}")]
codex-rs/core/src/hook_runtime.rs
+4 -8
View File
@@ -1,7 +1,6 @@
use std::future::Future;
use std::sync::Arc;
use codex_hooks::PermissionRequestDecision;
use codex_hooks::PermissionRequestOutcome;
use codex_hooks::PermissionRequestRequest;
use codex_hooks::PostToolUseOutcome;
@@ -157,7 +156,7 @@ pub(crate) async fn run_permission_request_hooks(
turn_context: &Arc<TurnContext>,
run_id_suffix: &str,
payload: PermissionRequestPayload,
) -> Option<PermissionRequestDecision> {
) -> PermissionRequestOutcome {
let request = PermissionRequestRequest {
session_id: sess.conversation_id,
turn_id: turn_context.sub_id.clone(),
@@ -173,13 +172,10 @@ pub(crate) async fn run_permission_request_hooks(
let preview_runs = sess.hooks().preview_permission_request(&request);
emit_hook_started_events(sess, turn_context, preview_runs).await;
let PermissionRequestOutcome {
hook_events,
decision,
} = sess.hooks().run_permission_request(request).await;
emit_hook_completed_events(sess, turn_context, hook_events).await;
let outcome = sess.hooks().run_permission_request(request).await;
emit_hook_completed_events(sess, turn_context, outcome.hook_events.clone()).await;
decision
outcome
}
pub(crate) async fn run_post_tool_use_hooks(
codex-rs/core/src/stream_events_utils.rs
+7 -1
View File
@@ -14,6 +14,7 @@ use crate::function_tool::FunctionCallError;
use crate::memories::citations::get_thread_id_from_citations;
use crate::memories::citations::parse_memory_citation;
use crate::parse_turn_item;
use crate::tools::parallel::ToolCallCompletion;
use crate::tools::parallel::ToolCallRuntime;
use crate::tools::router::ToolRouter;
use codex_protocol::error::CodexErr;
@@ -184,7 +185,7 @@ async fn record_stage1_output_usage_for_completed_item(
/// queuing any tool execution futures. This records items immediately so
/// history and rollout stay in sync even if the turn is later cancelled.
pub(crate) type InFlightFuture<'f> =
Pin<Box<dyn Future<Output = Result<ResponseInputItem>> + Send + 'f>>;
Pin<Box<dyn Future<Output = Result<ToolCallCompletion>> + Send + 'f>>;
#[derive(Default)]
pub(crate) struct OutputItemResult {
@@ -320,6 +321,11 @@ pub(crate) async fn handle_output_item_done(
output.needs_follow_up = true;
}
Err(FunctionCallError::StopTurn(_)) => {
record_completed_response_item(ctx.sess.as_ref(), ctx.turn_context.as_ref(), &item)
.await;
output.needs_follow_up = true;
}
// A fatal error occurred; surface it back into history.
Err(FunctionCallError::Fatal(message)) => {
return Err(CodexErr::Fatal(message));
codex-rs/core/src/tools/events.rs
+8
View File
@@ -353,6 +353,14 @@ impl ToolEmitter {
let result = Err(FunctionCallError::RespondToModel(normalized));
(event, result)
}
Err(ToolError::StopTurn(reason)) => {
let message = reason
.clone()
.unwrap_or_else(|| "PermissionRequest hook stopped execution".to_string());
let event = ToolEventStage::Failure(ToolEventFailure::Rejected(message));
let result = Err(FunctionCallError::StopTurn(reason));
(event, result)
}
};
self.emit(ctx, event).await;
result
codex-rs/core/src/tools/handlers/unified_exec.rs
+11 -5
View File
@@ -309,7 +309,7 @@ impl ToolHandler for UnifiedExecHandler {
}
emit_unified_exec_tty_metric(&turn.session_telemetry, tty);
manager
match manager
.exec_command(
ExecCommandRequest {
command,
@@ -331,11 +331,17 @@ impl ToolHandler for UnifiedExecHandler {
&context,
)
.await
.map_err(|err| {
FunctionCallError::RespondToModel(format!(
{
Ok(output) => output,
Err(crate::unified_exec::UnifiedExecError::StopTurn(reason)) => {
return Err(FunctionCallError::StopTurn(reason));
}
Err(err) => {
return Err(FunctionCallError::RespondToModel(format!(
"exec_command failed for `{command_for_display}`: {err:?}"
))
})?
)));
}
}
}
"write_stdin" => {
let args: WriteStdinArgs = parse_arguments(&arguments)?;
codex-rs/core/src/tools/network_approval.rs
+18 -3
View File
@@ -118,6 +118,7 @@ enum PendingApprovalDecision {
enum NetworkApprovalOutcome {
DeniedByUser,
DeniedByPolicy(String),
StopTurn(Option<String>),
}
/// Whether an allowlist miss may be reviewed instead of hard-denied.
@@ -381,7 +382,7 @@ impl NetworkApprovalService {
let command = owner_call
.as_ref()
.map_or_else(|| prompt_command.join(" "), |call| call.command.clone());
if let Some(permission_request_decision) = run_permission_request_hooks(
let permission_request_outcome = run_permission_request_hooks(
&session,
&turn_context,
&guardian_approval_id,
@@ -391,8 +392,21 @@ impl NetworkApprovalService {
description: Some(format!("network-access {target}")),
},
)
.await
{
.await;
if permission_request_outcome.should_stop {
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
NetworkApprovalOutcome::StopTurn(permission_request_outcome.stop_reason),
)
.await;
}
pending.set_decision(PendingApprovalDecision::Deny).await;
let mut pending_approvals = self.pending_host_approvals.lock().await;
pending_approvals.remove(&key);
return NetworkDecision::deny(REASON_NOT_ALLOWED);
}
if let Some(permission_request_decision) = permission_request_outcome.decision {
match permission_request_decision {
PermissionRequestDecision::Allow => {
pending
@@ -668,6 +682,7 @@ pub(crate) async fn finish_immediate_network_approval(
Err(ToolError::Rejected("rejected by user".to_string()))
}
Some(NetworkApprovalOutcome::DeniedByPolicy(message)) => Err(ToolError::Rejected(message)),
Some(NetworkApprovalOutcome::StopTurn(reason)) => Err(ToolError::StopTurn(reason)),
None => Ok(()),
}
}
codex-rs/core/src/tools/orchestrator.rs
+6 -3
View File
@@ -369,14 +369,17 @@ impl ToolOrchestrator {
T: ToolRuntime<Rq, Out>,
{
if let Some(permission_request) = tool.permission_request_payload(req) {
match run_permission_request_hooks(
let permission_request_outcome = run_permission_request_hooks(
approval_ctx.session,
approval_ctx.turn,
permission_request_run_id,
permission_request,
)
.await
{
.await;
if permission_request_outcome.should_stop {
return Err(ToolError::StopTurn(permission_request_outcome.stop_reason));
}
match permission_request_outcome.decision {
Some(PermissionRequestDecision::Allow) => {
telemetry.otel.tool_decision(
telemetry.tool_name,
codex-rs/core/src/tools/parallel.rs
+13 -3
View File
@@ -32,6 +32,11 @@ pub(crate) struct ToolCallRuntime {
parallel_execution: Arc<RwLock<()>>,
}
pub(crate) enum ToolCallCompletion {
Response(ResponseInputItem),
StopTurn { reason: Option<String> },
}
impl ToolCallRuntime {
pub(crate) fn new(
router: Arc<ToolRouter>,
@@ -57,15 +62,20 @@ impl ToolCallRuntime {
self,
call: ToolCall,
cancellation_token: CancellationToken,
) -> impl std::future::Future<Output = Result<ResponseInputItem, CodexErr>> {
) -> impl std::future::Future<Output = Result<ToolCallCompletion, CodexErr>> {
let error_call = call.clone();
let future =
self.handle_tool_call_with_source(call, ToolCallSource::Direct, cancellation_token);
async move {
match future.await {
Ok(response) => Ok(response.into_response()),
Ok(response) => Ok(ToolCallCompletion::Response(response.into_response())),
Err(FunctionCallError::StopTurn(reason)) => {
Ok(ToolCallCompletion::StopTurn { reason })
}
Err(FunctionCallError::Fatal(message)) => Err(CodexErr::Fatal(message)),
Err(other) => Ok(Self::failure_response(error_call, other)),
Err(other) => Ok(ToolCallCompletion::Response(Self::failure_response(
error_call, other,
))),
}
}
.in_current_span()
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
+37 -4
View File
@@ -216,7 +216,7 @@ pub(super) async fn try_run_zsh_fork(
let exec_result = escalate_server
.exec(exec_params, cancel_token, Arc::new(command_executor))
.await
.map_err(|err| ToolError::Rejected(err.to_string()))?;
.map_err(map_escalation_error)?;
map_exec_result(attempt.sandbox, exec_result).map(Some)
}
@@ -325,6 +325,13 @@ struct PromptDecision {
decision: ReviewDecision,
guardian_review_id: Option<String>,
rejection_message: Option<String>,
stop_turn: Option<String>,
}
#[derive(Debug, thiserror::Error)]
#[error("stop turn")]
struct StopTurnRequested {
reason: Option<String>,
}
fn execve_prompt_is_rejected_by_policy(
@@ -406,19 +413,28 @@ impl CoreShellActionProvider {
description: None,
};
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
match run_permission_request_hooks(
let permission_request_outcome = run_permission_request_hooks(
&session,
&turn,
&effective_approval_id,
permission_request,
)
.await
{
.await;
if permission_request_outcome.should_stop {
return PromptDecision {
decision: ReviewDecision::Denied,
guardian_review_id: None,
rejection_message: None,
stop_turn: permission_request_outcome.stop_reason,
};
}
match permission_request_outcome.decision {
Some(PermissionRequestDecision::Allow) => {
return PromptDecision {
decision: ReviewDecision::Approved,
guardian_review_id: None,
rejection_message: None,
stop_turn: None,
};
}
Some(PermissionRequestDecision::Deny { message }) => {
@@ -426,6 +442,7 @@ impl CoreShellActionProvider {
decision: ReviewDecision::Denied,
guardian_review_id: None,
rejection_message: Some(message),
stop_turn: None,
};
}
None => {}
@@ -452,6 +469,7 @@ impl CoreShellActionProvider {
decision,
guardian_review_id,
rejection_message: None,
stop_turn: None,
};
}
@@ -474,6 +492,7 @@ impl CoreShellActionProvider {
decision,
guardian_review_id: None,
rejection_message: None,
stop_turn: None,
}
})
.await)
@@ -504,6 +523,12 @@ impl CoreShellActionProvider {
let prompt_decision = self
.prompt(program, argv, workdir, &self.stopwatch, prompt_permissions)
.await?;
if let Some(reason) = prompt_decision.stop_turn {
return Err(StopTurnRequested {
reason: Some(reason),
}
.into());
}
match prompt_decision.decision {
ReviewDecision::Approved
| ReviewDecision::ApprovedForSession
@@ -566,6 +591,14 @@ impl CoreShellActionProvider {
}
}
fn map_escalation_error(err: anyhow::Error) -> ToolError {
if let Some(stop_turn) = err.downcast_ref::<StopTurnRequested>() {
ToolError::StopTurn(stop_turn.reason.clone())
} else {
ToolError::Rejected(err.to_string())
}
}
// Shell-wrapper parsing is weaker than direct exec interception because it can
// only see the script text, not the final resolved executable path. Keep it
// disabled by default so path-sensitive rules rely on the later authoritative
codex-rs/core/src/tools/sandboxing.rs
+1
View File
@@ -321,6 +321,7 @@ pub(crate) struct ToolCtx {
#[derive(Debug)]
pub(crate) enum ToolError {
Rejected(String),
StopTurn(Option<String>),
Codex(CodexErr),
}
codex-rs/core/src/unified_exec/errors.rs
+6
View File
@@ -3,6 +3,8 @@ use thiserror::Error;
#[derive(Debug, Error)]
pub(crate) enum UnifiedExecError {
#[error("stop turn")]
StopTurn(Option<String>),
#[error("Failed to create unified exec process: {message}")]
CreateProcess { message: String },
#[error("Unified exec process failed: {message}")]
@@ -26,6 +28,10 @@ pub(crate) enum UnifiedExecError {
}
impl UnifiedExecError {
pub(crate) fn stop_turn(reason: Option<String>) -> Self {
Self::StopTurn(reason)
}
pub(crate) fn create_process(message: String) -> Self {
Self::CreateProcess { message }
}
codex-rs/core/src/unified_exec/process_manager.rs
+6 -1
View File
@@ -778,7 +778,12 @@ impl UnifiedExecProcessManager {
)
.await
.map(|result| (result.output, result.deferred_network_approval))
.map_err(|e| UnifiedExecError::create_process(format!("{e:?}")))
.map_err(|err| match err {
crate::tools::sandboxing::ToolError::StopTurn(reason) => {
UnifiedExecError::stop_turn(reason)
}
other => UnifiedExecError::create_process(format!("{other:?}")),
})
}
pub(super) async fn collect_output_until_deadline(
codex-rs/core/tests/suite/hooks.rs
+170
View File
@@ -294,6 +294,11 @@ elif mode == "deny":
}}
}}
}}))
elif mode == "continue_false":
print(json.dumps({{
"continue": False,
"stopReason": reason
}}))
elif mode == "exit_2":
sys.stderr.write(reason + "\n")
raise SystemExit(2)
@@ -1221,6 +1226,81 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() ->
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn permission_request_hook_continue_false_stops_turn_without_tool_output() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "permissionrequest-continue-false";
let marker = std::env::temp_dir().join("permissionrequest-continue-false-marker");
let command = format!("rm -f {}", marker.display());
let stop_reason = "stop after permission request";
let args = serde_json::json!({ "command": command });
let responses = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
ev_completed("resp-1"),
]),
)
.await;
let test = test_codex()
.with_pre_build_hook(|home| {
if let Err(error) = write_permission_request_hook(
home,
Some(PERMISSION_REQUEST_HOOK_MATCHER),
"continue_false",
stop_reason,
) {
panic!("failed to write permission request hook test fixture: {error}");
}
})
.with_config(|config| {
config
.features
.enable(Feature::CodexHooks)
.expect("test config should allow feature update");
})
.build(&server)
.await?;
fs::write(&marker, "seed").context("create permission request continue-false marker")?;
test.submit_turn_with_policies(
"run the shell command after permission request hook",
AskForApproval::OnRequest,
SandboxPolicy::DangerFullAccess,
)
.await?;
assert_eq!(responses.requests().len(), 1);
assert!(
marker.exists(),
"continue:false should stop before the tool executes"
);
assert_single_permission_request_hook_input(
test.codex_home_path(),
&command,
/*description*/ None,
)?;
assert!(
timeout(
Duration::from_secs(2),
wait_for_event(&test.codex, |event| matches!(
event,
EventMsg::ExecApprovalRequest(_)
))
)
.await
.is_err(),
"expected the permission request hook to stop before approval UI appears"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -1302,6 +1382,96 @@ async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn permission_request_hook_continue_false_stops_exec_command_turn() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "permissionrequest-exec-command-continue-false";
let marker = std::env::temp_dir().join("permissionrequest-exec-command-continue-false-marker");
let command = format!("rm -f {}", marker.display());
let stop_reason = "stop after permission request";
let justification = "keep the temporary marker in place";
let args = serde_json::json!({
"cmd": command,
"login": true,
"sandbox_permissions": "require_escalated",
"justification": justification,
});
let responses = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
core_test_support::responses::ev_function_call(
call_id,
"exec_command",
&serde_json::to_string(&args)?,
),
ev_completed("resp-1"),
]),
)
.await;
let test = test_codex()
.with_pre_build_hook(|home| {
if let Err(error) = write_permission_request_hook(
home,
Some(PERMISSION_REQUEST_HOOK_MATCHER),
"continue_false",
stop_reason,
) {
panic!("failed to write permission request hook test fixture: {error}");
}
})
.with_config(|config| {
config.use_experimental_unified_exec_tool = true;
config
.features
.enable(Feature::CodexHooks)
.expect("test config should allow feature update");
config
.features
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
})
.build(&server)
.await?;
fs::write(&marker, "seed").context("create exec command continue-false marker")?;
test.submit_turn_with_policies(
"run the exec command after permission request hook",
AskForApproval::OnRequest,
codex_protocol::protocol::SandboxPolicy::new_read_only_policy(),
)
.await?;
assert_eq!(responses.requests().len(), 1);
assert!(
marker.exists(),
"continue:false should stop before exec_command executes"
);
assert_single_permission_request_hook_input(
test.codex_home_path(),
&command,
Some(justification),
)?;
assert!(
timeout(
Duration::from_secs(2),
wait_for_event(&test.codex, |event| matches!(
event,
EventMsg::ExecApprovalRequest(_)
))
)
.await
.is_err(),
"expected the permission request hook to stop before approval UI appears"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn permission_request_hook_allows_network_approval_without_prompt() -> Result<()> {
skip_if_no_network!(Ok(()));
codex-rs/hooks/src/engine/output_parser.rs
+27 -5
View File
@@ -275,11 +275,7 @@ fn unsupported_pre_tool_use_universal(universal: &UniversalOutput) -> Option<Str
}
fn unsupported_permission_request_universal(universal: &UniversalOutput) -> Option<String> {
if !universal.continue_processing {
Some("PermissionRequest hook returned unsupported continue:false".to_string())
} else if universal.stop_reason.is_some() {
Some("PermissionRequest hook returned unsupported stopReason".to_string())
} else if universal.suppress_output {
if universal.suppress_output {
Some("PermissionRequest hook returned unsupported suppressOutput".to_string())
} else {
None
@@ -491,4 +487,30 @@ mod tests {
Some("PermissionRequest hook returned unsupported interrupt:true".to_string())
);
}
#[test]
fn permission_request_accepts_continue_false_and_stop_reason() {
let parsed = parse_permission_request(
&json!({
"continue": false,
"stopReason": "stop now",
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": {
"behavior": "allow"
}
}
})
.to_string(),
)
.expect("permission request hook output should parse");
assert_eq!(parsed.invalid_reason, None);
assert!(!parsed.universal.continue_processing);
assert_eq!(parsed.universal.stop_reason.as_deref(), Some("stop now"));
assert_eq!(
parsed.decision,
Some(super::PermissionRequestDecision::Allow)
);
}
}
codex-rs/hooks/src/events/permission_request.rs
+96 -3
View File
@@ -54,11 +54,15 @@ pub enum PermissionRequestDecision {
#[derive(Debug)]
pub struct PermissionRequestOutcome {
pub hook_events: Vec<HookCompletedEvent>,
pub should_stop: bool,
pub stop_reason: Option<String>,
pub decision: Option<PermissionRequestDecision>,
}
#[derive(Debug, Default, PartialEq, Eq)]
struct PermissionRequestHandlerData {
should_stop: bool,
stop_reason: Option<String>,
decision: Option<PermissionRequestDecision>,
}
@@ -94,6 +98,8 @@ pub(crate) async fn run(
if matched.is_empty() {
return PermissionRequestOutcome {
hook_events: Vec::new(),
should_stop: false,
stop_reason: None,
decision: None,
};
}
@@ -109,6 +115,8 @@ pub(crate) async fn run(
);
return PermissionRequestOutcome {
hook_events,
should_stop: false,
stop_reason: None,
decision: None,
};
}
@@ -124,6 +132,10 @@ pub(crate) async fn run(
)
.await;
let should_stop = results.iter().any(|result| result.data.should_stop);
let stop_reason = results
.iter()
.find_map(|result| result.data.stop_reason.clone());
// Preserve the most specific matching allow, but treat any deny as final so
// broader policy layers cannot accidentally overrule a more specific block.
let decision = resolve_permission_request_decision(
@@ -139,7 +151,9 @@ pub(crate) async fn run(
common::hook_completed_for_tool_use(result.completed, &request.run_id_suffix)
})
.collect(),
decision,
should_stop,
stop_reason,
decision: (!should_stop).then_some(decision).flatten(),
}
}
@@ -189,6 +203,8 @@ fn parse_completed(
) -> dispatcher::ParsedHandler<PermissionRequestHandlerData> {
let mut entries = Vec::new();
let mut status = HookRunStatus::Completed;
let mut should_stop = false;
let mut stop_reason = None;
let mut decision = None;
match run_result.error.as_deref() {
@@ -212,7 +228,18 @@ fn parse_completed(
text: system_message,
});
}
if let Some(invalid_reason) = parsed.invalid_reason {
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.unwrap_or_else(|| {
"PermissionRequest hook stopped execution".to_string()
});
entries.push(HookOutputEntry {
kind: HookOutputEntryKind::Stop,
text: stop_text,
});
} else if let Some(invalid_reason) = parsed.invalid_reason {
status = HookRunStatus::Failed;
entries.push(HookOutputEntry {
kind: HookOutputEntryKind::Error,
@@ -281,16 +308,28 @@ fn parse_completed(
dispatcher::ParsedHandler {
completed,
data: PermissionRequestHandlerData { decision },
data: PermissionRequestHandlerData {
should_stop,
stop_reason,
decision,
},
}
}
#[cfg(test)]
mod tests {
use codex_protocol::protocol::HookEventName;
use codex_protocol::protocol::HookOutputEntry;
use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use pretty_assertions::assert_eq;
use super::PermissionRequestDecision;
use super::PermissionRequestHandlerData;
use super::parse_completed;
use super::resolve_permission_request_decision;
use crate::engine::ConfiguredHandler;
use crate::engine::command_runner::CommandRunResult;
#[test]
fn permission_request_deny_overrides_earlier_allow() {
@@ -328,4 +367,58 @@ mod tests {
assert_eq!(resolve_permission_request_decision(decisions.iter()), None);
}
#[test]
fn continue_false_stops_permission_request_flow() {
let parsed = parse_completed(
&handler(),
run_result(
Some(0),
r#"{"continue":false,"stopReason":"stop now","hookSpecificOutput":{"hookEventName":"PermissionRequest","decision":{"behavior":"allow"}}}"#,
"",
),
Some("turn-1".to_string()),
);
assert_eq!(
parsed.data,
PermissionRequestHandlerData {
should_stop: true,
stop_reason: Some("stop now".to_string()),
decision: None,
}
);
assert_eq!(parsed.completed.run.status, HookRunStatus::Stopped);
assert_eq!(
parsed.completed.run.entries,
vec![HookOutputEntry {
kind: HookOutputEntryKind::Stop,
text: "stop now".to_string(),
}]
);
}
fn handler() -> ConfiguredHandler {
ConfiguredHandler {
event_name: HookEventName::PermissionRequest,
matcher: None,
command: "python3 hook.py".to_string(),
timeout_sec: 30,
status_message: None,
source_path: std::path::PathBuf::from("/tmp/hooks.json"),
display_order: 0,
}
}
fn run_result(exit_code: Option<i32>, stdout: &str, stderr: &str) -> CommandRunResult {
CommandRunResult {
started_at: 0,
completed_at: 1,
duration_ms: 1,
exit_code,
stdout: stdout.to_string(),
stderr: stderr.to_string(),
error: None,
}
}
}