Address subagent hook review feedback

This commit is contained in:
Abhinav Vedmala
2026-05-14 17:17:46 -07:00
parent 566c05b886
commit bde5e187f2
17 changed files with 15 additions and 60 deletions

View File

@@ -163,7 +163,7 @@ impl AgentControl {
self.session_id
}
pub(crate) async fn thread_rollout_path(&self, thread_id: ThreadId) -> Option<PathBuf> {
pub(crate) async fn rollout_path_for_thread(&self, thread_id: ThreadId) -> Option<PathBuf> {
let state = self.upgrade().ok()?;
state.get_thread(thread_id).await.ok()?.rollout_path()
}

View File

@@ -69,7 +69,7 @@ pub(crate) enum PendingInputRecord {
},
}
pub(crate) struct StopHookOutcome {
pub(crate) struct TurnStopHookOutcome {
pub should_stop: bool,
pub should_block: bool,
pub continuation_fragments: Vec<codex_protocol::items::HookPromptFragment>,
@@ -345,7 +345,7 @@ pub(crate) async fn run_turn_stop_hooks(
turn_context: &Arc<TurnContext>,
stop_hook_active: bool,
last_assistant_message: Option<String>,
) -> StopHookOutcome {
) -> TurnStopHookOutcome {
let hooks = sess.hooks();
let outcome = if let Some(metadata) = subagent_hook_metadata(sess, turn_context).await {
let request = codex_hooks::SubagentStopRequest {
@@ -381,7 +381,7 @@ pub(crate) async fn run_turn_stop_hooks(
};
emit_hook_completed_events(sess, turn_context, outcome.hook_events).await;
StopHookOutcome {
TurnStopHookOutcome {
should_stop: outcome.should_stop,
should_block: outcome.should_block,
continuation_fragments: outcome.continuation_fragments,
@@ -733,7 +733,7 @@ async fn subagent_hook_metadata(
let parent_transcript_path = if let Some(parent_thread_id) = parent_thread_id {
sess.services
.agent_control
.thread_rollout_path(parent_thread_id)
.rollout_path_for_thread(parent_thread_id)
.await
} else {
None
@@ -747,6 +747,9 @@ async fn subagent_hook_metadata(
})
}
// Hook `agent_type` mirrors the spawn_agent `agent_type` argument. Internally,
// thread-spawned agents store that value as `agent_role`; omitted values use
// the default role, while system subagents expose fixed type labels.
fn subagent_hook_agent_type(subagent_source: &SubAgentSource) -> String {
match subagent_source {
SubAgentSource::ThreadSpawn { agent_role, .. } => agent_role

View File

@@ -41,7 +41,6 @@
"$ref": "#/definitions/NullableString"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -34,7 +34,6 @@
"type": "string"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -45,7 +45,6 @@
"$ref": "#/definitions/NullableString"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -34,7 +34,6 @@
"type": "string"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -44,7 +44,6 @@
"$ref": "#/definitions/NullableString"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -43,7 +43,6 @@
"$ref": "#/definitions/NullableString"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -24,7 +24,6 @@
},
"reason": {
"default": null,
"description": "Claude requires `reason` when `decision` is `block`; we enforce that semantic rule during output parsing rather than in the JSON schema.",
"type": "string"
},
"stopReason": {

View File

@@ -43,7 +43,6 @@
"$ref": "#/definitions/NullableString"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -52,7 +52,6 @@
"$ref": "#/definitions/NullableString"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -24,7 +24,6 @@
},
"reason": {
"default": null,
"description": "Claude requires `reason` when `decision` is `block`; we enforce that semantic rule during output parsing rather than in the JSON schema.",
"type": "string"
},
"stopReason": {

View File

@@ -40,7 +40,6 @@
"$ref": "#/definitions/NullableString"
},
"turn_id": {
"description": "Codex extension: expose the active turn id to internal turn-scoped hooks.",
"type": "string"
}
},

View File

@@ -10,7 +10,6 @@ pub(crate) struct UniversalOutput {
pub(crate) struct SessionStartOutput {
pub universal: UniversalOutput,
pub additional_context: Option<String>,
pub invalid_reason: Option<String>,
}
#[derive(Debug, Clone)]
@@ -99,24 +98,17 @@ pub(crate) fn parse_session_start(stdout: &str) -> Option<SessionStartOutput> {
Some(SessionStartOutput {
universal: UniversalOutput::from(wire.universal),
additional_context,
invalid_reason: None,
})
}
pub(crate) fn parse_subagent_start(stdout: &str) -> Option<SessionStartOutput> {
let wire: SubagentStartCommandOutputWire = parse_json(stdout)?;
let universal = UniversalOutput::from(wire.universal);
let invalid_reason = unsupported_subagent_start_universal(&universal);
let additional_context = if invalid_reason.is_none() {
wire.hook_specific_output
.and_then(|output| output.additional_context)
} else {
None
};
let additional_context = wire
.hook_specific_output
.and_then(|output| output.additional_context);
Some(SessionStartOutput {
universal,
universal: UniversalOutput::from(wire.universal),
additional_context,
invalid_reason,
})
}
@@ -393,18 +385,6 @@ fn unsupported_post_tool_use_universal(universal: &UniversalOutput) -> Option<St
}
}
fn unsupported_subagent_start_universal(universal: &UniversalOutput) -> Option<String> {
if !universal.continue_processing {
Some("SubagentStart hook returned unsupported continue:false".to_string())
} else if universal.stop_reason.is_some() {
Some("SubagentStart hook returned unsupported stopReason".to_string())
} else if universal.suppress_output {
Some("SubagentStart hook returned unsupported suppressOutput".to_string())
} else {
None
}
}
fn unsupported_permission_request_hook_specific_output(
decision: Option<&PermissionRequestDecisionWire>,
) -> Option<String> {

View File

@@ -144,13 +144,7 @@ fn parse_completed(
text: system_message,
});
}
if let Some(invalid_reason) = parsed.invalid_reason {
status = HookRunStatus::Failed;
entries.push(HookOutputEntry {
kind: HookOutputEntryKind::Error,
text: invalid_reason,
});
} else if let Some(additional_context) = parsed.additional_context {
if let Some(additional_context) = parsed.additional_context {
common::append_additional_context(
&mut entries,
&mut additional_contexts_for_model,

View File

@@ -249,7 +249,6 @@ pub(crate) enum PreToolUseDecisionWire {
#[schemars(rename = "pre-tool-use.command.input")]
pub(crate) struct PreToolUseCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -268,7 +267,6 @@ pub(crate) struct PreToolUseCommandInput {
#[schemars(rename = "permission-request.command.input")]
pub(crate) struct PermissionRequestCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -286,7 +284,6 @@ pub(crate) struct PermissionRequestCommandInput {
#[schemars(rename = "post-tool-use.command.input")]
pub(crate) struct PostToolUseCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -306,7 +303,6 @@ pub(crate) struct PostToolUseCommandInput {
#[schemars(rename = "pre-compact.command.input")]
pub(crate) struct PreCompactCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -322,7 +318,6 @@ pub(crate) struct PreCompactCommandInput {
#[schemars(rename = "post-compact.command.input")]
pub(crate) struct PostCompactCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -406,8 +401,6 @@ pub(crate) struct StopCommandOutputWire {
pub universal: HookUniversalOutputWire,
#[serde(default)]
pub decision: Option<BlockDecisionWire>,
/// Claude requires `reason` when `decision` is `block`; we enforce that
/// semantic rule during output parsing rather than in the JSON schema.
#[serde(default)]
pub reason: Option<String>,
}
@@ -421,8 +414,6 @@ pub(crate) struct SubagentStopCommandOutputWire {
pub universal: HookUniversalOutputWire,
#[serde(default)]
pub decision: Option<BlockDecisionWire>,
/// Claude requires `reason` when `decision` is `block`; we enforce that
/// semantic rule during output parsing rather than in the JSON schema.
#[serde(default)]
pub reason: Option<String>,
}
@@ -475,7 +466,6 @@ impl SessionStartCommandInput {
#[schemars(rename = "user-prompt-submit.command.input")]
pub(crate) struct UserPromptSubmitCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -492,7 +482,6 @@ pub(crate) struct UserPromptSubmitCommandInput {
#[schemars(rename = "subagent-start.command.input")]
pub(crate) struct SubagentStartCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -510,7 +499,6 @@ pub(crate) struct SubagentStartCommandInput {
#[schemars(rename = "stop.command.input")]
pub(crate) struct StopCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,
@@ -528,7 +516,6 @@ pub(crate) struct StopCommandInput {
#[schemars(rename = "subagent-stop.command.input")]
pub(crate) struct SubagentStopCommandInput {
pub session_id: String,
/// Codex extension: expose the active turn id to internal turn-scoped hooks.
pub turn_id: String,
pub transcript_path: NullableString,
pub cwd: String,

View File

@@ -17,6 +17,8 @@ expression: popup
PostCompact 0 0 After context compaction
SessionStart 0 0 When a new session starts
UserPromptSubmit 0 0 When the user submits a prompt
SubagentStart 0 0 When a subagent is created
SubagentStop 0 0 Right before a subagent ends its turn
Stop 0 0 Right before Codex ends its turn
Press enter to view hooks; esc to close