mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Add configurable hook context spill limits (#34393)
## What changed - Add `additionalContextLimit` to command hook configuration for events that can emit `additionalContext`. - Apply the limit independently to each hook's context before it is sent to the model. Unset values retain the 2,500-token default, while `0` disables spilling for that hook. - Expose configured limits through app-server hook and requirements responses, and show them in the TUI hooks browser. ## Testing - Cover JSON and TOML parsing, discovery and hashing, per-hook spilling behavior, app-server responses, and TUI rendering. GitOrigin-RevId: 55ddc2dbbdf9067dceee3e1968c03617bbf6bcea
This commit is contained in:
committed by
copyberry
parent
bd92b056dd
commit
e4836f998d
+18
@@ -8761,6 +8761,15 @@
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"additionalContextLimit": {
|
||||
"description": "Approximate token threshold for spilling this hook's `additionalContext` to disk. `null` uses 2,500 tokens; `0` disables spilling for this hook. The threshold is evaluated against the original context; a spilled preview also includes recovery metadata.",
|
||||
"format": "uint",
|
||||
"minimum": 0.0,
|
||||
"type": [
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"async": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -11211,6 +11220,15 @@
|
||||
},
|
||||
"HookMetadata": {
|
||||
"properties": {
|
||||
"additionalContextLimit": {
|
||||
"description": "Configured `additionalContext` spill threshold. `null` uses 2,500 tokens; `0` disables spilling.",
|
||||
"format": "uint",
|
||||
"minimum": 0.0,
|
||||
"type": [
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"command": {
|
||||
"type": [
|
||||
"string",
|
||||
|
||||
+18
@@ -4954,6 +4954,15 @@
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"additionalContextLimit": {
|
||||
"description": "Approximate token threshold for spilling this hook's `additionalContext` to disk. `null` uses 2,500 tokens; `0` disables spilling for this hook. The threshold is evaluated against the original context; a spilled preview also includes recovery metadata.",
|
||||
"format": "uint",
|
||||
"minimum": 0.0,
|
||||
"type": [
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"async": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -7515,6 +7524,15 @@
|
||||
},
|
||||
"HookMetadata": {
|
||||
"properties": {
|
||||
"additionalContextLimit": {
|
||||
"description": "Configured `additionalContext` spill threshold. `null` uses 2,500 tokens; `0` disables spilling.",
|
||||
"format": "uint",
|
||||
"minimum": 0.0,
|
||||
"type": [
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"command": {
|
||||
"type": [
|
||||
"string",
|
||||
|
||||
+9
@@ -187,6 +187,15 @@
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"additionalContextLimit": {
|
||||
"description": "Approximate token threshold for spilling this hook's `additionalContext` to disk. `null` uses 2,500 tokens; `0` disables spilling for this hook. The threshold is evaluated against the original context; a spilled preview also includes recovery metadata.",
|
||||
"format": "uint",
|
||||
"minimum": 0.0,
|
||||
"type": [
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"async": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -46,6 +46,15 @@
|
||||
},
|
||||
"HookMetadata": {
|
||||
"properties": {
|
||||
"additionalContextLimit": {
|
||||
"description": "Configured `additionalContext` spill threshold. `null` uses 2,500 tokens; `0` disables spilling.",
|
||||
"format": "uint",
|
||||
"minimum": 0.0,
|
||||
"type": [
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"command": {
|
||||
"type": [
|
||||
"string",
|
||||
|
||||
+8
-1
@@ -2,4 +2,11 @@
|
||||
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
|
||||
export type ConfiguredHookHandler = { "type": "command", command: string, commandWindows: string | null, timeoutSec: bigint | null, async: boolean, statusMessage: string | null, } | { "type": "prompt", } | { "type": "agent", };
|
||||
export type ConfiguredHookHandler = { "type": "command", command: string, commandWindows: string | null, timeoutSec: bigint | null, async: boolean, statusMessage: string | null,
|
||||
/**
|
||||
* Approximate token threshold for spilling this hook's `additionalContext` to disk.
|
||||
* `null` uses 2,500 tokens; `0` disables spilling for this hook. The threshold is
|
||||
* evaluated against the original context; a spilled preview also includes recovery
|
||||
* metadata.
|
||||
*/
|
||||
additionalContextLimit: number | null, } | { "type": "prompt", } | { "type": "agent", };
|
||||
|
||||
@@ -7,4 +7,9 @@ import type { HookHandlerType } from "./HookHandlerType";
|
||||
import type { HookSource } from "./HookSource";
|
||||
import type { HookTrustStatus } from "./HookTrustStatus";
|
||||
|
||||
export type HookMetadata = { key: string, eventName: HookEventName, handlerType: HookHandlerType, matcher: string | null, command: string | null, timeoutSec: bigint, statusMessage: string | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, currentHash: string, trustStatus: HookTrustStatus, };
|
||||
export type HookMetadata = { key: string, eventName: HookEventName, handlerType: HookHandlerType, matcher: string | null, command: string | null, timeoutSec: bigint, statusMessage: string | null,
|
||||
/**
|
||||
* Configured `additionalContext` spill threshold.
|
||||
* `null` uses 2,500 tokens; `0` disables spilling.
|
||||
*/
|
||||
additionalContextLimit: number | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, currentHash: string, trustStatus: HookTrustStatus, };
|
||||
|
||||
@@ -484,6 +484,13 @@ pub enum ConfiguredHookHandler {
|
||||
#[serde(rename = "statusMessage")]
|
||||
#[ts(rename = "statusMessage")]
|
||||
status_message: Option<String>,
|
||||
/// Approximate token threshold for spilling this hook's `additionalContext` to disk.
|
||||
/// `null` uses 2,500 tokens; `0` disables spilling for this hook. The threshold is
|
||||
/// evaluated against the original context; a spilled preview also includes recovery
|
||||
/// metadata.
|
||||
#[serde(rename = "additionalContextLimit")]
|
||||
#[ts(rename = "additionalContextLimit")]
|
||||
additional_context_limit: Option<usize>,
|
||||
},
|
||||
#[serde(rename = "prompt")]
|
||||
#[ts(rename = "prompt")]
|
||||
|
||||
@@ -519,6 +519,9 @@ pub struct HookMetadata {
|
||||
pub command: Option<String>,
|
||||
pub timeout_sec: u64,
|
||||
pub status_message: Option<String>,
|
||||
/// Configured `additionalContext` spill threshold.
|
||||
/// `null` uses 2,500 tokens; `0` disables spilling.
|
||||
pub additional_context_limit: Option<usize>,
|
||||
pub source_path: AbsolutePathBuf,
|
||||
pub source: HookSource,
|
||||
pub plugin_id: Option<String>,
|
||||
|
||||
@@ -248,7 +248,7 @@ Example with notification opt-out:
|
||||
- `externalAgentConfig/import/readHistories` — read completed import histories and connector candidates detected from successfully imported session histories. Connector candidates include a normalized display `name`, the number of imported sessions that used the connector, and the source metadata field used for detection.
|
||||
- `config/value/write` — write a single config key/value to the user's config.toml on disk; dotted paths such as `desktop.someKey` use the same generic write surface.
|
||||
- `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk, with optional `reloadUserConfig: true` to hot-reload loaded threads, including multiple `desktop.*` edits.
|
||||
- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), the layered permission-profile allow map (`allowedPermissionProfiles`), the managed permission-profile default (`defaultPermissions`), lifecycle hook lockdown (`allowManagedHooksOnly`), remote-control policy (`allowRemoteControl`; `false` force-disables remote control while `true` or `null` preserves existing behavior), computer use policy (`computerUse`), pinned feature values (`featureRequirements`), managed lifecycle hooks (`hooks`), `enforceResidency`, managed new-thread defaults (`models.newThread.model`, `models.newThread.modelReasoningEffort`, and `models.newThread.serviceTier`), and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`.
|
||||
- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), the layered permission-profile allow map (`allowedPermissionProfiles`), the managed permission-profile default (`defaultPermissions`), lifecycle hook lockdown (`allowManagedHooksOnly`), remote-control policy (`allowRemoteControl`; `false` force-disables remote control while `true` or `null` preserves existing behavior), computer use policy (`computerUse`), pinned feature values (`featureRequirements`), managed lifecycle hooks (`hooks`, including each command handler's optional `additionalContextLimit`), `enforceResidency`, managed new-thread defaults (`models.newThread.model`, `models.newThread.modelReasoningEffort`, and `models.newThread.serviceTier`), and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`.
|
||||
|
||||
### Example: Start or resume a thread
|
||||
|
||||
@@ -1832,6 +1832,7 @@ For unmanaged hooks, `currentHash` and `trustStatus` describe whether the curren
|
||||
"command": "python3 /Users/me/hook.py",
|
||||
"timeoutSec": 5,
|
||||
"statusMessage": "running hook",
|
||||
"additionalContextLimit": null,
|
||||
"sourcePath": "/Users/me/.codex/config.toml",
|
||||
"source": "user",
|
||||
"pluginId": null,
|
||||
|
||||
@@ -72,6 +72,7 @@ fn hooks_to_info(hooks: &[codex_hooks::HookListEntry]) -> Vec<HookMetadata> {
|
||||
command: hook.command.clone(),
|
||||
timeout_sec: hook.timeout_sec,
|
||||
status_message: hook.status_message.clone(),
|
||||
additional_context_limit: hook.additional_context_limit,
|
||||
source_path: hook.source_path.clone(),
|
||||
source: hook.source.into(),
|
||||
plugin_id: hook.plugin_id.clone(),
|
||||
|
||||
@@ -460,12 +460,14 @@ fn map_hook_handler_to_api(handler: CoreHookHandlerConfig) -> ConfiguredHookHand
|
||||
timeout_sec,
|
||||
r#async,
|
||||
status_message,
|
||||
additional_context_limit,
|
||||
} => ConfiguredHookHandler::Command {
|
||||
command,
|
||||
command_windows,
|
||||
timeout_sec,
|
||||
r#async,
|
||||
status_message,
|
||||
additional_context_limit,
|
||||
},
|
||||
CoreHookHandlerConfig::Prompt {} => ConfiguredHookHandler::Prompt {},
|
||||
CoreHookHandlerConfig::Agent {} => ConfiguredHookHandler::Agent {},
|
||||
|
||||
@@ -17,6 +17,7 @@ use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigRequirementsReadResponse;
|
||||
use codex_app_server_protocol::ConfigValueWriteParams;
|
||||
use codex_app_server_protocol::ConfigWriteResponse;
|
||||
use codex_app_server_protocol::ConfiguredHookHandler;
|
||||
use codex_app_server_protocol::ForcedChatgptWorkspaceIds;
|
||||
use codex_app_server_protocol::JSONRPCError;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
@@ -49,11 +50,21 @@ fn write_config(codex_home: &TempDir, contents: &str) -> Result<()> {
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_requirements_read_includes_allow_remote_control() -> Result<()> {
|
||||
async fn config_requirements_read_includes_remote_control_and_managed_hooks() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
std::fs::write(
|
||||
codex_home.path().join("requirements.toml"),
|
||||
"allow_remote_control = false\n",
|
||||
r#"allow_remote_control = false
|
||||
|
||||
[hooks]
|
||||
|
||||
[[hooks.SessionStart]]
|
||||
|
||||
[[hooks.SessionStart.hooks]]
|
||||
type = "command"
|
||||
command = "echo managed"
|
||||
additionalContextLimit = 4096
|
||||
"#,
|
||||
)?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
@@ -69,12 +80,24 @@ async fn config_requirements_read_includes_allow_remote_control() -> Result<()>
|
||||
)
|
||||
.await??;
|
||||
let response: ConfigRequirementsReadResponse = to_response(response)?;
|
||||
let requirements = response
|
||||
.requirements
|
||||
.expect("managed requirements should be returned");
|
||||
assert_eq!(requirements.allow_remote_control, Some(false));
|
||||
assert_eq!(
|
||||
response
|
||||
.requirements
|
||||
.expect("managed requirements should be returned")
|
||||
.allow_remote_control,
|
||||
Some(false)
|
||||
requirements
|
||||
.hooks
|
||||
.expect("managed hooks should be returned")
|
||||
.session_start[0]
|
||||
.hooks,
|
||||
vec![ConfiguredHookHandler::Command {
|
||||
command: "echo managed".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: Some(4_096),
|
||||
}]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -47,6 +47,7 @@ fn command_hook_hash(
|
||||
command: &str,
|
||||
timeout_sec: u64,
|
||||
status_message: Option<&str>,
|
||||
additional_context_limit: Option<usize>,
|
||||
) -> String {
|
||||
let identity = NormalizedHookIdentity {
|
||||
event_name,
|
||||
@@ -58,6 +59,7 @@ fn command_hook_hash(
|
||||
timeout_sec: Some(timeout_sec),
|
||||
r#async: false,
|
||||
status_message: status_message.map(ToOwned::to_owned),
|
||||
additional_context_limit,
|
||||
}],
|
||||
},
|
||||
};
|
||||
@@ -80,6 +82,7 @@ type = "command"
|
||||
command = "python3 /tmp/listed-hook.py"
|
||||
timeout = 5
|
||||
statusMessage = "running listed hook"
|
||||
additionalContextLimit = 4096
|
||||
"#,
|
||||
)?;
|
||||
Ok(())
|
||||
@@ -168,6 +171,7 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> {
|
||||
command: Some("python3 /tmp/listed-hook.py".to_string()),
|
||||
timeout_sec: 5,
|
||||
status_message: Some("running listed hook".to_string()),
|
||||
additional_context_limit: Some(4_096),
|
||||
source_path: config_path,
|
||||
source: HookSource::User,
|
||||
plugin_id: None,
|
||||
@@ -180,6 +184,7 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> {
|
||||
"python3 /tmp/listed-hook.py",
|
||||
/*timeout_sec*/ 5,
|
||||
Some("running listed hook"),
|
||||
/*additional_context_limit*/ Some(4_096),
|
||||
),
|
||||
trust_status: HookTrustStatus::Untrusted,
|
||||
}],
|
||||
@@ -250,6 +255,7 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> {
|
||||
command: Some("echo plugin hook".to_string()),
|
||||
timeout_sec: 7,
|
||||
status_message: Some("running plugin hook".to_string()),
|
||||
additional_context_limit: None,
|
||||
source_path: plugin_hooks_path,
|
||||
source: HookSource::Plugin,
|
||||
plugin_id: Some("demo@test".to_string()),
|
||||
@@ -262,6 +268,7 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> {
|
||||
"echo plugin hook",
|
||||
/*timeout_sec*/ 7,
|
||||
Some("running plugin hook"),
|
||||
/*additional_context_limit*/ None,
|
||||
),
|
||||
trust_status: HookTrustStatus::Untrusted,
|
||||
}],
|
||||
@@ -464,6 +471,7 @@ timeout = 5
|
||||
command: Some("echo project hook".to_string()),
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
source_path: project_config_path,
|
||||
source: HookSource::Project,
|
||||
plugin_id: None,
|
||||
@@ -476,6 +484,7 @@ timeout = 5
|
||||
"echo project hook",
|
||||
/*timeout_sec*/ 5,
|
||||
/*status_message*/ None,
|
||||
/*additional_context_limit*/ None,
|
||||
),
|
||||
trust_status: HookTrustStatus::Untrusted,
|
||||
}],
|
||||
|
||||
@@ -158,6 +158,16 @@ pub enum HookHandlerConfig {
|
||||
r#async: bool,
|
||||
#[serde(default, rename = "statusMessage")]
|
||||
status_message: Option<String>,
|
||||
/// Approximate token threshold for spilling this hook's `additionalContext` to disk.
|
||||
/// Unset uses 2,500 tokens; `0` disables spilling for this hook. The threshold is
|
||||
/// evaluated against the original context; a spilled preview also includes recovery
|
||||
/// metadata.
|
||||
#[serde(
|
||||
default,
|
||||
rename = "additionalContextLimit",
|
||||
skip_serializing_if = "Option::is_none"
|
||||
)]
|
||||
additional_context_limit: Option<usize>,
|
||||
},
|
||||
#[serde(rename = "prompt")]
|
||||
Prompt {},
|
||||
|
||||
@@ -23,7 +23,8 @@ fn hooks_file_deserializes_existing_json_shape() {
|
||||
"type": "command",
|
||||
"command": "python3 /tmp/pre.py",
|
||||
"timeout": 10,
|
||||
"statusMessage": "checking"
|
||||
"statusMessage": "checking",
|
||||
"additionalContextLimit": 4096
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -46,6 +47,7 @@ fn hooks_file_deserializes_existing_json_shape() {
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: Some(4096),
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -90,6 +92,7 @@ type = "command"
|
||||
command = "python3 /tmp/pre.py"
|
||||
timeout = 10
|
||||
statusMessage = "checking"
|
||||
additionalContextLimit = 4096
|
||||
"#,
|
||||
)
|
||||
.expect("hook events TOML should deserialize");
|
||||
@@ -105,6 +108,7 @@ statusMessage = "checking"
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: Some(4096),
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -142,6 +146,7 @@ command = "python3 /tmp/pre.py"
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -187,6 +192,7 @@ command = "python3 /enterprise/place/pre.py"
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -223,6 +229,7 @@ command_windows = "powershell -File C:\\enterprise\\hooks\\pre.ps1"
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -258,9 +265,26 @@ commandWindows = "powershell -File C:\\enterprise\\hooks\\pre.ps1"
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_handler_omits_unset_additional_context_limit() {
|
||||
let handler = HookHandlerConfig::Command {
|
||||
command: "python3 /tmp/pre.py".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
};
|
||||
|
||||
let serialized = serde_json::to_value(handler).expect("hook handler should serialize");
|
||||
|
||||
assert_eq!(serialized.get("additionalContextLimit"), None);
|
||||
}
|
||||
|
||||
@@ -1198,6 +1198,12 @@
|
||||
"oneOf": [
|
||||
{
|
||||
"properties": {
|
||||
"additionalContextLimit": {
|
||||
"description": "Approximate token threshold for spilling this hook's `additionalContext` to disk. Unset uses 2,500 tokens; `0` disables spilling for this hook. The threshold is evaluated against the original context; a spilled preview also includes recovery metadata.",
|
||||
"format": "uint",
|
||||
"minimum": 0.0,
|
||||
"type": "integer"
|
||||
},
|
||||
"async": {
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
|
||||
@@ -1503,6 +1503,7 @@ async fn refresh_runtime_config_refreshes_hooks() -> anyhow::Result<()> {
|
||||
timeout_sec: Some(600),
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
},
|
||||
};
|
||||
|
||||
@@ -842,6 +842,34 @@ print(json.dumps({{
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_session_start_hooks_with_individual_context_limits(
|
||||
home: &Path,
|
||||
limited_additional_context: &str,
|
||||
expanded_additional_context: &str,
|
||||
) -> Result<()> {
|
||||
let mut hook_handlers = Vec::new();
|
||||
for (additional_context, additional_context_limit) in [
|
||||
(limited_additional_context, 1),
|
||||
(expanded_additional_context, 100),
|
||||
] {
|
||||
hook_handlers.push(serde_json::json!({
|
||||
"type": "command",
|
||||
"command": format!("echo {additional_context}"),
|
||||
"additionalContextLimit": additional_context_limit,
|
||||
}));
|
||||
}
|
||||
let hooks = serde_json::json!({
|
||||
"hooks": {
|
||||
"SessionStart": [{
|
||||
"hooks": hook_handlers,
|
||||
}]
|
||||
}
|
||||
});
|
||||
|
||||
fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_compact_session_start_hook_with_context(
|
||||
home: &Path,
|
||||
additional_context: &str,
|
||||
@@ -1441,6 +1469,59 @@ async fn session_start_hook_spills_large_additional_context() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_start_hooks_apply_additional_context_limits_individually() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let response = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_assistant_message("msg-1", "hello from the reef"),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let limited_additional_context = "spill this limited reef context".to_string();
|
||||
let expanded_additional_context = "keep this expanded reef context inline".to_string();
|
||||
|
||||
let test = test_codex()
|
||||
.with_pre_build_hook({
|
||||
let limited_additional_context = limited_additional_context.clone();
|
||||
let expanded_additional_context = expanded_additional_context.clone();
|
||||
move |home| {
|
||||
write_session_start_hooks_with_individual_context_limits(
|
||||
home,
|
||||
&limited_additional_context,
|
||||
&expanded_additional_context,
|
||||
)
|
||||
.expect("failed to write session start hook test fixtures");
|
||||
}
|
||||
})
|
||||
.with_config(trust_discovered_hooks)
|
||||
.build(&server)
|
||||
.await?;
|
||||
|
||||
test.submit_turn("hello").await?;
|
||||
|
||||
let request = response.single_request();
|
||||
let developer_messages = request.message_input_texts("developer");
|
||||
let spilled_message = developer_messages
|
||||
.iter()
|
||||
.find(|message| spilled_hook_output_path(message).is_some())
|
||||
.context("spilled limited session start context")?;
|
||||
let path = spilled_hook_output_path(spilled_message).context("spill path")?;
|
||||
assert_eq!(fs::read_to_string(path)?, limited_additional_context);
|
||||
assert!(
|
||||
developer_messages
|
||||
.iter()
|
||||
.any(|message| message == &expanded_additional_context),
|
||||
"expected the expanded-limit hook context inline, got {developer_messages:?}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pre_tool_use_hook_spills_large_additional_context() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -69,6 +69,7 @@ mod tests {
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
},
|
||||
],
|
||||
}],
|
||||
|
||||
@@ -29,6 +29,8 @@ use crate::events::common::matcher_pattern_for_event;
|
||||
use crate::events::common::validate_matcher_pattern;
|
||||
use crate::events::session_end::SESSION_END_DEFAULT_TIMEOUT_SEC;
|
||||
use crate::events::session_end::SESSION_END_MAX_TIMEOUT_SEC;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
use crate::output_spill::DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT;
|
||||
use codex_protocol::protocol::HookHandlerType;
|
||||
use codex_protocol::protocol::HookSource;
|
||||
use codex_protocol::protocol::HookTrustStatus;
|
||||
@@ -468,6 +470,7 @@ fn append_matcher_groups(
|
||||
timeout_sec,
|
||||
r#async,
|
||||
status_message,
|
||||
additional_context_limit,
|
||||
} => {
|
||||
let command = if cfg!(windows) {
|
||||
command_windows.unwrap_or(command)
|
||||
@@ -501,12 +504,33 @@ fn append_matcher_groups(
|
||||
source.path.display()
|
||||
));
|
||||
}
|
||||
let additional_context_limit = if matches!(
|
||||
event_name,
|
||||
codex_protocol::protocol::HookEventName::PreToolUse
|
||||
| codex_protocol::protocol::HookEventName::PostToolUse
|
||||
| codex_protocol::protocol::HookEventName::SessionStart
|
||||
| codex_protocol::protocol::HookEventName::UserPromptSubmit
|
||||
| codex_protocol::protocol::HookEventName::SubagentStart
|
||||
) {
|
||||
additional_context_limit
|
||||
} else {
|
||||
if additional_context_limit.is_some() {
|
||||
warnings.push(format!(
|
||||
"ignoring additionalContextLimit for {event_name:?} hook in {}: this event cannot emit additionalContext",
|
||||
source.path.display()
|
||||
));
|
||||
}
|
||||
None
|
||||
};
|
||||
let normalized_additional_context_limit = additional_context_limit
|
||||
.filter(|limit| *limit != DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT);
|
||||
let normalized_handler = HookHandlerConfig::Command {
|
||||
command: command.clone(),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(timeout_sec),
|
||||
r#async,
|
||||
status_message: status_message.clone(),
|
||||
additional_context_limit: normalized_additional_context_limit,
|
||||
};
|
||||
let current_hash =
|
||||
command_hook_hash(event_name, matcher, &group, normalized_handler);
|
||||
@@ -529,6 +553,7 @@ fn append_matcher_groups(
|
||||
command: Some(command.clone()),
|
||||
timeout_sec,
|
||||
status_message: status_message.clone(),
|
||||
additional_context_limit,
|
||||
source_path: source.path.clone(),
|
||||
source: source.source,
|
||||
plugin_id: source.plugin_id.clone(),
|
||||
@@ -551,6 +576,9 @@ fn append_matcher_groups(
|
||||
command,
|
||||
timeout_sec,
|
||||
status_message,
|
||||
additional_context_limit: AdditionalContextLimit::from_config(
|
||||
additional_context_limit,
|
||||
),
|
||||
source_path: source.path.clone(),
|
||||
source: source.source,
|
||||
display_order: *display_order,
|
||||
@@ -703,7 +731,10 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::ConfiguredHandler;
|
||||
use super::HookListEntry;
|
||||
use super::append_matcher_groups;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
use crate::output_spill::DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT;
|
||||
use codex_config::HookHandlerConfig;
|
||||
use codex_config::HookStateToml;
|
||||
use codex_config::MatcherGroup;
|
||||
@@ -795,10 +826,113 @@ mod tests {
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}
|
||||
}
|
||||
|
||||
fn command_group_with_additional_context_limit(
|
||||
additional_context_limit: usize,
|
||||
) -> MatcherGroup {
|
||||
MatcherGroup {
|
||||
matcher: None,
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "echo hello".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: Some(additional_context_limit),
|
||||
}],
|
||||
}
|
||||
}
|
||||
|
||||
fn discover_command(
|
||||
event_name: HookEventName,
|
||||
additional_context_limit: Option<usize>,
|
||||
) -> (ConfiguredHandler, HookListEntry, Vec<String>) {
|
||||
let source_path = source_path();
|
||||
let hook_states = std::collections::HashMap::new();
|
||||
let mut handlers = Vec::new();
|
||||
let mut entries = Vec::new();
|
||||
let mut warnings = Vec::new();
|
||||
let mut display_order = 0;
|
||||
append_matcher_groups(
|
||||
&mut handlers,
|
||||
&mut entries,
|
||||
&mut warnings,
|
||||
&mut display_order,
|
||||
&hook_handler_source(&source_path, &hook_states),
|
||||
event_name,
|
||||
vec![match additional_context_limit {
|
||||
Some(limit) => command_group_with_additional_context_limit(limit),
|
||||
None => command_group(/*matcher*/ None),
|
||||
}],
|
||||
);
|
||||
(handlers.remove(0), entries.remove(0), warnings)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn supported_events_retain_per_handler_additional_context_limit_and_hash_it() {
|
||||
for event_name in [
|
||||
HookEventName::PreToolUse,
|
||||
HookEventName::PostToolUse,
|
||||
HookEventName::SessionStart,
|
||||
HookEventName::UserPromptSubmit,
|
||||
HookEventName::SubagentStart,
|
||||
] {
|
||||
let (_, default_entry, _) =
|
||||
discover_command(event_name, /*additional_context_limit*/ None);
|
||||
let (explicit_default_handler, explicit_default_entry, _) = discover_command(
|
||||
event_name,
|
||||
/*additional_context_limit*/ Some(DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT),
|
||||
);
|
||||
let (custom_handler, custom_entry, _) =
|
||||
discover_command(event_name, /*additional_context_limit*/ Some(20_000));
|
||||
let (unlimited_handler, unlimited_entry, _) =
|
||||
discover_command(event_name, /*additional_context_limit*/ Some(0));
|
||||
|
||||
assert_eq!(
|
||||
custom_handler.additional_context_limit,
|
||||
AdditionalContextLimit::from_config(Some(20_000))
|
||||
);
|
||||
assert_eq!(custom_entry.additional_context_limit, Some(20_000));
|
||||
assert_ne!(default_entry.current_hash, custom_entry.current_hash);
|
||||
assert_eq!(
|
||||
explicit_default_handler.additional_context_limit,
|
||||
AdditionalContextLimit::from_config(Some(DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT))
|
||||
);
|
||||
assert_eq!(
|
||||
explicit_default_entry.additional_context_limit,
|
||||
Some(DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT)
|
||||
);
|
||||
assert_eq!(
|
||||
default_entry.current_hash,
|
||||
explicit_default_entry.current_hash
|
||||
);
|
||||
assert_eq!(
|
||||
unlimited_handler.additional_context_limit,
|
||||
AdditionalContextLimit::from_config(Some(0))
|
||||
);
|
||||
assert_eq!(unlimited_entry.additional_context_limit, Some(0));
|
||||
assert_ne!(default_entry.current_hash, unlimited_entry.current_hash);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_event_warns_and_ignores_additional_context_limit() {
|
||||
let source_path = source_path();
|
||||
let (handler, _, warnings) = discover_command(
|
||||
HookEventName::Stop,
|
||||
/*additional_context_limit*/ Some(4_096),
|
||||
);
|
||||
|
||||
assert_eq!(handler.additional_context_limit, Default::default());
|
||||
assert_eq!(warnings.len(), 1);
|
||||
assert!(warnings[0].contains("ignoring additionalContextLimit for Stop hook"));
|
||||
assert!(warnings[0].contains(&source_path.display().to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_prompt_submit_ignores_invalid_matcher_during_discovery() {
|
||||
let mut handlers = Vec::new();
|
||||
@@ -826,6 +960,7 @@ mod tests {
|
||||
command: "echo hello".to_string(),
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: source_path.clone(),
|
||||
source: hook_source(),
|
||||
display_order: 0,
|
||||
@@ -861,6 +996,7 @@ mod tests {
|
||||
command: "echo hello".to_string(),
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: source_path.clone(),
|
||||
source: hook_source(),
|
||||
display_order: 0,
|
||||
@@ -894,6 +1030,7 @@ mod tests {
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
},
|
||||
HookHandlerConfig::Command {
|
||||
command: "echo clamped".to_string(),
|
||||
@@ -901,6 +1038,7 @@ mod tests {
|
||||
timeout_sec: Some(600),
|
||||
r#async: true,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
},
|
||||
],
|
||||
}],
|
||||
@@ -1088,6 +1226,7 @@ mod tests {
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -1118,6 +1257,7 @@ mod tests {
|
||||
timeout_sec: None,
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
);
|
||||
|
||||
@@ -236,6 +236,7 @@ mod tests {
|
||||
command: command.to_string(),
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: HookSource::User,
|
||||
display_order,
|
||||
|
||||
@@ -22,6 +22,7 @@ use crate::events::stop::StopOutcome;
|
||||
use crate::events::stop::StopRequest;
|
||||
use crate::events::user_prompt_submit::UserPromptSubmitOutcome;
|
||||
use crate::events::user_prompt_submit::UserPromptSubmitRequest;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
use crate::output_spill::HookOutputSpiller;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_plugin::PluginHookSource;
|
||||
@@ -47,6 +48,7 @@ pub(crate) struct ConfiguredHandler {
|
||||
pub command: String,
|
||||
pub timeout_sec: u64,
|
||||
pub status_message: Option<String>,
|
||||
pub additional_context_limit: AdditionalContextLimit,
|
||||
pub source_path: AbsolutePathBuf,
|
||||
pub source: HookSource,
|
||||
pub display_order: i64,
|
||||
@@ -89,6 +91,7 @@ pub struct HookListEntry {
|
||||
pub command: Option<String>,
|
||||
pub timeout_sec: u64,
|
||||
pub status_message: Option<String>,
|
||||
pub additional_context_limit: Option<usize>,
|
||||
pub source_path: AbsolutePathBuf,
|
||||
pub source: HookSource,
|
||||
pub plugin_id: Option<String>,
|
||||
@@ -174,23 +177,19 @@ impl ClaudeHooksEngine {
|
||||
request: SessionStartRequest,
|
||||
turn_id: Option<String>,
|
||||
) -> SessionStartOutcome {
|
||||
let session_id = request.session_id;
|
||||
let mut outcome =
|
||||
crate::events::session_start::run(&self.handlers, &self.shell, request, turn_id).await;
|
||||
outcome.additional_contexts = self
|
||||
.maybe_spill_texts(session_id, outcome.additional_contexts)
|
||||
.await;
|
||||
outcome
|
||||
crate::events::session_start::run(
|
||||
&self.handlers,
|
||||
&self.shell,
|
||||
&self.output_spiller,
|
||||
request,
|
||||
turn_id,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn run_pre_tool_use(&self, request: PreToolUseRequest) -> PreToolUseOutcome {
|
||||
let session_id = request.session_id;
|
||||
let mut outcome =
|
||||
crate::events::pre_tool_use::run(&self.handlers, &self.shell, request).await;
|
||||
outcome.additional_contexts = self
|
||||
.maybe_spill_texts(session_id, outcome.additional_contexts)
|
||||
.await;
|
||||
outcome
|
||||
crate::events::pre_tool_use::run(&self.handlers, &self.shell, &self.output_spiller, request)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn run_permission_request(
|
||||
@@ -205,11 +204,13 @@ impl ClaudeHooksEngine {
|
||||
request: PostToolUseRequest,
|
||||
) -> PostToolUseOutcome {
|
||||
let session_id = request.session_id;
|
||||
let mut outcome =
|
||||
crate::events::post_tool_use::run(&self.handlers, &self.shell, request).await;
|
||||
outcome.additional_contexts = self
|
||||
.maybe_spill_texts(session_id, outcome.additional_contexts)
|
||||
.await;
|
||||
let mut outcome = crate::events::post_tool_use::run(
|
||||
&self.handlers,
|
||||
&self.shell,
|
||||
&self.output_spiller,
|
||||
request,
|
||||
)
|
||||
.await;
|
||||
outcome.feedback_message = self
|
||||
.maybe_spill_text(session_id, outcome.feedback_message)
|
||||
.await;
|
||||
@@ -246,13 +247,13 @@ impl ClaudeHooksEngine {
|
||||
&self,
|
||||
request: UserPromptSubmitRequest,
|
||||
) -> UserPromptSubmitOutcome {
|
||||
let session_id = request.session_id;
|
||||
let mut outcome =
|
||||
crate::events::user_prompt_submit::run(&self.handlers, &self.shell, request).await;
|
||||
outcome.additional_contexts = self
|
||||
.maybe_spill_texts(session_id, outcome.additional_contexts)
|
||||
.await;
|
||||
outcome
|
||||
crate::events::user_prompt_submit::run(
|
||||
&self.handlers,
|
||||
&self.shell,
|
||||
&self.output_spiller,
|
||||
request,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) fn preview_stop(&self, request: &StopRequest) -> Vec<HookRunSummary> {
|
||||
@@ -276,12 +277,6 @@ impl ClaudeHooksEngine {
|
||||
outcome
|
||||
}
|
||||
|
||||
async fn maybe_spill_texts(&self, session_id: ThreadId, texts: Vec<String>) -> Vec<String> {
|
||||
self.output_spiller
|
||||
.maybe_spill_texts(session_id, texts)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn maybe_spill_text(&self, session_id: ThreadId, text: Option<String>) -> Option<String> {
|
||||
match text {
|
||||
Some(text) => Some(self.output_spiller.maybe_spill_text(session_id, text).await),
|
||||
|
||||
@@ -66,6 +66,7 @@ fn pre_tool_use_hook_events(command: impl Into<String>) -> HookEventsToml {
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -173,6 +174,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -279,6 +281,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() {
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -358,6 +361,7 @@ fn unknown_requirement_source_hooks_stay_managed() {
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -426,6 +430,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() {
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -667,6 +672,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -1320,6 +1326,7 @@ print(json.dumps({
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
@@ -1439,6 +1446,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
|
||||
timeout_sec: Some(5),
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
|
||||
@@ -7,6 +7,7 @@ use codex_protocol::protocol::HookRunSummary;
|
||||
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
|
||||
/// Identifies a thread-spawned subagent when a normal hook runs inside it.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -34,19 +35,23 @@ pub(crate) fn trimmed_non_empty(text: &str) -> Option<String> {
|
||||
|
||||
pub(crate) fn append_additional_context(
|
||||
entries: &mut Vec<HookOutputEntry>,
|
||||
additional_contexts_for_model: &mut Vec<String>,
|
||||
additional_contexts_for_model: &mut Vec<AdditionalContext>,
|
||||
handler: &ConfiguredHandler,
|
||||
additional_context: String,
|
||||
) {
|
||||
entries.push(HookOutputEntry {
|
||||
kind: HookOutputEntryKind::Context,
|
||||
text: additional_context.clone(),
|
||||
});
|
||||
additional_contexts_for_model.push(additional_context);
|
||||
additional_contexts_for_model.push(AdditionalContext {
|
||||
text: additional_context,
|
||||
limit: handler.additional_context_limit,
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn flatten_additional_contexts<'a>(
|
||||
additional_contexts: impl IntoIterator<Item = &'a [String]>,
|
||||
) -> Vec<String> {
|
||||
additional_contexts: impl IntoIterator<Item = &'a [AdditionalContext]>,
|
||||
) -> Vec<AdditionalContext> {
|
||||
additional_contexts
|
||||
.into_iter()
|
||||
.flat_map(|chunk| chunk.iter().cloned())
|
||||
|
||||
@@ -600,6 +600,7 @@ mod tests {
|
||||
command: "python3 compact_hook.py".to_string(),
|
||||
timeout_sec: 5,
|
||||
status_message: Some("running compact hook".to_string()),
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
|
||||
@@ -16,6 +16,8 @@ use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
use crate::output_spill::HookOutputSpiller;
|
||||
use crate::schema::PostToolUseCommandInput;
|
||||
use crate::schema::SubagentCommandInputFields;
|
||||
|
||||
@@ -46,7 +48,7 @@ pub struct PostToolUseOutcome {
|
||||
#[derive(Debug, Default, PartialEq, Eq)]
|
||||
struct PostToolUseHandlerData {
|
||||
should_block: bool,
|
||||
additional_contexts_for_model: Vec<String>,
|
||||
additional_contexts_for_model: Vec<AdditionalContext>,
|
||||
feedback_messages_for_model: Vec<String>,
|
||||
}
|
||||
|
||||
@@ -70,8 +72,10 @@ pub(crate) fn preview(
|
||||
pub(crate) async fn run(
|
||||
handlers: &[ConfiguredHandler],
|
||||
shell: &CommandShell,
|
||||
output_spiller: &HookOutputSpiller,
|
||||
request: PostToolUseRequest,
|
||||
) -> PostToolUseOutcome {
|
||||
let session_id = request.session_id;
|
||||
let matcher_inputs = common::matcher_inputs(&request.tool_name, &request.matcher_aliases);
|
||||
let matched = dispatcher::select_handlers_for_matcher_inputs(
|
||||
handlers,
|
||||
@@ -115,6 +119,9 @@ pub(crate) async fn run(
|
||||
.iter()
|
||||
.map(|result| result.data.additional_contexts_for_model.as_slice()),
|
||||
);
|
||||
let additional_contexts = output_spiller
|
||||
.maybe_spill_additional_contexts(session_id, additional_contexts)
|
||||
.await;
|
||||
let should_block = results.iter().any(|result| result.data.should_block);
|
||||
let feedback_message = common::join_text_chunks(
|
||||
results
|
||||
@@ -199,6 +206,7 @@ fn parse_completed(
|
||||
common::append_additional_context(
|
||||
&mut entries,
|
||||
&mut additional_contexts_for_model,
|
||||
handler,
|
||||
additional_context,
|
||||
);
|
||||
}
|
||||
@@ -327,6 +335,8 @@ mod tests {
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::events::common;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
|
||||
#[test]
|
||||
fn command_input_uses_request_tool_name() {
|
||||
@@ -365,8 +375,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn additional_context_is_recorded() {
|
||||
let mut handler = handler();
|
||||
handler.additional_context_limit = AdditionalContextLimit::from_config(Some(17));
|
||||
let parsed = parse_completed(
|
||||
&handler(),
|
||||
&handler,
|
||||
run_result(
|
||||
Some(0),
|
||||
r#"{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"Remember the bash cleanup note."}}"#,
|
||||
@@ -379,7 +391,10 @@ mod tests {
|
||||
parsed.data,
|
||||
PostToolUseHandlerData {
|
||||
should_block: false,
|
||||
additional_contexts_for_model: vec!["Remember the bash cleanup note.".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "Remember the bash cleanup note.".to_string(),
|
||||
limit: AdditionalContextLimit::from_config(Some(17)),
|
||||
}],
|
||||
feedback_messages_for_model: Vec::new(),
|
||||
}
|
||||
);
|
||||
@@ -564,6 +579,7 @@ mod tests {
|
||||
command: "python3 post_tool_use_hook.py".to_string(),
|
||||
timeout_sec: 5,
|
||||
status_message: Some("running post tool use hook".to_string()),
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
|
||||
@@ -16,6 +16,8 @@ use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
use crate::output_spill::HookOutputSpiller;
|
||||
use crate::schema::PreToolUseCommandInput;
|
||||
use crate::schema::SubagentCommandInputFields;
|
||||
|
||||
@@ -47,7 +49,7 @@ pub struct PreToolUseOutcome {
|
||||
struct PreToolUseHandlerData {
|
||||
should_block: bool,
|
||||
block_reason: Option<String>,
|
||||
additional_contexts_for_model: Vec<String>,
|
||||
additional_contexts_for_model: Vec<AdditionalContext>,
|
||||
updated_input: Option<Value>,
|
||||
}
|
||||
|
||||
@@ -71,8 +73,10 @@ pub(crate) fn preview(
|
||||
pub(crate) async fn run(
|
||||
handlers: &[ConfiguredHandler],
|
||||
shell: &CommandShell,
|
||||
output_spiller: &HookOutputSpiller,
|
||||
request: PreToolUseRequest,
|
||||
) -> PreToolUseOutcome {
|
||||
let session_id = request.session_id;
|
||||
let matcher_inputs = common::matcher_inputs(&request.tool_name, &request.matcher_aliases);
|
||||
let matched = dispatcher::select_handlers_for_matcher_inputs(
|
||||
handlers,
|
||||
@@ -121,6 +125,9 @@ pub(crate) async fn run(
|
||||
.iter()
|
||||
.map(|result| result.data.additional_contexts_for_model.as_slice()),
|
||||
);
|
||||
let additional_contexts = output_spiller
|
||||
.maybe_spill_additional_contexts(session_id, additional_contexts)
|
||||
.await;
|
||||
let updated_input = if should_block {
|
||||
None
|
||||
} else {
|
||||
@@ -227,6 +234,7 @@ fn parse_completed(
|
||||
common::append_additional_context(
|
||||
&mut entries,
|
||||
&mut additional_contexts_for_model,
|
||||
handler,
|
||||
additional_context,
|
||||
);
|
||||
}
|
||||
@@ -331,6 +339,8 @@ mod tests {
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::events::common;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
|
||||
#[test]
|
||||
fn command_input_uses_request_tool_name() {
|
||||
@@ -508,7 +518,10 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: true,
|
||||
block_reason: Some("do not run that".to_string()),
|
||||
additional_contexts_for_model: vec!["remember this".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "remember this".to_string(),
|
||||
limit: Default::default(),
|
||||
}],
|
||||
updated_input: None,
|
||||
}
|
||||
);
|
||||
@@ -588,8 +601,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn additional_context_is_recorded() {
|
||||
let mut handler = handler();
|
||||
handler.additional_context_limit = AdditionalContextLimit::from_config(Some(13));
|
||||
let parsed = parse_completed(
|
||||
&handler(),
|
||||
&handler,
|
||||
run_result(
|
||||
Some(0),
|
||||
r#"{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"do not run that","additionalContext":"nope"}}"#,
|
||||
@@ -603,7 +618,10 @@ mod tests {
|
||||
PreToolUseHandlerData {
|
||||
should_block: true,
|
||||
block_reason: Some("do not run that".to_string()),
|
||||
additional_contexts_for_model: vec!["nope".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "nope".to_string(),
|
||||
limit: AdditionalContextLimit::from_config(Some(13)),
|
||||
}],
|
||||
updated_input: None,
|
||||
}
|
||||
);
|
||||
@@ -745,6 +763,7 @@ mod tests {
|
||||
command: "echo hook".to_string(),
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
|
||||
@@ -65,6 +65,7 @@ fn handler(matcher: Option<&str>) -> ConfiguredHandler {
|
||||
command: "echo hook".to_string(),
|
||||
timeout_sec: 2,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
|
||||
@@ -15,6 +15,8 @@ use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
use crate::output_spill::HookOutputSpiller;
|
||||
use crate::schema::NullableString;
|
||||
use crate::schema::SessionStartCommandInput;
|
||||
use crate::schema::SubagentStartCommandInput;
|
||||
@@ -88,7 +90,7 @@ pub struct SessionStartOutcome {
|
||||
struct SessionStartHandlerData {
|
||||
should_stop: bool,
|
||||
stop_reason: Option<String>,
|
||||
additional_contexts_for_model: Vec<String>,
|
||||
additional_contexts_for_model: Vec<AdditionalContext>,
|
||||
}
|
||||
|
||||
pub(crate) fn preview(
|
||||
@@ -108,9 +110,11 @@ pub(crate) fn preview(
|
||||
pub(crate) async fn run(
|
||||
handlers: &[ConfiguredHandler],
|
||||
shell: &CommandShell,
|
||||
output_spiller: &HookOutputSpiller,
|
||||
request: SessionStartRequest,
|
||||
turn_id: Option<String>,
|
||||
) -> SessionStartOutcome {
|
||||
let session_id = request.session_id;
|
||||
let matched = dispatcher::select_handlers(
|
||||
handlers,
|
||||
request.target.event_name(),
|
||||
@@ -199,6 +203,9 @@ pub(crate) async fn run(
|
||||
.iter()
|
||||
.map(|result| result.data.additional_contexts_for_model.as_slice()),
|
||||
);
|
||||
let additional_contexts = output_spiller
|
||||
.maybe_spill_additional_contexts(session_id, additional_contexts)
|
||||
.await;
|
||||
|
||||
SessionStartOutcome {
|
||||
hook_events: results.into_iter().map(|result| result.completed).collect(),
|
||||
@@ -258,6 +265,7 @@ fn parse_completed(
|
||||
common::append_additional_context(
|
||||
&mut entries,
|
||||
&mut additional_contexts_for_model,
|
||||
handler,
|
||||
additional_context,
|
||||
);
|
||||
}
|
||||
@@ -297,6 +305,7 @@ fn parse_completed(
|
||||
common::append_additional_context(
|
||||
&mut entries,
|
||||
&mut additional_contexts_for_model,
|
||||
handler,
|
||||
additional_context,
|
||||
);
|
||||
}
|
||||
@@ -357,11 +366,15 @@ mod tests {
|
||||
use super::parse_completed;
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
|
||||
#[test]
|
||||
fn plain_stdout_becomes_model_context() {
|
||||
let mut handler = handler();
|
||||
handler.additional_context_limit = AdditionalContextLimit::from_config(Some(7));
|
||||
let parsed = parse_completed(
|
||||
&handler(),
|
||||
&handler,
|
||||
run_result(Some(0), "hello from hook\n", ""),
|
||||
/*turn_id*/ None,
|
||||
);
|
||||
@@ -371,7 +384,10 @@ mod tests {
|
||||
SessionStartHandlerData {
|
||||
should_stop: false,
|
||||
stop_reason: None,
|
||||
additional_contexts_for_model: vec!["hello from hook".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "hello from hook".to_string(),
|
||||
limit: AdditionalContextLimit::from_config(Some(7)),
|
||||
}],
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.run.status, HookRunStatus::Completed);
|
||||
@@ -401,7 +417,10 @@ mod tests {
|
||||
SessionStartHandlerData {
|
||||
should_stop: true,
|
||||
stop_reason: Some("pause".to_string()),
|
||||
additional_contexts_for_model: vec!["do not inject".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "do not inject".to_string(),
|
||||
limit: Default::default(),
|
||||
}],
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.run.status, HookRunStatus::Stopped);
|
||||
@@ -452,8 +471,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn subagent_start_plain_stdout_becomes_model_context() {
|
||||
let mut handler = handler_for(HookEventName::SubagentStart);
|
||||
handler.additional_context_limit = AdditionalContextLimit::from_config(Some(4_096));
|
||||
let parsed = parse_completed(
|
||||
&handler_for(HookEventName::SubagentStart),
|
||||
&handler,
|
||||
run_result(Some(0), "hello from subagent hook\n", ""),
|
||||
/*turn_id*/ Some("turn-1".to_string()),
|
||||
);
|
||||
@@ -463,7 +484,10 @@ mod tests {
|
||||
SessionStartHandlerData {
|
||||
should_stop: false,
|
||||
stop_reason: None,
|
||||
additional_contexts_for_model: vec!["hello from subagent hook".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "hello from subagent hook".to_string(),
|
||||
limit: AdditionalContextLimit::from_config(Some(4_096)),
|
||||
}],
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.turn_id.as_deref(), Some("turn-1"));
|
||||
@@ -494,7 +518,10 @@ mod tests {
|
||||
SessionStartHandlerData {
|
||||
should_stop: false,
|
||||
stop_reason: None,
|
||||
additional_contexts_for_model: vec!["child context".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "child context".to_string(),
|
||||
limit: Default::default(),
|
||||
}],
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.turn_id.as_deref(), Some("turn-1"));
|
||||
@@ -519,6 +546,7 @@ mod tests {
|
||||
command: "echo hook".to_string(),
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
|
||||
@@ -635,6 +635,7 @@ mod tests {
|
||||
command: "echo hook".to_string(),
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
|
||||
@@ -15,6 +15,8 @@ use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
use crate::output_spill::HookOutputSpiller;
|
||||
use crate::schema::NullableString;
|
||||
use crate::schema::SubagentCommandInputFields;
|
||||
use crate::schema::UserPromptSubmitCommandInput;
|
||||
@@ -43,7 +45,7 @@ pub struct UserPromptSubmitOutcome {
|
||||
struct UserPromptSubmitHandlerData {
|
||||
should_stop: bool,
|
||||
stop_reason: Option<String>,
|
||||
additional_contexts_for_model: Vec<String>,
|
||||
additional_contexts_for_model: Vec<AdditionalContext>,
|
||||
}
|
||||
|
||||
pub(crate) fn preview(
|
||||
@@ -63,8 +65,10 @@ pub(crate) fn preview(
|
||||
pub(crate) async fn run(
|
||||
handlers: &[ConfiguredHandler],
|
||||
shell: &CommandShell,
|
||||
output_spiller: &HookOutputSpiller,
|
||||
request: UserPromptSubmitRequest,
|
||||
) -> UserPromptSubmitOutcome {
|
||||
let session_id = request.session_id;
|
||||
let matched = dispatcher::select_handlers(
|
||||
handlers,
|
||||
HookEventName::UserPromptSubmit,
|
||||
@@ -121,6 +125,9 @@ pub(crate) async fn run(
|
||||
.iter()
|
||||
.map(|result| result.data.additional_contexts_for_model.as_slice()),
|
||||
);
|
||||
let additional_contexts = output_spiller
|
||||
.maybe_spill_additional_contexts(session_id, additional_contexts)
|
||||
.await;
|
||||
|
||||
UserPromptSubmitOutcome {
|
||||
hook_events: results.into_iter().map(|result| result.completed).collect(),
|
||||
@@ -168,6 +175,7 @@ fn parse_completed(
|
||||
common::append_additional_context(
|
||||
&mut entries,
|
||||
&mut additional_contexts_for_model,
|
||||
handler,
|
||||
additional_context,
|
||||
);
|
||||
}
|
||||
@@ -210,6 +218,7 @@ fn parse_completed(
|
||||
common::append_additional_context(
|
||||
&mut entries,
|
||||
&mut additional_contexts_for_model,
|
||||
handler,
|
||||
additional_context,
|
||||
);
|
||||
}
|
||||
@@ -287,6 +296,7 @@ mod tests {
|
||||
use super::parse_completed;
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
|
||||
#[test]
|
||||
fn continue_false_preserves_context_for_later_turns() {
|
||||
@@ -305,7 +315,10 @@ mod tests {
|
||||
UserPromptSubmitHandlerData {
|
||||
should_stop: true,
|
||||
stop_reason: Some("pause".to_string()),
|
||||
additional_contexts_for_model: vec!["do not inject".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "do not inject".to_string(),
|
||||
limit: Default::default(),
|
||||
}],
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.run.status, HookRunStatus::Stopped);
|
||||
@@ -341,7 +354,10 @@ mod tests {
|
||||
UserPromptSubmitHandlerData {
|
||||
should_stop: true,
|
||||
stop_reason: Some("slow down".to_string()),
|
||||
additional_contexts_for_model: vec!["do not inject".to_string()],
|
||||
additional_contexts_for_model: vec![AdditionalContext {
|
||||
text: "do not inject".to_string(),
|
||||
limit: Default::default(),
|
||||
}],
|
||||
}
|
||||
);
|
||||
assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked);
|
||||
@@ -424,6 +440,7 @@ mod tests {
|
||||
command: "echo hook".to_string(),
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
|
||||
@@ -9,7 +9,32 @@ use tracing::warn;
|
||||
use uuid::Uuid;
|
||||
|
||||
const HOOK_OUTPUTS_DIR: &str = "hook_outputs";
|
||||
const HOOK_OUTPUT_TOKEN_LIMIT: usize = 2_500;
|
||||
pub(crate) const DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT: usize = 2_500;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) struct AdditionalContextLimit {
|
||||
token_limit: usize,
|
||||
}
|
||||
|
||||
impl AdditionalContextLimit {
|
||||
pub(crate) fn from_config(value: Option<usize>) -> Self {
|
||||
Self {
|
||||
token_limit: value.unwrap_or(DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for AdditionalContextLimit {
|
||||
fn default() -> Self {
|
||||
Self::from_config(/*value*/ None)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct AdditionalContext {
|
||||
pub text: String,
|
||||
pub limit: AdditionalContextLimit,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct HookOutputSpiller {
|
||||
@@ -31,7 +56,18 @@ impl HookOutputSpiller {
|
||||
/// and replaced with the same head/tail preview style used for other truncated
|
||||
/// output, plus a path back to the preserved full text.
|
||||
pub(crate) async fn maybe_spill_text(&self, thread_id: ThreadId, text: String) -> String {
|
||||
if approx_token_count(&text) <= HOOK_OUTPUT_TOKEN_LIMIT {
|
||||
self.maybe_spill_text_with_limit(thread_id, text, AdditionalContextLimit::default())
|
||||
.await
|
||||
}
|
||||
|
||||
async fn maybe_spill_text_with_limit(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
text: String,
|
||||
limit: AdditionalContextLimit,
|
||||
) -> String {
|
||||
let token_limit = limit.token_limit;
|
||||
if token_limit == 0 || approx_token_count(&text) <= token_limit {
|
||||
return text;
|
||||
}
|
||||
|
||||
@@ -43,31 +79,28 @@ impl HookOutputSpiller {
|
||||
"failed to create hook output directory {}: {err}",
|
||||
parent.display()
|
||||
);
|
||||
return formatted_truncate_text(
|
||||
&text,
|
||||
TruncationPolicy::Tokens(HOOK_OUTPUT_TOKEN_LIMIT),
|
||||
);
|
||||
return formatted_truncate_text(&text, TruncationPolicy::Tokens(token_limit));
|
||||
}
|
||||
|
||||
if let Err(err) = fs::write(path.as_ref(), &text).await {
|
||||
warn!("failed to write hook output {}: {err}", path.display());
|
||||
return formatted_truncate_text(
|
||||
&text,
|
||||
TruncationPolicy::Tokens(HOOK_OUTPUT_TOKEN_LIMIT),
|
||||
);
|
||||
return formatted_truncate_text(&text, TruncationPolicy::Tokens(token_limit));
|
||||
}
|
||||
|
||||
spilled_hook_output_preview(&text, &path)
|
||||
spilled_hook_output_preview(&text, &path, token_limit)
|
||||
}
|
||||
|
||||
pub(crate) async fn maybe_spill_texts(
|
||||
pub(crate) async fn maybe_spill_additional_contexts(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
texts: Vec<String>,
|
||||
contexts: Vec<AdditionalContext>,
|
||||
) -> Vec<String> {
|
||||
let mut spilled = Vec::with_capacity(texts.len());
|
||||
for text in texts {
|
||||
spilled.push(self.maybe_spill_text(thread_id, text).await);
|
||||
let mut spilled = Vec::with_capacity(contexts.len());
|
||||
for context in contexts {
|
||||
spilled.push(
|
||||
self.maybe_spill_text_with_limit(thread_id, context.text, context.limit)
|
||||
.await,
|
||||
);
|
||||
}
|
||||
spilled
|
||||
}
|
||||
@@ -97,12 +130,11 @@ fn hook_output_path(output_dir: &AbsolutePathBuf, thread_id: ThreadId) -> Absolu
|
||||
/// Builds the model-visible replacement for a spilled hook output.
|
||||
///
|
||||
/// The path footer is budgeted before truncation so adding the recovery path
|
||||
/// does not let the preview grow past the hook-output limit.
|
||||
fn spilled_hook_output_preview(text: &str, path: &AbsolutePathBuf) -> String {
|
||||
/// does not consume the configured preview budget.
|
||||
fn spilled_hook_output_preview(text: &str, path: &AbsolutePathBuf, token_limit: usize) -> String {
|
||||
let footer = format!("\n\nFull hook output saved to: {}", path.display());
|
||||
let preview_policy = TruncationPolicy::Tokens(
|
||||
HOOK_OUTPUT_TOKEN_LIMIT.saturating_sub(approx_token_count(&footer)),
|
||||
);
|
||||
let preview_policy =
|
||||
TruncationPolicy::Tokens(token_limit.saturating_sub(approx_token_count(&footer)));
|
||||
format!("{}{footer}", formatted_truncate_text(text, preview_policy))
|
||||
}
|
||||
|
||||
|
||||
@@ -40,3 +40,39 @@ async fn large_hook_output_spills_to_file() -> Result<()> {
|
||||
assert_eq!(fs::read_to_string(path).await?, text);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn additional_contexts_apply_limits_individually() -> Result<()> {
|
||||
let dir = tempdir()?;
|
||||
let limited_text = "limited hook output ".repeat(1_000);
|
||||
let unlimited_text = "unlimited hook output ".repeat(5_000);
|
||||
assert!(approx_token_count(&unlimited_text) > 10_000);
|
||||
let output_dir = AbsolutePathBuf::from_absolute_path(dir.path())?.join(HOOK_OUTPUTS_DIR);
|
||||
let spiller = HookOutputSpiller { output_dir };
|
||||
let output = spiller
|
||||
.maybe_spill_additional_contexts(
|
||||
ThreadId::new(),
|
||||
vec![
|
||||
AdditionalContext {
|
||||
text: limited_text.clone(),
|
||||
limit: AdditionalContextLimit::from_config(Some(1)),
|
||||
},
|
||||
AdditionalContext {
|
||||
text: unlimited_text.clone(),
|
||||
limit: AdditionalContextLimit::from_config(Some(0)),
|
||||
},
|
||||
AdditionalContext {
|
||||
text: unlimited_text.clone(),
|
||||
limit: AdditionalContextLimit::from_config(Some(usize::MAX)),
|
||||
},
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let [limited_output, zero_limit_output, high_limit_output] = output.as_slice() else {
|
||||
panic!("expected one output for each additional context");
|
||||
};
|
||||
assert!(limited_output.contains("Full hook output saved to:"));
|
||||
assert_eq!(zero_limit_output, &unlimited_text);
|
||||
assert_eq!(high_limit_output, &unlimited_text);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -492,6 +492,14 @@ impl HooksBrowserView {
|
||||
Some(MAX_COMMAND_DETAIL_LINES),
|
||||
));
|
||||
lines.push(detail_line("Timeout", &format!("{}s", hook.timeout_sec)));
|
||||
if let Some(limit) = hook.additional_context_limit {
|
||||
let value = if limit == 0 {
|
||||
"unlimited".to_string()
|
||||
} else {
|
||||
format!("limit: {limit} approximate tokens")
|
||||
};
|
||||
lines.push(detail_line("Context", &value));
|
||||
}
|
||||
lines.push(detail_line("Trust", hook_trust_label(hook.trust_status)));
|
||||
lines
|
||||
}
|
||||
@@ -936,6 +944,7 @@ mod tests {
|
||||
command: Some(command.to_string()),
|
||||
timeout_sec: 30,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source,
|
||||
plugin_id: plugin_id.map(str::to_string),
|
||||
@@ -1080,6 +1089,35 @@ mod tests {
|
||||
assert_snapshot!("hooks_browser_handlers", render_lines(&view, /*width*/ 112));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_handler_additional_context_limit() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
let mut configured_hook = hook(
|
||||
"path:context-limit",
|
||||
HookEventName::PreToolUse,
|
||||
HookSource::User,
|
||||
/*plugin_id*/ None,
|
||||
"/tmp/pre-tool-use.sh",
|
||||
/*enabled*/ true,
|
||||
/*is_managed*/ false,
|
||||
/*display_order*/ 0,
|
||||
);
|
||||
configured_hook.additional_context_limit = Some(0);
|
||||
let mut view = HooksBrowserView::new(
|
||||
vec![configured_hook],
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
AppEventSender::new(tx_raw),
|
||||
);
|
||||
|
||||
view.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
assert_snapshot!(
|
||||
"hooks_browser_additional_context_limit",
|
||||
render_lines(&view, /*width*/ 112)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_untrusted_enabled_handler_as_inactive() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
|
||||
+19
@@ -0,0 +1,19 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/hooks_browser_view.rs
|
||||
expression: "render_lines(&view, 112)"
|
||||
---
|
||||
|
||||
PreToolUse hooks
|
||||
Turn hooks on or off. Your changes are saved automatically.
|
||||
|
||||
[x] Hook 1
|
||||
|
||||
Event PreToolUse
|
||||
Matcher Bash
|
||||
Source User config - /tmp/hooks.json
|
||||
Command /tmp/pre-tool-use.sh
|
||||
Timeout 30s
|
||||
Context unlimited
|
||||
Trust Trusted
|
||||
|
||||
Press space or enter to toggle; esc to go back
|
||||
@@ -1258,6 +1258,7 @@ approval_policy = "never"
|
||||
timeout_sec: Some(10),
|
||||
r#async: false,
|
||||
status_message: Some("checking".to_string()),
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
|
||||
@@ -311,6 +311,7 @@ mod tests {
|
||||
command: Some("/tmp/hook.sh".to_string()),
|
||||
timeout_sec: 30,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: HookSource::User,
|
||||
plugin_id: None,
|
||||
|
||||
Reference in New Issue
Block a user