mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
Address async hook review feedback
This commit is contained in:
@@ -624,8 +624,7 @@ pub async fn set_thread_memory_mode(sess: &Arc<Session>, sub_id: String, mode: T
|
||||
|
||||
async fn shutdown_session_runtime(sess: &Arc<Session>) {
|
||||
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
|
||||
let hooks = sess.hooks();
|
||||
hooks.shutdown().await;
|
||||
sess.hooks().shutdown().await;
|
||||
let _ = sess.conversation.shutdown().await;
|
||||
sess.services
|
||||
.unified_exec_manager
|
||||
|
||||
@@ -1520,14 +1520,15 @@ impl Session {
|
||||
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
|
||||
self.services.skills_manager.clear_cache();
|
||||
self.services.plugins_manager.clear_cache();
|
||||
let previous_hooks = self.hooks();
|
||||
let hooks = build_hooks_for_config(
|
||||
let hooks_config = build_hooks_config(
|
||||
config.as_ref(),
|
||||
self.services.plugins_manager.as_ref(),
|
||||
self.services.user_shell.as_ref(),
|
||||
Some(previous_hooks.as_ref()),
|
||||
)
|
||||
.await;
|
||||
// Reconfigure the existing hook service so background commands, queued
|
||||
// output, and async delivery generations survive a config refresh.
|
||||
let hooks = self.hooks().reconfigured(hooks_config);
|
||||
|
||||
let state = self.state.lock().await;
|
||||
// A newer refresh may have updated the config while this hook build was in flight.
|
||||
@@ -3410,13 +3411,16 @@ pub(crate) fn emit_subagent_session_started(
|
||||
});
|
||||
}
|
||||
|
||||
/// Builds the hook engine for one config snapshot, including any enabled plugin hooks.
|
||||
async fn build_hooks_for_config(
|
||||
/// Builds the reloadable hook configuration for one config snapshot.
|
||||
///
|
||||
/// Runtime state is intentionally not handled here. Initial session setup passes
|
||||
/// this config to `Hooks::new`, while config refresh passes it to
|
||||
/// `Hooks::reconfigured` so the hook crate can preserve its own in-flight work.
|
||||
async fn build_hooks_config(
|
||||
config: &Config,
|
||||
plugins_manager: &PluginsManager,
|
||||
user_shell: &crate::shell::Shell,
|
||||
previous_hooks: Option<&Hooks>,
|
||||
) -> Hooks {
|
||||
) -> HooksConfig {
|
||||
let mut hook_shell_argv = user_shell.derive_exec_args("", /*use_login_shell*/ false);
|
||||
let hook_shell_program = hook_shell_argv.remove(0);
|
||||
let _ = hook_shell_argv.pop();
|
||||
@@ -3424,7 +3428,7 @@ async fn build_hooks_for_config(
|
||||
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
|
||||
let plugin_hook_sources = plugin_outcome.effective_plugin_hook_sources();
|
||||
let plugin_hook_load_warnings = plugin_outcome.effective_plugin_hook_warnings();
|
||||
let hooks_config = HooksConfig {
|
||||
HooksConfig {
|
||||
legacy_notify_argv: config.notify.clone(),
|
||||
feature_enabled: config.features.enabled(Feature::CodexHooks),
|
||||
bypass_hook_trust: config.bypass_hook_trust,
|
||||
@@ -3433,10 +3437,6 @@ async fn build_hooks_for_config(
|
||||
plugin_hook_load_warnings,
|
||||
shell_program: Some(hook_shell_program),
|
||||
shell_args: hook_shell_argv,
|
||||
};
|
||||
match previous_hooks {
|
||||
Some(previous_hooks) => previous_hooks.reconfigured(hooks_config),
|
||||
None => Hooks::new(hooks_config),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -932,14 +932,9 @@ impl Session {
|
||||
(None, None)
|
||||
};
|
||||
|
||||
let hooks =
|
||||
build_hooks_for_config(
|
||||
&config,
|
||||
plugins_manager.as_ref(),
|
||||
&default_shell,
|
||||
/*previous_hooks*/ None,
|
||||
)
|
||||
.await;
|
||||
let hooks = Hooks::new(
|
||||
build_hooks_config(&config, plugins_manager.as_ref(), &default_shell).await,
|
||||
);
|
||||
for warning in hooks.startup_warnings() {
|
||||
post_session_configured_events.push(Event {
|
||||
id: INITIAL_SUBMIT_ID.to_owned(),
|
||||
|
||||
@@ -165,17 +165,9 @@ pub(crate) async fn run_turn(
|
||||
return None;
|
||||
}
|
||||
let mut can_drain_pending_input = input.is_empty();
|
||||
let async_hook_delivery_cutoff = sess.hooks().async_delivery_cutoff();
|
||||
let hook_input_outcome = run_hooks_and_record_inputs(&sess, &turn_context, &input).await;
|
||||
if hook_input_outcome.should_stop {
|
||||
if run_input_hooks_and_record(&sess, &turn_context, &input).await {
|
||||
return None;
|
||||
}
|
||||
if hook_input_outcome.accepted_user_input {
|
||||
let delivery = sess
|
||||
.hooks()
|
||||
.commit_accepted_turn_and_drain_async_output(async_hook_delivery_cutoff);
|
||||
record_async_hook_delivery(&sess, &turn_context, delivery).await;
|
||||
}
|
||||
|
||||
sess.merge_connector_selection(explicitly_enabled_connectors.clone())
|
||||
.await;
|
||||
@@ -218,18 +210,9 @@ pub(crate) async fn run_turn(
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
let async_hook_delivery_cutoff = sess.hooks().async_delivery_cutoff();
|
||||
let hook_input_outcome =
|
||||
run_hooks_and_record_inputs(&sess, &turn_context, &pending_input).await;
|
||||
if hook_input_outcome.should_stop {
|
||||
if run_input_hooks_and_record(&sess, &turn_context, &pending_input).await {
|
||||
break;
|
||||
}
|
||||
if hook_input_outcome.accepted_user_input {
|
||||
let delivery = sess
|
||||
.hooks()
|
||||
.commit_accepted_turn_and_drain_async_output(async_hook_delivery_cutoff);
|
||||
record_async_hook_delivery(&sess, &turn_context, delivery).await;
|
||||
}
|
||||
|
||||
// Construct the input that we will send to the model.
|
||||
let sampling_request_input: Vec<ResponseItem> = {
|
||||
@@ -414,16 +397,18 @@ async fn turn_diff_display_roots(turn_context: &TurnContext) -> Vec<(String, Pat
|
||||
display_roots
|
||||
}
|
||||
|
||||
struct HookInputOutcome {
|
||||
should_stop: bool,
|
||||
accepted_user_input: bool,
|
||||
}
|
||||
|
||||
async fn run_hooks_and_record_inputs(
|
||||
/// Runs prompt-submit hooks, records accepted input, and advances async delivery.
|
||||
///
|
||||
/// The readiness cutoff is captured before synchronous hooks run. Async output
|
||||
/// that completes while those hooks are running therefore waits for a later
|
||||
/// accepted model request. If every user input is blocked, the accepted
|
||||
/// generation is not advanced and queued output remains pending.
|
||||
async fn run_input_hooks_and_record(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
input: &[TurnInput],
|
||||
) -> HookInputOutcome {
|
||||
) -> bool {
|
||||
let async_hook_delivery_cutoff = sess.hooks().async_delivery_cutoff();
|
||||
let mut blocked_input = false;
|
||||
let mut accepted_user_input = false;
|
||||
for input_item in input {
|
||||
@@ -444,10 +429,13 @@ async fn run_hooks_and_record_inputs(
|
||||
.await;
|
||||
}
|
||||
}
|
||||
HookInputOutcome {
|
||||
should_stop: blocked_input && !accepted_user_input,
|
||||
accepted_user_input,
|
||||
if accepted_user_input {
|
||||
let delivery = sess
|
||||
.hooks()
|
||||
.commit_accepted_turn_and_drain_async_output(async_hook_delivery_cutoff);
|
||||
record_async_hook_delivery(sess, turn_context, delivery).await;
|
||||
}
|
||||
blocked_input && !accepted_user_input
|
||||
}
|
||||
|
||||
#[expect(
|
||||
|
||||
@@ -1,3 +1,20 @@
|
||||
//! Session-scoped runtime for detached command hooks.
|
||||
//!
|
||||
//! Async hooks cannot affect the operation that launched them. Successful
|
||||
//! informational output is queued until a later model request accepts user
|
||||
//! input. Delivery uses two independent gates:
|
||||
//!
|
||||
//! - An accepted-input generation prevents output from being delivered before
|
||||
//! its eligible model request. Most events target the next generation;
|
||||
//! session and subagent start events target the generation after that so
|
||||
//! their output always skips the model request that runs the start hook.
|
||||
//! - A readiness sequence provides a per-submission cutoff. Core snapshots the
|
||||
//! cutoff before synchronous prompt hooks run, so async output that completes
|
||||
//! during that work cannot race into the same model request.
|
||||
//!
|
||||
//! The runtime survives hook configuration refreshes and bounds concurrent
|
||||
//! commands, queued completions, and the amount delivered to one request.
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
@@ -7,6 +24,7 @@ use std::sync::atomic::AtomicU64;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
use codex_utils_output_truncation::approx_token_count;
|
||||
use tokio::task::JoinHandle;
|
||||
|
||||
@@ -21,33 +39,43 @@ const MAX_IN_FLIGHT_COMMANDS: usize = 32;
|
||||
const MAX_DELIVERED_COMPLETIONS_PER_TURN: usize = 8;
|
||||
const MAX_DELIVERED_CONTEXT_TOKENS_PER_TURN: usize = 10_000;
|
||||
|
||||
/// Informational async hook output ready to be recorded for a model request.
|
||||
#[derive(Debug, Default, PartialEq, Eq)]
|
||||
pub struct AsyncHookDelivery {
|
||||
/// Context fragments to inject into model-visible conversation history.
|
||||
pub additional_contexts: Vec<String>,
|
||||
/// User-visible messages to emit without adding them to model context.
|
||||
pub system_messages: Vec<String>,
|
||||
}
|
||||
|
||||
/// Snapshot of completions that were ready before prompt submission began.
|
||||
///
|
||||
/// The sequence is opaque outside this module so callers cannot manufacture or
|
||||
/// compare cutoffs independently of the runtime that created them.
|
||||
#[doc(hidden)]
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub struct AsyncHookDeliveryCutoff {
|
||||
ready_sequence: u64,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub(crate) enum AsyncDeliveryTiming {
|
||||
NextAcceptedTurn,
|
||||
AcceptedTurnAfterNext,
|
||||
}
|
||||
|
||||
impl AsyncDeliveryTiming {
|
||||
fn generation_delay(self) -> u64 {
|
||||
match self {
|
||||
Self::NextAcceptedTurn => 1,
|
||||
Self::AcceptedTurnAfterNext => 2,
|
||||
}
|
||||
fn generation_delay(event_name: HookEventName) -> u64 {
|
||||
match event_name {
|
||||
HookEventName::SessionStart | HookEventName::SubagentStart => 2,
|
||||
HookEventName::PreToolUse
|
||||
| HookEventName::PermissionRequest
|
||||
| HookEventName::PostToolUse
|
||||
| HookEventName::PreCompact
|
||||
| HookEventName::PostCompact
|
||||
| HookEventName::UserPromptSubmit
|
||||
| HookEventName::SubagentStop
|
||||
| HookEventName::Stop => 1,
|
||||
}
|
||||
}
|
||||
|
||||
/// Shared runtime state for async commands launched during one Codex session.
|
||||
///
|
||||
/// Clones refer to the same in-flight tasks, queued output, and delivery
|
||||
/// generations. This lets hook configuration refresh without orphaning work.
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct AsyncCommandRuntime {
|
||||
inner: Arc<AsyncCommandRuntimeInner>,
|
||||
@@ -76,6 +104,7 @@ struct AsyncHookCompletion {
|
||||
}
|
||||
|
||||
impl AsyncCommandRuntime {
|
||||
/// Creates an empty runtime for a newly started Codex session.
|
||||
pub(crate) fn new() -> Self {
|
||||
Self {
|
||||
inner: Arc::new(AsyncCommandRuntimeInner {
|
||||
@@ -88,10 +117,19 @@ impl AsyncCommandRuntime {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the spiller shared by synchronous and asynchronous hook output.
|
||||
///
|
||||
/// Keeping it with the refresh-stable runtime lets detached commands spill
|
||||
/// output even if hook configuration changes before they finish.
|
||||
pub(crate) fn output_spiller(&self) -> &HookOutputSpiller {
|
||||
&self.inner.output_spiller
|
||||
}
|
||||
|
||||
/// Captures which async completions were ready before prompt hooks run.
|
||||
///
|
||||
/// A completion registered after this snapshot is ineligible for the
|
||||
/// accepted model request associated with the snapshot, even if the command
|
||||
/// finishes before synchronous prompt hooks return.
|
||||
pub(crate) fn delivery_cutoff(&self) -> AsyncHookDeliveryCutoff {
|
||||
let ready_sequence = self
|
||||
.inner
|
||||
@@ -102,6 +140,11 @@ impl AsyncCommandRuntime {
|
||||
AsyncHookDeliveryCutoff { ready_sequence }
|
||||
}
|
||||
|
||||
/// Launches one command without waiting for it or emitting hook lifecycle events.
|
||||
///
|
||||
/// Only successful informational output is queued. Control decisions are
|
||||
/// discarded by the async parser. The event determines the earliest
|
||||
/// accepted-input generation at which the output may be delivered.
|
||||
pub(crate) fn spawn(
|
||||
&self,
|
||||
shell: CommandShell,
|
||||
@@ -109,7 +152,6 @@ impl AsyncCommandRuntime {
|
||||
input_json: String,
|
||||
cwd: PathBuf,
|
||||
thread_id: ThreadId,
|
||||
delivery_timing: AsyncDeliveryTiming,
|
||||
) {
|
||||
if self.inner.shutting_down.load(Ordering::Acquire) {
|
||||
return;
|
||||
@@ -142,7 +184,7 @@ impl AsyncCommandRuntime {
|
||||
.inner
|
||||
.accepted_turn_generation
|
||||
.load(Ordering::Acquire)
|
||||
.saturating_add(delivery_timing.generation_delay());
|
||||
.saturating_add(generation_delay(handler.event_name));
|
||||
let inner = Arc::clone(&self.inner);
|
||||
let handle = tokio::spawn(async move {
|
||||
let result = run_command(&shell, &handler, &input_json, &cwd).await;
|
||||
@@ -203,6 +245,11 @@ impl AsyncCommandRuntime {
|
||||
state.tasks.push(handle);
|
||||
}
|
||||
|
||||
/// Advances the accepted-input generation and drains eligible output.
|
||||
///
|
||||
/// Output must both target the new generation and have been ready before
|
||||
/// `cutoff`. Delivery is bounded; remaining eligible output stays queued for
|
||||
/// later accepted model requests.
|
||||
pub(crate) fn commit_accepted_turn_and_drain(
|
||||
&self,
|
||||
cutoff: AsyncHookDeliveryCutoff,
|
||||
@@ -256,6 +303,10 @@ impl AsyncCommandRuntime {
|
||||
delivery
|
||||
}
|
||||
|
||||
/// Stops accepting output, clears queued completions, and aborts all tasks.
|
||||
///
|
||||
/// Shutdown waits for every aborted Tokio task so no detached hook command
|
||||
/// remains owned by the session after this method returns.
|
||||
pub(crate) async fn shutdown(&self) {
|
||||
self.inner.shutting_down.store(true, Ordering::Release);
|
||||
let tasks = {
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::AsyncCommandRuntime;
|
||||
use super::AsyncDeliveryTiming;
|
||||
use super::AsyncHookCompletion;
|
||||
use super::MAX_DELIVERED_CONTEXT_TOKENS_PER_TURN;
|
||||
use super::MAX_IN_FLIGHT_COMMANDS;
|
||||
@@ -122,7 +121,6 @@ async fn launch_is_skipped_at_session_concurrency_limit() {
|
||||
String::new(),
|
||||
std::env::current_dir().expect("current dir"),
|
||||
ThreadId::new(),
|
||||
AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
|
||||
@@ -15,7 +15,6 @@ use codex_protocol::protocol::HookScope;
|
||||
use super::CommandShell;
|
||||
use super::ConfiguredHandler;
|
||||
use super::async_command::AsyncCommandRuntime;
|
||||
use super::async_command::AsyncDeliveryTiming;
|
||||
use super::command_runner::CommandRunResult;
|
||||
use super::command_runner::run_command;
|
||||
use crate::events::common::matches_matcher;
|
||||
@@ -27,12 +26,6 @@ pub(crate) struct ParsedHandler<T> {
|
||||
pub completion_order: usize,
|
||||
}
|
||||
|
||||
pub(crate) struct AsyncHandlerExecution<'a> {
|
||||
pub session_id: ThreadId,
|
||||
pub runtime: &'a AsyncCommandRuntime,
|
||||
pub delivery_timing: AsyncDeliveryTiming,
|
||||
}
|
||||
|
||||
pub(crate) fn select_handlers(
|
||||
handlers: &[ConfiguredHandler],
|
||||
event_name: HookEventName,
|
||||
@@ -117,13 +110,18 @@ pub(crate) fn running_summary(handler: &ConfiguredHandler) -> HookRunSummary {
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::too_many_arguments,
|
||||
reason = "keeping async runtime and session identity explicit avoids event-specific wrappers"
|
||||
)]
|
||||
pub(crate) async fn execute_handlers<T>(
|
||||
shell: &CommandShell,
|
||||
handlers: Vec<ConfiguredHandler>,
|
||||
input_json: String,
|
||||
cwd: &Path,
|
||||
turn_id: Option<String>,
|
||||
async_execution: AsyncHandlerExecution<'_>,
|
||||
async_runtime: &AsyncCommandRuntime,
|
||||
session_id: ThreadId,
|
||||
parse: fn(&ConfiguredHandler, CommandRunResult, Option<String>) -> ParsedHandler<T>,
|
||||
) -> Vec<ParsedHandler<T>> {
|
||||
let mut pending = FuturesUnordered::new();
|
||||
@@ -137,13 +135,12 @@ pub(crate) async fn execute_handlers<T>(
|
||||
(configured_order, parse(&handler, result, turn_id))
|
||||
});
|
||||
}
|
||||
HookExecutionMode::Async => async_execution.runtime.spawn(
|
||||
HookExecutionMode::Async => async_runtime.spawn(
|
||||
shell.clone(),
|
||||
handler,
|
||||
input_json,
|
||||
cwd.to_path_buf(),
|
||||
async_execution.session_id,
|
||||
async_execution.delivery_timing,
|
||||
session_id,
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -117,25 +117,6 @@ impl ClaudeHooksEngine {
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
shell: CommandShell,
|
||||
) -> Self {
|
||||
Self::new_with_runtime(
|
||||
enabled,
|
||||
bypass_hook_trust,
|
||||
config_layer_stack,
|
||||
plugin_hook_sources,
|
||||
plugin_hook_load_warnings,
|
||||
shell,
|
||||
AsyncCommandRuntime::new(),
|
||||
)
|
||||
}
|
||||
|
||||
fn new_with_runtime(
|
||||
enabled: bool,
|
||||
bypass_hook_trust: bool,
|
||||
config_layer_stack: Option<&ConfigLayerStack>,
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
shell: CommandShell,
|
||||
async_runtime: AsyncCommandRuntime,
|
||||
) -> Self {
|
||||
if !enabled {
|
||||
@@ -162,24 +143,9 @@ impl ClaudeHooksEngine {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn reconfigured(
|
||||
&self,
|
||||
enabled: bool,
|
||||
bypass_hook_trust: bool,
|
||||
config_layer_stack: Option<&ConfigLayerStack>,
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
shell: CommandShell,
|
||||
) -> Self {
|
||||
Self::new_with_runtime(
|
||||
enabled,
|
||||
bypass_hook_trust,
|
||||
config_layer_stack,
|
||||
plugin_hook_sources,
|
||||
plugin_hook_load_warnings,
|
||||
shell,
|
||||
self.async_runtime.clone(),
|
||||
)
|
||||
/// Returns the refresh-stable runtime for a replacement handler snapshot.
|
||||
pub(crate) fn async_runtime(&self) -> AsyncCommandRuntime {
|
||||
self.async_runtime.clone()
|
||||
}
|
||||
|
||||
pub(crate) fn async_delivery_cutoff(&self) -> AsyncHookDeliveryCutoff {
|
||||
|
||||
@@ -30,12 +30,32 @@ use tempfile::tempdir;
|
||||
|
||||
use super::ClaudeHooksEngine;
|
||||
use super::CommandShell;
|
||||
use super::async_command::AsyncCommandRuntime;
|
||||
use crate::events::pre_tool_use::PreToolUseRequest;
|
||||
|
||||
fn cwd() -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::current_dir().expect("current dir")
|
||||
}
|
||||
|
||||
fn test_engine(
|
||||
enabled: bool,
|
||||
bypass_hook_trust: bool,
|
||||
config_layer_stack: Option<&ConfigLayerStack>,
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
shell: CommandShell,
|
||||
) -> ClaudeHooksEngine {
|
||||
ClaudeHooksEngine::new(
|
||||
enabled,
|
||||
bypass_hook_trust,
|
||||
config_layer_stack,
|
||||
plugin_hook_sources,
|
||||
plugin_hook_load_warnings,
|
||||
shell,
|
||||
AsyncCommandRuntime::new(),
|
||||
)
|
||||
}
|
||||
|
||||
fn managed_hooks_for_current_platform(
|
||||
managed_dir: impl AsRef<Path>,
|
||||
hooks: HookEventsToml,
|
||||
@@ -194,7 +214,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -300,7 +320,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -379,7 +399,7 @@ fn unknown_requirement_source_hooks_stay_managed() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -461,7 +481,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -527,7 +547,7 @@ fn user_disablement_does_not_filter_managed_layer_hooks() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -688,7 +708,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -744,7 +764,7 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -798,7 +818,7 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -868,7 +888,7 @@ fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -907,7 +927,7 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() {
|
||||
let config_layer_stack = ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -979,7 +999,7 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -1089,7 +1109,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
|
||||
)
|
||||
.expect("config layer stack");
|
||||
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -1182,7 +1202,7 @@ print(json.dumps({
|
||||
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"),
|
||||
&plugin_hook_sources,
|
||||
);
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -1301,7 +1321,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
|
||||
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"),
|
||||
&plugin_hook_sources,
|
||||
);
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
Some(&config_layer_stack),
|
||||
@@ -1345,7 +1365,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
|
||||
|
||||
#[test]
|
||||
fn plugin_hook_load_warnings_are_startup_warnings() {
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
let engine = test_engine(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
/*config_layer_stack*/ None,
|
||||
|
||||
@@ -300,6 +300,13 @@ pub(crate) fn parse_stop(stdout: &str) -> Option<StopOutput> {
|
||||
))
|
||||
}
|
||||
|
||||
/// Extracts only informational fields from a successful async command result.
|
||||
///
|
||||
/// This reuses the event-specific synchronous parser so existing support for
|
||||
/// `additionalContext`, `systemMessage`, and plain stdout remains consistent.
|
||||
/// Blocking decisions, steering, updated input, and every other control field
|
||||
/// are intentionally discarded because async hooks cannot affect the operation
|
||||
/// that launched them.
|
||||
pub(crate) fn parse_async_informational(
|
||||
event_name: HookEventName,
|
||||
result: &CommandRunResult,
|
||||
|
||||
@@ -109,11 +109,8 @@ pub(crate) async fn run_pre(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
Some(request.turn_id),
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing: crate::engine::async_command::AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_pre_completed,
|
||||
)
|
||||
.await;
|
||||
@@ -197,11 +194,8 @@ pub(crate) async fn run_post(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
Some(request.turn_id),
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing: crate::engine::async_command::AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_post_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -125,11 +125,8 @@ pub(crate) async fn run(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
Some(request.turn_id.clone()),
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing: crate::engine::async_command::AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -110,11 +110,8 @@ pub(crate) async fn run(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
Some(request.turn_id.clone()),
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing: crate::engine::async_command::AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -109,11 +109,8 @@ pub(crate) async fn run(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
Some(request.turn_id.clone()),
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing: crate::engine::async_command::AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -187,12 +187,8 @@ pub(crate) async fn run(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
turn_id,
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing:
|
||||
crate::engine::async_command::AsyncDeliveryTiming::AcceptedTurnAfterNext,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -184,11 +184,8 @@ pub(crate) async fn run(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
Some(request.turn_id),
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing: crate::engine::async_command::AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -109,11 +109,8 @@ pub(crate) async fn run(
|
||||
input_json,
|
||||
request.cwd.as_path(),
|
||||
Some(request.turn_id),
|
||||
dispatcher::AsyncHandlerExecution {
|
||||
session_id: request.session_id,
|
||||
runtime: async_runtime,
|
||||
delivery_timing: crate::engine::async_command::AsyncDeliveryTiming::NextAcceptedTurn,
|
||||
},
|
||||
async_runtime,
|
||||
request.session_id,
|
||||
parse_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::engine::AsyncHookDeliveryCutoff;
|
||||
use crate::engine::ClaudeHooksEngine;
|
||||
use crate::engine::CommandShell;
|
||||
use crate::engine::HookListEntry;
|
||||
use crate::engine::async_command::AsyncCommandRuntime;
|
||||
use crate::events::compact::PostCompactRequest;
|
||||
use crate::events::compact::PreCompactOutcome;
|
||||
use crate::events::compact::PreCompactRequest;
|
||||
@@ -85,6 +86,7 @@ impl Hooks {
|
||||
program: shell_program.unwrap_or_default(),
|
||||
args: shell_args,
|
||||
},
|
||||
AsyncCommandRuntime::new(),
|
||||
);
|
||||
Self {
|
||||
after_agent,
|
||||
@@ -108,7 +110,7 @@ impl Hooks {
|
||||
.map(crate::notify_hook)
|
||||
.into_iter()
|
||||
.collect();
|
||||
let engine = self.engine.reconfigured(
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
feature_enabled,
|
||||
bypass_hook_trust,
|
||||
config_layer_stack.as_ref(),
|
||||
@@ -118,6 +120,7 @@ impl Hooks {
|
||||
program: shell_program.unwrap_or_default(),
|
||||
args: shell_args,
|
||||
},
|
||||
self.engine.async_runtime(),
|
||||
);
|
||||
Self {
|
||||
after_agent,
|
||||
|
||||
Reference in New Issue
Block a user