Repository navigation
Conversation
![kit-code-agent[bot]](https://avatars.githubusercontent.com/in/4797102?s=60&v=4){kind=small}
| self.inner = self.inner.with_backend(HookedCompactionBackend { | ||
| backend: backend.clone(), | ||
| hooks, | ||
| session_id: session_id.clone(), |
There was a problem hiding this comment.
[P2] Report compaction hooks with the owning session ID
With AHP hooks enabled, runtime compaction passes a private compaction-<UUID> (or compaction-{session}) ID into automatic(). This tuple retains that summarizer/model session ID, and HookedCompactionBackend then emits it as session.id in both context.compact.before and context.compact.after. Neither event identifies the session whose transcript is being compacted, so consumers keyed by the owning session.start ID cannot correlate the operation and can miss session-specific instruction or summary policies. Pass the owning native session ID separately when attaching hooks and use it for both compaction events; retain the private ID only for the summarizer model session.
For agents:
Validate the following issue, address if needed:
<comment>Compaction hook events use the private summarizer session ID instead of the owning native session ID, breaking session correlation and session-specific policies.</comment>
<file_context>
@@ -668,11 +870,24 @@ fn user_message_from_marker(
pub struct AutomaticCompactor {
inner: StrategyCompactor,
+ hook_backend: Option<(Arc<dyn CompactionBackend>, SessionId)>,
persistence: Option<SessionObserver>,
mirror: Option<crate::session::TransientTranscript>,
}
impl AutomaticCompactor {
+ /// Applies AHP edits at generation and pre-installation boundaries.
+ pub(crate) fn with_hooks(mut self, hooks: Arc<crate::ahp::AhpHost>) -> Self {
+ if let Some((backend, session_id)) = &self.hook_backend {
+ self.inner = self.inner.with_backend(HookedCompactionBackend {
+ backend: backend.clone(),
+ hooks,
+ session_id: session_id.clone(),
+ });
+ }
+ self
+ }
+
/// Applies each replacement to an in-memory transcript as well.
pub(crate) fn mirroring(mut self, transcript: crate::session::TransientTranscript) -> Self {
self.mirror = Some(transcript);
</file_context>
| "tool requires an execution scope".into(), | ||
| )); | ||
| }; | ||
| scope.executor = match &self.hooks { |
There was a problem hiding this comment.
[P2] Preserve the MCP child terminal receipt on cancellation or timeout
Once a dynamically discovered MCP child completes tool.before, interrupting the turn or reaching timeout_seconds while remote execution is pending drops dispatch_call and its wrapped child future. AhpToolExecutor::run only calls hooks.after after awaiting the inner execution, so this path never emits the child's terminal tool.after; the timeout can also drop an in-progress after-hook delivery. The enclosing native tool wrapper's receipt has a different event identity and cannot close that child lineage, leaving hook consumers with an admitted invocation that never terminates. Retain the admitted child's effective request and event identity across cancellation/timeout, then await its bounded cancellation/error receipt outside the dropped dispatch future while preserving invocation/replay cleanup.
For agents:
Validate the following issue, address if needed:
<comment>Cancellation or timeout drops the hooked MCP child before its terminal receipt completes, leaving its admitted tool event lineage unclosed.</comment>
<file_context>
@@ -3446,11 +3454,19 @@ impl McpTool {
Err(error) => return ToolExecutionOutcome::Failed(error),
};
let tool_name = ToolName::new(name.clone());
- let Some(scope) = context.execution_scope.clone() else {
+ let Some(mut scope) = context.execution_scope.clone() else {
return ToolExecutionOutcome::Failed(ToolError::Unavailable(
"tool requires an execution scope".into(),
));
};
+ scope.executor = match &self.hooks {
+ Some(hooks) => Arc::new(super::ahp::AhpToolExecutor::for_context(
+ self.executor.clone(),
+ hooks.clone(),
+ context,
+ )),
+ None => self.executor.clone(),
+ };
let timeout_name = name.clone();
let call = tokio::time::timeout(timeout, async {
// A resumed conversation can invoke a previously discovered tool
</file_context>
There was a problem hiding this comment.
Two previously reported P2 findings remain unresolved:
- Compaction hooks still emit the private summarizer session ID rather than the owning session ID, breaking session correlation and session-specific policies.
- Cancelling or timing out an admitted MCP child still drops its terminal
tool.afterreceipt, leaving that child event lineage unclosed.
| (global) and `<root>/.agents/hooks.json` (project-local). No `[ahp]` TOML table is | ||
| required. If neither file exists, hooks are disabled. Registration validation, |
There was a problem hiding this comment.
| (global) and `<root>/.agents/hooks.json` (project-local). No `[ahp]` TOML table is | |
| required. If neither file exists, hooks are disabled. Registration validation, | |
| (global) and `<root>/.agents/hooks.json` (project-local). If neither file exists, hooks are disabled. Registration validation, |
implementation posterity
|
|
||
| ## Configure hooks | ||
|
|
||
| Put a canonical registration directly in either `hooks.json` file: |
There was a problem hiding this comment.
| Put a canonical registration directly in either `hooks.json` file: | |
| Put a registration directly in either `hooks.json` file: |
There was a problem hiding this comment.
this file needs to be cleaned up, it's a blend of story telling, actual user docs, implementation details, tests (???); preserve user docs for user-facing docs
| `<root>/.agents/hooks.json`. Both canonical registrations are merged by hook ID; | ||
| project-local definitions replace conflicting global definitions. No TOML hook | ||
| configuration is required. |
There was a problem hiding this comment.
why would TOML hook configuration be required?
| `a2a` applies to `serve` and `tui`. Configured plugins can provide MCP servers without `mcp_config`; supported `stdio` and `streamable-http` declarations are registered, while `sse` declarations are skipped with a stderr diagnostic. MCP servers merge by name in this order: plugins, configured `mcp_config`, `<canonical-root>/.mcp.json`, then `--mcp-config`. Higher layers replace whole conflicts while preserving non-conflicting lower entries; live removal reveals the next lower layer. Plugin data is stored under `<config-directory>/plugin-data/<plugin-manifest-name>`. See [Agent Plugins](agent-plugins.md) for placeholders, collision rules, and ACP child behavior. `otel_endpoint` enables OTLP export of AgentKit's GenAI trace spans. `otel_protocol` accepts exactly `grpc`, `http/protobuf`, or `http/json`; gRPC is the default. For either HTTP protocol, Kit appends `/v1/traces` to a base endpoint unless it already ends with that path. `credential_store` selects one backend for OpenAI, Speakeasy, and MCP and defaults to `memory`; selecting `file` requires `credential_dir`, while a credential directory is invalid with `memory` or `keychain`. Memory credentials are process-local and are not shared with the TUI server process or nested Kit children. Standalone OpenAI and Speakeasy login requires persistent `keychain` or `file` storage. ACP profiles are direct executable-and-argument configurations, not shell command strings. `[subagent].harness` must name an available fully qualified profile such as `acp.review`; otherwise startup reports `unknown subagent ACP harness`. When no subagent harness is selected, the built-in `acp.kit` profile is used. | ||
|
|
||
| Native AHP hooks are discovered from `~/.agents/hooks.json` and | ||
| `<root>/.agents/hooks.json`. Both canonical registrations are merged by hook ID; |
There was a problem hiding this comment.
can we stop using canonical, like it's "umm"
| let validate = || -> Result<Value, String> { | ||
| let value: Value = serde_json::from_slice(&bytes).map_err(|error| error.to_string())?; | ||
| let capabilities = grants | ||
| .iter() | ||
| .map(|(name, declaration)| { | ||
| EventGrant::from_declaration(declaration).map(|grant| (name.clone(), grant)) | ||
| }) | ||
| .collect::<Result<_, _>>() | ||
| .map_err(|error| error.to_string())?; | ||
| // The SDK validates the canonical schema, duplicate backend ids and | ||
| // registration routes. Its registered transports are lazy: construction | ||
| // does not spawn a process, send a request or start a worker/runtime. | ||
| Hooks::new( | ||
| &value, | ||
| HooksOptions::new("urn:kit:ahp:config", capabilities), | ||
| ) | ||
| .map_err(|error| error.to_string())?; | ||
| Ok(value) | ||
| }; | ||
| validate() | ||
| .map(Some) | ||
| .map_err(|error| format!("{}: {error}", path.display())) | ||
| } |
There was a problem hiding this comment.
this entire function is extremely oddly structured
| let global_path = global_path | ||
| .as_ref() | ||
| .ok_or_else(|| "AHP global registration has no source path".to_owned())?; | ||
| if global["protocolVersion"] != local["protocolVersion"] { |
There was a problem hiding this comment.
not our problem, SDK owns compatibility
| let hooks = global["hooks"].as_array_mut().ok_or_else(|| { | ||
| format!( | ||
| "{}: SDK-validated registration has no hooks array", | ||
| global_path.display() | ||
| ) | ||
| })?; | ||
| let local_hooks = local["hooks"].as_array().ok_or_else(|| { | ||
| format!( | ||
| "{}: SDK-validated registration has no hooks array", | ||
| local_path.display() | ||
| ) | ||
| })?; | ||
| for hook in local_hooks { | ||
| if let Some(index) = hooks | ||
| .iter() | ||
| .position(|existing| existing["id"] == hook["id"]) | ||
| { | ||
| hooks[index] = hook.clone(); | ||
| } else { | ||
| hooks.push(hook.clone()); | ||
| } | ||
| } | ||
| let local_fields = local.as_object().ok_or_else(|| { | ||
| format!( | ||
| "{}: SDK-validated registration is not an object", | ||
| local_path.display() | ||
| ) | ||
| })?; | ||
| for (key, value) in local_fields { | ||
| if key != "hooks" { | ||
| global[key] = value.clone(); | ||
| } | ||
| } | ||
| global | ||
| } | ||
| }; |
There was a problem hiding this comment.
deserialize into the config struct, why are we trying to validate line by line?
There was a problem hiding this comment.
it'd probably be cleaner to define capabilities as a JSON that's then statically built into the binary
There was a problem hiding this comment.
is this python or rust? none of this should be required
1 participant
Summary
Integrate Agent Hooks Protocol into Kit’s native session, turn, model, tool, compaction, ACP configuration, and committed-edit boundaries. Use SDK 0.4.1 typed settlement for effective native write-back, with invocation-scoped compose/MCP lineage and global/project hook discovery.
Impact
Hooks can observe implemented boundaries, modify supported prompt/tool/output/compaction values, and deny supported operations. Tool input edits are checked through SDK contracts using native argument decoders where available and advertised schemas for runtime-only tools. Invalid configuration sources are warned and skipped independently; registered hook failure policies remain enforced.
Technical details
Native authority and persistence
Await native lifecycle boundaries rather than bridging observer notifications into tasks. Validate native transcript invariants before applying settled edits and persist committed transcript rewrites across resume and fork.
SDK-owned settlement and content
Use typed SDK settled events, transports, reducers, cancellation, and attachment ownership. Ordinary messages remain inline; binary attachments are immutable. Preserve opaque facts and decode failures without granting execution authority.
Tool argument contracts
Keep the tool-contract implementation in a separate commit. Fourteen builtins reuse their invocation decoders through lossless wire envelopes; schema-only tools and lazily discovered MCP tools use runtime contracts without claiming compile-time argument types.
Lifecycle-capable AgentKit
Pin AgentKit to revision
3d75d7179ad6ef4d675d6b9d88cc1864f59be759for awaited interception, terminal delivery, and transcript replacement notifications. Only implemented native effects are advertised.